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

Matroska container header meta data parsing #710

Merged
merged 1 commit into from
Sep 3, 2023
Merged

Matroska container header meta data parsing #710

merged 1 commit into from
Sep 3, 2023

Conversation

GenericGuy
Copy link
Contributor

@GenericGuy GenericGuy commented Sep 3, 2023

From what I've gathere online, Matroska (mkv) files don't provide the same interface as MP4 files for reading metadata. Duration and creation date are not stored in the stream section, but in the container section.

Mainly 3 additions:

  • Duration is read from the format section only if it's not set already (haven't found too much information on this, but based on my personal files the stream metadata seems to be more accurate for the duration)
  • Bit rate from the format section overwrites previously found bit rates (since the stream bit rate really only accounts for video, it makes more sense to use the average overall bit rate)
  • Creation date also overwrites previous creation dates (found no file in my collection where these two were not the same).

I've also added a testcase for mkv metadata parsing and updated the bit rates for the other testcases.

tldr: before this PR any mkv files in my library would use the filesystem modification time, now the embedded creation time is used.

Adds parsing of container header data, which provides correct dates
for Matroska files and also fixes the issue that the bitrate shown
in the UI reflects only the video bitrate (not video + audio bitrate).
@bpatrik
Copy link
Owner

bpatrik commented Sep 3, 2023

Thank you for the clean PR.

This might also help with #682 (need to check)

@bpatrik bpatrik merged commit 1086bc1 into bpatrik:master Sep 3, 2023
@bpatrik bpatrik added this to the Next milestone Sep 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants