Fix embedding for .mkv .webm #27117
Fix embedding for .mkv .webm #27117
Conversation
|
|
|
What if *.mkv file has 5 video and 10 audio tracks inside? What the client should to do? |
| @@ -111,6 +111,9 @@ MimeType MimeTypeForFile(const QFileInfo &file) { | |||
| return MimeType(MimeType::Known::TDesktopTheme); | |||
| } else if (path.endsWith(u".tdesktop-palette"_q, Qt::CaseInsensitive)) { | |||
| return MimeType(MimeType::Known::TDesktopPalette); | |||
| } else if (path.endsWith(u".mkv"_q, Qt::CaseInsensitive)) { | |||
| // This will ensure .mkv is detected as a video file by the client when drag-n-dropped | |||
| return MimeType(QMimeDatabase().mimeTypeForName("video/mp4")); | |||
There was a problem hiding this comment.
This doesn't look like a right change: if you want to allow .mkv to be videos then do exactly that, don't pretend it to be .mp4. But I guess @john-preston won't allow that. Also, most mkv videos are unlikely to be played by the official builds as they have ffmpeg built only with require codecs enabled.
There was a problem hiding this comment.
if you want to allow .mkv to be videos then do exactly that, don't pretend it to be .mp4
Yes, I do believe adding .mkv to extensions in FileLoadTask::CheckForVideo would achieve the same result.
static const auto extensions = {
u".mp4"_q,
u".mov"_q,
u".m4v"_q,
u".webm"_q,
};`
There was a problem hiding this comment.
You can also add the mimetype. Searching by video/mp4 it's not the only place where it has to be added for full-feature support.
| @@ -875,6 +875,11 @@ ChatRestriction DocumentData::requiredSendRight() const { | |||
| void DocumentData::setFileName(const QString &remoteFileName) { | |||
| _filename = remoteFileName; | |||
|
|
|||
| // Needed in order to force the server to embed the video | |||
| if (_filename.toLower().endsWith(".webm")) { | |||
There was a problem hiding this comment.
This really looks like a candidate for various forks as getting through explicit server-side restrictions in an official client looks very weird.
There was a problem hiding this comment.
- Restricting .webm embedding server-side doesn't make any sense
- User can achieve the same result by changing the file extension manually
There was a problem hiding this comment.
This doesn't mean official client is allowed to help the user to overcome server-side restriction
There was a problem hiding this comment.
If there's such a server-side restriction, then FileLoadTask::CheckForVideo probably shouldn't return true for webms resulting in broken embedding
imho, prefer defaults :) |
Related: #10000
Client doesn't treat .mkv as a video file type when user tries to upload one.
Fixing that by returning "video/mp4" in MimeTypeForFile
Related: #24003
Client fools the user by displaying uploaded .webm videos as embedded for some time, but they do not get embedded server-side and turn into documents. Why not fix it with the most logical solution of supplying .webm videos with MIME type "video/mp4" and changing the filename: filename.webm to filename[webm].mp4