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

[Marton Balint]

On Sat, 3 Jul 2021, Tomas Härdin wrote:

> lör 2021-07-03 klockan 15:13 +0200 skrev emcodem at ffastrans.com:
>> 
>> 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.
>
> The patch works (except for the samples not being in FATE)

Actually I found one file where the packetization behaviour changes, 
because after the patch a fake index is generated based of the now 
recognized duration:

samples/MXF/freemxf/freeMXF-mxf-dv-1.mxf

but I guess the file is wrong because clip wrapped UL is used when the 
file seems frame wrapped instead?

>
>> +    // Index Table is special because it might be added manually without
>> +    // partition and we iterate thorugh all instances of them. Also some files
>> +    // use the same Instance UID for different index tables...
>> +    if (type != IndexTableSegment) {
>> +        for (int i = 0; i < mxf->metadata_sets_count; i++) {
>> +            if (!memcmp((*metadata_set)->uid, mxf->metadata_sets[i]->uid, 16) && type == mxf->metadata_sets[i]->type) {
>> +                MXFPartition *old_p = mxf->metadata_sets[i]->partition;
>> +                int old_s = partition_score(old_p);
>> +                int new_s = partition_score(new_p);
>> +                if (old_s > new_s || old_s == new_s && old_p->this_partition > new_p->this_partition) {
>> +                     mxf_free_metadataset(metadata_set, 1);
>> +                     return 0;
>
> This seems asymmetric. Shouldn't this also delete metadata sets that
> metadata_set scores higher than?

I was uneasy about deleting existing metadata sets because I thought 
various MXF structs might already have pointers to existing metadata sets, 
or some allocated members of existing metadata sets. On the other hand, 
when adding a new set, we can free it without the risk of having refences 
to it.

Regards,
Marton