[nnra6864]

This just works, so I don't see any harm in having it.
Preview is a bit ugly, but that's unrelated to .mkv.

The ugly preview seems to happen with both .mkv and .mp4 when you send the original, but compressing either just fixes them.
I could open a separate issue for this if you'd like.

It would be neat if we could have an option to remux a .mkv file to .mp4 when uploading(would be useful for bridges, like to discord for example, as discord doesn't support .mkv).
Remuxing can easily be done with the following ffmpeg command:

ffmpeg -i input.mkv -map 0 -c copy output.mp4

closes #1052

[Airyzz]

thanks, does this also correctly detect the mime type of an mkv file when you upload through commet?

[nnra6864]

thanks, does this also correctly detect the mime type of an mkv file when you upload through commet?

How can I check that?

Also, slightly unrelated, but I couldn't get opus to work no matter what, would you mind taking a look at this?
There's an open issue, and simply adding a mime type wasn't enough, even tho flutter should support it natively.
#1052

Sorry if this is not the right place to ask, I just wanted to add both as a part of this PR, but gave up when I couldn't make it work.

[Airyzz]

How can I check that?

After sending the message, you can view the event source and see which (if any) mime type was detected. If sending the message properly embeds the video, then its probably working.

Worth checking the same thing with opus, it might be supported by the player but just not detecting the file type properly

[nnra6864]

Ok yes, just checked, and it correctly detects matroska.
let me see what happens with opus.

[nnra6864]

And yep, you nailed it, opus is not being detected.

    "info": {
      "mimetype": "application/octet-stream",
      "size": 2831204
    }

This is the only reference to octet in the entire codebase I dug up, I am failing to see how it's related but I'll take a slightly deeper look.

commet/commet/lib/client/matrix/matrix_room.dart

Line 435 in b52451e

attachment.mimeType = 'application/octet-stream';

[Airyzz]

yea basically thats just the mime type for any generic sequence of bytes that it cant detect, you might want to take a look here

commet/commet/lib/utils/mime.dart

Line 89 in b52451e

resolver.addMagicNumber([0x42, 0x4d], "image/bmp");

to see how you can make it detect a different type based on the header of the file

[nnra6864]

Ok, so I tried doing what you suggested, but it still doesn't find it:

resolver.addMagicNumber([0x4F, 0x67, 0x67, 0x53, 0x00, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x2E, 0x4F, 0x70, 0x75, 0x73, 0x48, 0x65, 0x61, 0x64], "audio/opus"); // '<opus

I tried audio/opus, audio/ogg and audio/ogg; codec=opus(as suggested in the issue), but neither worked.

Do I have to populate the dictionaries above too?
Looking at the SVG example, it's not found in any of those.

[nnra6864]

Ok, so I managed to make this work actually, and I had to use a shorter magic number for some reason:

resolver.addMagicNumber([0x4F, 0x67, 0x67, 0x53], "audio/opus"); // '<opus

And I also had to add audio/opus to playable audio types.

Now, I'd appreciate your guidance on what mime I should actually use here.
audio/ogg, audio/opus, audio/ogg; codec=opus?
And which do I keep/add in the playable list?

Sorry for a bit of a spam, I didn't think I'd resolve it this quickly.

[Airyzz]

I think the magic number you are using, 0x4F, 0x67, 0x67, 0x53 which translates to OggS isn't enough information to say for sure this is an opus file, but it is enough to atleast identify an ogg container, which makes me think probably just audio/ogg should be used, otherwise we might detect other ogg files (which arent using opus) as an opus file.

Since afaik opus files are just ogg containers, it should be fine to treat them this way

[nnra6864]

This is ready for review, everything works as expected on my end.
There's only that weird media player scaling thing I mentioned above, but I could open a separate issue about that if you'd like.

Here's opus playing too:

[Airyzz]

I guess it could also be worth adding audio/opus to the playable audio types, just incase other clients are handling it differently. if you could also remove the < from the comment for OggS that would be good, that character only exists for the comment next to svg because the < itself is part of the file, since svg files start typically with <svg ...>

[Airyzz]

for the UI scaling thing, i would just check that in the event source, the provided width/height for the file seems accurate, otherwise im happy to look in to that myself, i think its a side effect of something else i had changed recently

[nnra6864]

I am assuming audio/ogg codec=opus should already be covered by audio/ogg?
If not, should I add that too just in case, or is that unnecessary?

As for the video scaling thing, you called it, it doesn't actually add that data when uploading original.

Processed:

    "info": {
      "mimetype": "video/x-matroska",
      "size": 2377807,
      "w": 2560,
      "h": 1440,
      "duration": 20007
    }

Original:

    "info": {
      "mimetype": "video/x-matroska",
      "size": 2377807
    }

Should I open an issue/pr for this?
I am genuinely not sure if I'd be capable of fixing it on my own, so whatever works best for you.

[Airyzz]

ah okay, yea thats a bug on my end, im happy to take a look in to that one

[Airyzz]

pushed a fix for that, let me know if that works for you!

I am assuming audio/ogg codec=opus should already be covered by audio/ogg?

I think yea just having audio/ogg should be sufficient

[nnra6864]

Works great!

[Airyzz]

Thanks!