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

[emcodem at ffastrans.com]

Am 2021-06-28 03:00, schrieb Marton Balint:
> On Sun, 27 Jun 2021, emcodem at ffastrans.com wrote:
> 
>> Am 2021-06-27 20:12, schrieb Marton Balint:
>>> 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?
>> 
>> OK, i just had a try on this but honestly i don't know how this could 
>> work without a very deep change of the whole mxfdec.c.
>> The problem is that i cannot just add a field to the struct 
>> MXFMetadataSet as this points to raw data which has been read from the 
>> mxf file. I could add a field but if i initialize the value, i will 
>> automatically destroy the original raw data which was read from the 
>> mxf file.
> 
> See the attached patch, that is what I had in mind. Or is it overkill?
> Can you test it with your dataset and see if it makes any difference?
> 
> Thanks,
> Marton
> _______________________________________________
> 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".

Tested your patch pleasure, thanks for the support! The "score approach" 
seems to work in practice exactly as my previous patch, the only thing i 
fear about is that it is a little harder now to foresee which metadata 
source is taken from a users perspective but i also think it is now more 
compliant than it was before.

Using my test fileset which contains 4.476 mxf files (not all unique, 
maybe half is duplicates and most focus on xdcamhd and D10), we have 90 
differences between ffprobe before your patch and after your patch. All 
of the differences are only in files that have openincomplete header. 
Most of the differences just changes the duration from a guessed one to 
the analyzed one:

All STREAMS (NEW - OLD):
       "duration_ts": 3000,              "duration_ts": 3099,
       "duration": "120.000000",         "duration": "123.960000",
FORMAT      (NEW - OLD):
     "duration": "120.000000",           "duration": "123.969813",
     "bit_rate": "61178142",             "bit_rate": "59219070",

Exception one Op1b self contained file, where the "old" version did not 
spit out a "duration" value at all, so it was not even calculated from 
bitrate, it was just missing in the format section and set to 0 in the 
stream section.
Exception two, there were 4 files (3 were samples from IRT and 1 a real 
world file from old omneon version) where the startc OLD was 0 and the 
new one was the MP starttimecode from MP, so perfect.
So the conclusion is that of course your version had the same effect on 
my testfileset than my patch version, so thats nice.

Also, the FATE samples i shared will still work and can be used for this 
patch.
Attached a patch for adding only the fate samples. Note that these Fate 
tests of course fail when the patch for mxfdec.c is not applied.
https://we.tl/t-MVmyG2mZHq

Thanks a lot!
-emcodem
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mxf-Fate-tests-for-openincomplete-and-truncated.patch
Type: text/x-diff
Size: 22588 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210628/b9cf30f2/attachment.patch>