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

Fix MKV metadata loading #18892

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

malaterre
Copy link
Contributor

The correct Mastroska tag used by ffmpeg is 'DATE', not 'date_released'.

Description

I'd like to be able to retrieve both 'title' and 'date' as stored in different containers (eg. avi, mp4 and mkv) in a portable manner.

Motivation and Context

The makes the mkv scrapping consistant with other containers (eg. avi or mp4).

How Has This Been Tested?

Setup kodi to load Video/tag. Settings/Media>'Advanced'>Videos>'Extract thumbnails and video information'

Prepare a folder with:

$ mkdir /tmp/demo
$ cd /tmp/demo
$ wget http://techslides.com/demos/sample-videos/small.mp4
$ ffmpeg -i small.mp4 -map_metadata -1 -metadata title="the mummy" -metadata date="1932" movie.mp4
$ ffmpeg -i small.mp4 -map_metadata -1 -metadata title="the mummy" -metadata date="1932" movie.avi
$ ffmpeg -i small.mp4 -map_metadata -1 -metadata title="the mummy" -metadata date="1932" movie.mkv
$ ffmpeg -i movie.avi movie.avi.mkv
$ ffmpeg -i movie.mp4 movie.mp4.mkv
$ rm small.mp4

All 5 files should load the exact same movie:

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@malaterre
Copy link
Contributor Author

For reference:

@malaterre
Copy link
Contributor Author

@notspiff let me know if you'd prefer to keep some kind of backward compatible code path (instead of a breaking change). Thanks.

@notspiff
Copy link
Contributor

notspiff commented Dec 2, 2020

back then it definitely used date_released, as per https://www.matroska.org/technical/tagging-video-example.html

if ffmpeg has changed this, there is no reason for backwards compatibility, so it's all good.

The correct Mastroska tag used by ffmpeg is 'DATE', not 'date_released'.
@malaterre malaterre marked this pull request as draft December 4, 2020 07:43
@malaterre
Copy link
Contributor Author

@notspiff You may have actually found a regression in ffmpeg. The specification seems to be quite clear about temporal information in MKV container:

Let's see what the ffmpeg developers have to say:

@malaterre
Copy link
Contributor Author

Reported as bug against ffmpeg instead:

@luzpaz
Copy link
Contributor

luzpaz commented Jan 9, 2024

No activity upstream unfortunately.

@luzpaz
Copy link
Contributor

luzpaz commented Mar 2, 2024

This PR should be tagged 'waiting upstream' or something similar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants