-
Notifications
You must be signed in to change notification settings - Fork 649
feat(ffmpeg): Read CICP metadata, and add test #4882
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
Conversation
Add test using Matroska + VP9, to maximize the chance of an ffmpeg build supporting them. They are royalty free, built into ffmpeg and relatively old. Also recognize mkv (Matroska) extension. Signed-off-by: Brecht Van Lommel <[email protected]>
| DATE: "2025/09/13 21:58:33" | ||
| ENCODER: "Lavf61.7.100" | ||
| FramesPerSecond: 24/1 (24) | ||
| SCENE: "Scene" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Lack of an ffmpeg test has masked (until now) that we aren't translating some of the common items into the canonical OIIO names. "DATE" should be "DateTime". "SCENE" probably should be "ImageDescription"?
This is a longstanding issue, you don't need to fix it as part of this PR. But we should come back to it. In fact, I will add it as an issue that feels like an appropriate Dev Days sized task!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This movie file was written by Blender. I would rather expect something should be fixed on the Blender side to follow metadata naming conventions. The code for this is very old, and probably at the time was mainly intended to be consumed by Blender.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, maybe I misunderstood. If those are simply the names Blender chose, then we are reporting it accurately. I think I assumed that these were some kind of standard naming that ffmpeg imposes. Carry on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to explain:
Lots of times, different image file formats use different metadata names for what are conceptually the same thing. Like, what the TIFF spec calls "ImageDescription" is obviously meant to be the same thing that the JPEG spec calls "Caption".
Since a design goal of OIIO is to allow format-agnostic reading, an application should be able to use the same API calls to read from many formats without needing to special case them (as much as possible), so the last thing an app wants to do is somehow know under what metadata name it will find the image description or the capture date, for each and every format it may encounter.
So for the intersection of the most useful and most common metadata items, we have a canonical name that OIIO uses and presents to the user, with each file format handler doing the name translation on the way in and out. Hence, they all translate the date and time data into the same string layout and call it "DateTime", etc.
But that only matters if it's something determined by the format itself. If this is just arbitrary name/value stuff that was put in by the app that wrote the file, we don't try to guess what it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that makes sense. It's indeed just arbitrary metadata.
I was just thinking it would be nice if Blender wrote metadata in such a way that it ends up as e.g. DateTime in OIIO, and then caries over better when converting to other file formats. But that's not OIIO's problem of course.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to add to the confusion, it's called "capTime" in OpenEXR. But OIIO knows how to translate metadata called "DateTime" into "capTime" when it's writing an exr file.
Yes, if you happen to name it DateTime, then if it's read by OIIO and written to other files, it will know that it's the canonical date/time field and will rename it appropriately for each format that has a different specified name for the obviously same meaning of the data.
lgritz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
4945b57
into
AcademySoftwareFoundation:main
…tion#4882) The actual CICP reading code is trivial. Also recognize mkv (Matroska) extension for the test. Add test using Matroska + VP9, to maximize the chance of an ffmpeg build supporting them. They are royalty free, built into ffmpeg and relatively old. Signed-off-by: Brecht Van Lommel <[email protected]>
…tion#4882) The actual CICP reading code is trivial. Also recognize mkv (Matroska) extension for the test. Add test using Matroska + VP9, to maximize the chance of an ffmpeg build supporting them. They are royalty free, built into ffmpeg and relatively old. Signed-off-by: Brecht Van Lommel <[email protected]> Signed-off-by: Zach Lewis <[email protected]>
Description
The actual CICP reading code is trivial.
Also recognize mkv (Matroska) extension for the test.
Tests
Add test using Matroska + VP9, to maximize the chance of an ffmpeg build supporting them. They are royalty free, built into ffmpeg and relatively old.
Checklist:
need to update the documentation, for example if this is a bug fix that
doesn't change the API.)
(adding new test cases if necessary).
corresponding Python bindings (and if altering ImageBufAlgo functions, also
exposed the new functionality as oiiotool options).
already run clang-format before submitting, I definitely will look at the CI
test that runs clang-format and fix anything that it highlights as being
nonconforming.