[FFmpeg-devel] [PATCH] mxfdec.c: prefer metadata from Footer

[emcodem at ffastrans.com]

Am 2021-06-27 20:12, schrieb Marton Balint:
> On Sun, 27 Jun 2021, Tomas Härdin wrote:

>>> Order of parsing depends on the file, we usually do header, footer 
>>> and then backwards the partitions. So after your patch metadata in 
>>> the second partition will have priority over footer, which is also 
>>> against spec I believe.
>> 
>> I was going to say this is not right because mxf_read_partition_pack()
>> will reverse the order in which partitions are added when parsing
>> backwards, so everything ends up in the correct order. But this 
>> doesn't
>> apply to mxf->metadata_sets, only mxf->partitions..
>> 
>> We've had a similar discussion on here roughly a month ago. There's a
>> bit of finessing around the order in which to prefer any one metadata
>> set. IIRC something like: FooterPartition, then any Complete 
>> non-footer Partition, then the latest OpenPartition
>> 
>> /Tomas

My original patch a month ago was more like this "decision" approach: 
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210524103027.30367-1-emcodem@ffastrans.com/
I basically only wanted to correct a case where the starttc was 0 in 
header but the correct value in footer. Tomas in the chat then came up 
with the idea that it could work for all metadata, so i changed my stuff 
and tested the outcome on multiple thousand files containing about 150 
different flavours. It turned out that in all cases where we had a new 
metadata value, it was always more accurate than before.

"Comaring and Deciding" cannot be done in a very general way, there 
would be some special cases, like the Starttc: SMPTE 377 says whenever 
some value is unknown for the encoder, it shall put the "distinguished 
value" as a placeholder. But for the starttimecode (and many other 
values), it does not define such a distinguished value. Which makes 
sense because it feels kind of impossible that the encoder does not know 
the starttimecode at the start of recording. So one might argue that 
what i am doing is supporting the files of a broke encoder version.
I just fear that this "Compare and decide" approach would end up with 
lots of special cases.

SMPTE 377m 2009, 7.5:
MXF decoders should always use Header Metadata from a Closed Partition. 
When processing files that contain
updated Header Metadata repetitions and when a Closed Partition 
containing Header Metadata is not available,
MXF decoders should use the repetition of a Header Metadata Set with the 
Generation UID value that equals
the This Generation UID Property of the Identification Set at the 
highest index in the Identifications Property of
the Preface Set. All other versions of the same Header Metadata Set 
should be considered outdated.

Most important: i still think this version of the patch is good in 
general. Taking the Values from a possible Header Metadata Repetition in 
a Body IMHO does not directly "violate the rules" and in worst case it 
would spit out the same value that it does now (without this patch). The 
standard seems to say "should" instead of "must".

So now the question for me is if Tomas maybe now prefers my initial 
patch, if that's so, i'd send a separate Patch for the corresponding 
Fate tests. Of course i'd still need someone to put my 2 testfiles to 
the fate-suite/mxf.