feat: FFmpeg additional metadata

[darbyjohnston]

Description

This change adds additional FFmpeg metadata from the video and data streams. These streams can hold the start timecode which is very useful in OpenTimelineIO applications.

I also added av_guess_frame_rate() which might be more robust than just checking stream->avg_frame_rate, but I can also create a separate PR if it is better to break it up.

Tests

It doesn't look like there are any FFmpeg tests in testsuite?

I tested with the DPEL ALab trailer and some Netflix Open Content movies, and was able to see the timecode metadata in the OIIO image attributes.

Checklist:

• [x] I have read the contribution guidelines.
• [x] I have updated the documentation, if applicable.
• [x] I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
• [x] If I added or modified a C++ API call, I have also amended the corresponding Python bindings (and if altering ImageBufAlgo functions, also exposed the new functionality as oiiotool options).
• [x] My code follows the prevailing code style of this project. If I haven't 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.

[lgritz]

Yeah, I think you are correct that there is no current test in the testsuite for video files. That seems like a bad oversight, I would love to see somebody add one just to exercise the basics and get some code coverage there. (Best would be to find/make a movie file or two that is relatively short and low-res so we can check it into the oiio-images repo without it being too huge.)

But short of that, which I don't expect as part of this PR, can you maybe just add a comment to this PR itself showing the before and after output (of, say, oiiotool --info -v mymovie). I'm just not sure I understand what additional metadata is being picked up.

As far as the code and so on, this looks fine to me.

[darbyjohnston]

I might have time to work on a test, what would you consider a reasonable size? For reference I have a couple 320x180 movies checked into a GitHub repo that are about ~1.5MB.

As far as the metadata, I'm actually only interested in the timecode. Thinking about it some more, maybe it would be better to just extract the timecode instead of merging all the metadata from different streams. The timecode seems straightforward, but mixing other metadata might not make sense, especially if there are conflicting names.

Here is the output for one of the Netflix Open Content movies before the change:

D:\Netflix\SolLevante\SolLevante_Animatics_ver1_H264.mov : 1920 x 1080, 3 channel, uint8 FFmpeg movie
    3890 subimages: 1920x1080 [u8,u8,u8]
    channel list: R, G, B
    compatible_brands: "qt  "
    creation_time: "2018-08-24T03:57:08.000000Z"
    FramesPerSecond: 24000/1001 (23.976)
    major_brand: "qt  "
    minor_version: "0"
    ffmpeg:codec_name: "H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10"
    oiio:BitsPerSample: 8
    oiio:Movie: 1
    oiio:subimages: 3890

After the change:

D:\Netflix\SolLevante\SolLevante_Animatics_ver1_H264.mov : 1920 x 1080, 3 channel, uint8 FFmpeg movie
    3890 subimages: 1920x1080 [u8,u8,u8]
    channel list: R, G, B
    compatible_brands: "qt  "
    creation_time: "2018-08-24T03:57:08.000000Z"
    encoder: "H.264"
    FramesPerSecond: 24000/1001 (23.976)
    handler_name: "Core Media Video"
    language: "und"
    major_brand: "qt  "
    minor_version: "0"
    timecode: "00:59:50:00"
    vendor_id: "[0][0][0][0]"
    ffmpeg:codec_name: "H.264 / AVC / MPEG-4 AVC / MPEG-4 part 10"
    oiio:BitsPerSample: 8
    oiio:Movie: 1
    oiio:subimages: 3890

[lgritz]

Whatever you think is best.

Timecode is fine with the generic code, but if you keep some of the other ones, like "encoder" or "handler_name", you might want to prefix them with "ffmpeg:" much like we already did with codec_name.

The general rule is the we use a format-specific prefix for items that could only be reasonably known or meaningful to one format and that should not be propagated from an input of one format to an output of another. The writers all know that any attribute is prefixed by the name of ANOTHER format should be suppressed. In other words, if you extract a frame from an mpeg movie and write it to an exr file, you might want to propagate the timecode, but you sure don't want it to be passing the ffmpeg encoder name.

Also, would you mind adding the timecode to the documentation of the ffmpeg reader in builtinplugins.rst? We may as well document that it will be available since you've made a point of adding it.

[darbyjohnston]

On second thought maybe I will just extract the timecode metadata. I'll make the changes and update the docs.

Should it be "ffmpeg:TimeCode"? It looks like there is also "dpx:TimeCode" and "smpte:TimeCode" from the DPX reader (stored as an int), and "smpte:TimeCode" from the OpenEXR reader (stored as two ints).

[lgritz]

If you can coax it into the SMPTE 2-int encoding of a timecode, then make it smpte:TimeCode.

I guess there's a lingering question (and I'm happy to defer to the wisdom of the OTIO folks on this one) about exactly how we should standardize the communication of timecodes on the app side of the OIIO interface. The fact that it's not quite uniform now is probably a sign of a problem waiting to be fixed.

[lgritz]

I might have time to work on a test, what would you consider a reasonable size? For reference I have a couple 320x180 movies checked into a GitHub repo that are about ~1.5MB.

That's totally fine in the oiio-images repo. There are DSLR raw example files bigger than that.

[JGoldstone]

