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

[Marton Balint]

On Sun, 27 Jun 2021, Tomas Härdin wrote:

> sön 2021-06-27 klockan 12:03 +0200 skrev Marton Balint:
>> 
>> On Sat, 26 Jun 2021, emcodem wrote:
>> 
>> > In case there is a Footer, regarding to SMPTE 377 all versions, the metadata in Footer shall be correct (where in Header it can be incomplete)..
>> > If there is no footer (stream, truncated...) it will still work as usual.
>> > Tested with a huge set of files and compared old/new ffprobes, it will not change lots of metadata, mainly duration and in some cases start timecode.
>> > Without this change, especially Duration would be often inaccurate because it is unknown in header and calculated from bitrate.
>> > 
>> > The new sample files should be added to \fate-suite\mxf, as i do not have an account to upload the files, i shared them on wetransfer:
>> > 
>> > https://we.tl/t-MVmyG2mZHq
>> > 
>> > omneon_6.4.1.0.1_xdcam_truncated.mxf
>> > An original Omneon File from an older Version, file is truncated. It shall prove that Metadata is being parsed even when there is no Footer.
>> > 
>> > omneon_8.3.0.0_xdcam_startc_footer.mxf
>> > An original Omneon File from a recent Version with "better" Metadata in Footer than in Header. I needed to hexedit this file and set the MP and SP start timecode in header to 0.
>> > This test is for proving that metadata from Footer is preferred.
>> > 
>> > ---
>> > libavformat/mxfdec.c                      |   2 +-
>> > tests/fate/mxf.mak                        |  10 +
>> > tests/ref/fate/mxf-probe-xdcamhd-oit      | 442 ++++++++++++++++++++++
>> > tests/ref/fate/mxf-probe-xdcamhd-tcfooter | 442 ++++++++++++++++++++++
>> > 4 files changed, 895 insertions(+), 1 deletion(-)
>> > create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-oit
>> > create mode 100644 tests/ref/fate/mxf-probe-xdcamhd-tcfooter
>> > 
>> > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> > index 7b40076fb4..a7f552c753 100644
>> > --- a/libavformat/mxfdec.c
>> > +++ b/libavformat/mxfdec.c
>> > @@ -1402,7 +1402,7 @@ static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum MXFMe
>> > 
>> >     if (!strong_ref)
>> >         return NULL;
>> > -    for (i = 0; i < mxf->metadata_sets_count; i++) {
>> > +    for (i = mxf->metadata_sets_count-1; i >= 0; i--) {
>> 
>> Do we have to store all metadata in the first place? Why not do the 
>> decision which one is better when adding the metadata set?
>
> This could be done, but it entails rewriting much of the demuxer

Why? I though it is enough if you store the partition number in the 
metadata set, that way you should be able to compare if the existing 
metadata set is better than the current one when adding a new metadata 
set. Or am I missing something?

Thanks,
Marton

>
>> 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
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".