[FFmpeg-devel] Fwd: Fate test for mxfdec.c: prefer metadata from Footer -

[emcodem at ffastrans.com]

Am 2023-02-16 18:14, schrieb emcodem at ffastrans.com:
> Am 2023-02-12 01:25, schrieb Tomas Härdin:
>> fre 2023-02-03 klockan 14:54 +0100 skrev emcodem at ffastrans.com:
>>> Am 2021-07-03 15:13, schrieb emcodem at ffastrans.com:
>>> > Am 2021-06-28 21:58, schrieb 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
>>> > > _______________________________________________
>>> > > 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".
>>> >
>>> > Unfortunately the wetransfer link for the fate samples expired, so
>>> > i
>>> > thought it might be a good idea to resend it as link to gdrive:
>>> > https://drive.google.com/file/d/1yXTdS9RfOsduzg49vBLEshdmIzdaVQfd/view?usp=sharing
>>> >
>>> > Also attached the 2 patches: 1 from cus for mxfdec.c and one from
>>> > myself for the corresponding fate samples.
>>> > After applying the mxfdec.c patch, fate will pass with the
>>> > currently
>>> > existing tests but the files in the zip must be uploaded to the
>>> > fate
>>> > suite before applying my corresponding patch of course (otherwise
>>> > the
>>> > files don't exist).
>>> >
>>> > It would be cool if someone found the time and wants to apply this.
>>> >
>>> > Thanks!
>>> > -emcodem
>>> 
>>> Hi Thomas,
>>> sorry i totally forgot about completing the fate tests for this.
>>> The needed file is meanwhile in the official fate-suite repo so this
>>> should pass the automatic fate tests.
>>> 
>>> I did add 2 different tests for the patch that Marton applied that
>>> makes
>>> the demuxer prefer closed complete footer over header.
>>> Both tests use the same file in fate-suite:
>>> omneon_8.3.0.0_xdcam_startc_footer.mxf
>>> This reference file is the shortest completely original example that
>>> i
>>> was able to produce on a real omneon broadcast server. It serves as a
>>> good reference not only for this patch.
>>> 
>>> Test 1) test if TC is preferred from footer.
>>> Test 2) test if it still works as expected when there is no footer.
>>> (more or less inverted test 1) This is done by simulating a
>>> "truncated
>>> file" using subfile protocol i chose the end of file is in the video
>>> data of the last mpeg2 frame because that is what i see mostly with
>>> truncated files in production.
>>> 
>>> Hope this can be applied now as it is.
>> 
>> Meant to reply to this earlier, but yes good samples are crucial for
>> these kinds of tests. Obviously footer metadata will tend to be the
>> most accurate and should be preferred.
>> 
>> It's getting late, hopefully I'll have some time to look at this next
>> week. There are some other things that need doing in mxfdec as well..
>> 
>> /Tomas
> 
> Let me know if you need assistence with any of the "other doings" :D

This is mostly for Tomas Härdin i guess because we have a history with 
this...
Attached a revised version of the fate tests for the "prefer metadata 
from Footer" patch.
Thanks to the guys i met on the Linux days in Chemnitz, we found and 
fixed an issue that would only occur on Mac (needed to changed quoting 
around subfile protocol to fix it). Special thanks to the great Mr. Carl 
Eugen at this point but also to Thilo who actally executed the revison 
and Alex Strasser (sorry that we didnt have the chance to say goodbye 
actually, it was great to meet you all) :D

Please let me know any doubts.

Thanks,
emcodem
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rev2_PATCH-mxf Fate tests for openincomplete and truncated.patch
Type: text/x-diff
Size: 24140 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20230313/1dae4833/attachment.patch>