"The timecode seems straightforward, but …" is a phrase I don't hear very often. Usually I also see something indicating whether or not TC digits are to be interpreted as drop-frame. From a camera original POV, there is often an indicator of the timecode provenance to help relate it to other media with TC values: is it continuously incrementing ('free run') or does it only increment when it's recording? Is it TOD, or is it free-run or rec-run offset from some magic value (I think 01:00:00:00 is common)?

Here's a reasonable first-pass-approximation article on the topic: https://blog.frame.io/2017/07/17/timecode-and-frame-rates/#Jam-sync

I don't think we need to carry provenance info at this point, but a drop-frame boolean seems, at least to me, like a worthy addition.

n.b. before someone says "we never had those problems in the film days" I will say just three words: "three two pulldown".

[darbyjohnston]

I didn't mean that interpreting the timecode is straightforward, but hopefully extracting it from the metadata is (vs. the other metadata in the streams).

I would prefer not to try and convert the timecode and leave that up to OTIO. The OTIO RationalTime class has a number of timecode functions, including one that will take the FFmpeg timecode string and the playback speed/rate and return a time value. I could open an OTIO issue to also add conversions for the DPX/SMPTE timecode formats.

I changed the code to just extract the timecode and also added the attribute to the documentation (maybe ffmpeg:codec_name should also be added?).

[lgritz]

How does ffmpeg give you the timecode? Is it already as a string? Does it consist of the same data fundamentally, or different, than the SMPTE one in OpenEXR?

[lgritz]

Dunno if it helps but when you retrieve an openexr timecode data from OIIO, if you just ask for it in the native type, you get the 2-int set of bitfields, but if you do something like

std::string s = spec.get_string_attribute("smpte:timecode");

you will be given the human-readable kind in the form "{:02d}:{:02d}:{:02d}:{:02d}. Generally speaking, asking for a string attribute explicitly like this will do its best to convert any time to a sensible string.

[darbyjohnston]

Yes, the FFmpeg timecode is a string like: "00:59:50:00". I'm not sure what spec it conforms to, but OTIO seems to interpret it OK, at least with the DPEL and Netflix Open Content movies.

[darbyjohnston]

Ah, that's nice about the string conversion. Hopefully in that case those strings can be used with the existing OTIO conversion function.

[lgritz]

As far as I'm concerned, this PR is ready to merge, though I think there is perhaps some subsequent reconsideration to do about how we want to handle timecodes generally that is beyond my expertise and I hope somebody else would be willing to make a few sensible policy decisions for us.

First, maybe somebody can clarify: Is the 2-int bitfield encoding that OpenEXR uses officially an SMPTE thing, or is that just an OpenEXR-specific compact encoding of the data that SMPTE says should be in a timecode?

I think we have basically three choices (somebody chime in if you see it differently):

1. We could pick a single timecode representation and have all the readers conform, even if not stored in that specific form in the file, so that apps can request the timecode and treat it the same way without regard to file format. (As I said, retrieving as a string will already convert the openexr style to the human readable style, and I'm happy to add other utility functions to OIIO for timecode representation conversion.)

2. We could let every file format that has timecodes store it in its own peculiar "native" style, prefixed by the format name (like "ffmpeg:timecode"). This would mean, though, that apps would need to be somewhat aware of what format they may be reading and know to ask for the specific correct name and know what to do with it when they get it. Or, we could add a utility function that would look for any of the timecode names and convert to any of the timecode representations, to try to tame the chaos.

3. Somewhere in between, maybe we would see that the different file formats supporting timecodes tend to cluster into just 2-3 "timecode styles", and we should pick a name for each style that's not tied to the file format, unless a file format has a truly unique way to represent them.

Or anything else you think would be best. I'm happy to adapt OIIO's handling of timecodes to whatever the people who consume them would find the best approach.

[darbyjohnston]

I'm not an expert, but as a user of the library I would prefer either #1 or #3. I can imagine #1 might not be practical so #3 seems like a reasonable compromise.

[lgritz]

So where are we with this PR now, @darbyjohnston? Do you think this is sufficient for now, or were there more changes you wanted?

I think it's ok to merge this (pending the one suggestion I made) and then come back in a subsequent PR clean up the timecode nomenclature across all the formats. Is there any timecode format that's common enough (and common-sense enough) to just be called TimeCode instead of <formatname>:TimeCode? Is ffmpeg:TimeCode in the same format as the dpx:TimeCode and both should be renamed something more generic?

[darbyjohnston]

Thanks, I think this is fine for now, I changed the case of the metadata name as requested.

The FFmpeg file libavutil/timecode.h does mention SMPTE 12M which is the same format as DPX. However I don't have a copy of the SMPTE 12M doc, so I'm not sure about the different encodings (int vs. string).

[lgritz]

OK, I merged what you had and will backport to the release branch.

Just for the record, I think we have two action items for somebody to tackle who is inclined:

1. It seems that we have nothing in the testsuite now that exercises the ffmpeg reader at all, and that feels like something that needs to be fixed (preferably with a test that requires a small-ish movie file).
2. Somebody who really has knowledge of and a stake in timecode data should decide and implement a naming and data representation policy so we don't have every file format with timecodes storing a file-specific representation under a file-specific name.