Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Videos greater than 25MB uploaded as m.file rather than m.video (clone of 1752 from riot-ios) #6426

Open
repli-a opened this issue Apr 1, 2018 · 10 comments
Labels
A-Media P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@repli-a
Copy link

repli-a commented Apr 1, 2018

Copy of 1752 from riot-ios.

On all versions of the Riot app (web, Android, iOS), any video file over 25MB is uploaded as an m.file rather than m.video, and is not presented in an embedded HTML5 player as a result. Would be great to be able to specify where this cutoff is in case we want to use this for sending larger screeners.

It looks like @manuroe already has this in the pipeline on the iOS side, per @turt2live posting here to make sure there's visibility on the desktop app side as well.

Thanks!

@lampholder lampholder added T-Defect P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround A-Media labels Apr 16, 2018
@repli-a
Copy link
Author

repli-a commented May 8, 2018

Any updates on this bug?

@t3chguy
Copy link
Member

t3chguy commented Jun 1, 2018

Did some investigation
cannot repro on Firefox.
Can easily repro on Chrome/Electron

image

@repli-a
Copy link
Author

repli-a commented Jun 1, 2018

Thanks Michael. Will try to get this across to the Chrome devs and see if there's anything they can do to rectify.

@t3chguy
Copy link
Member

t3chguy commented Jun 1, 2018

@repli-a
Copy link
Author

repli-a commented Jun 3, 2018

Tested this in Firefox on client hosted at riot.im today and was able to reproduce the issue. Does not seem to be limited to Chrome. Don't see the same ffmpeg error in the console though.

@repli-a
Copy link
Author

repli-a commented Jun 3, 2018

I also confirmed that even on Firefox 60.0.1 (64-bit), in a non-E2E room, this problem occurs.

@t3chguy
Copy link
Member

t3chguy commented Jun 3, 2018

The code path doesn't log to console, you need to breakpoint on it

@repli-a
Copy link
Author

repli-a commented Jun 3, 2018

I have a breakpoint on lines 165 through 167 in ContentMessages.js as you indicated above, but I don't receive an error in the console on upload completion even when the upload is incorrectly categorized as m.file.

@Half-Shot
Copy link
Member

Starting to wonder if this might be related to #4264, except it doesn't quite go as far as crashing the app. Chrome/Electron doesn't seem to like large files sent as file objects.

@turt2live
Copy link
Member

I think this one is much more closely related to #7069 given I had a 4.77kb image end up being an m.file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Media P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Development

No branches or pull requests

5 participants