[FFmpeg-devel] [PATCH] avformat/mxfdec: only check index_edit_rate when calculating the index tables

[Marton Balint]

On Fri, 3 May 2024, Tomas Härdin wrote:

> tor 2024-05-02 klockan 23:01 +0200 skrev Marton Balint:
>> 
>> 
>> On Mon, 29 Apr 2024, Tomas Härdin wrote:
>> 
>> > mån 2024-04-15 klockan 21:34 +0200 skrev Marton Balint:
>> > > Commit ed49391961999f028e0bc55767d0eef6eeb15e49 started rejecting
>> > > negative
>> > > index segment edit rates to avoid negative av_rescale parameters.
>> > > There are two
>> > > problems with this:
>> > > 
>> > > 1) there is already a validation for zero (uninitialized) rates
>> > > later
>> > > on
>> > > 2) it rejects files with 0/0 index edit rates which do exist and
>> > > we
>> > > should
>> > > continue to support those
>> > 
>> > There are no such files in FATE last time I checked. At the very
>> > least
>> > we need to know which vendor is producing such broken files.
>> > 
>> > Without tests covering the workflows we want to support, we have no
>> > confidence in refactoriing. This leads to cargo culting. And to be
>> > sure, every workflow we support means a non-trivial amount of work,
>> > especially when it comes to MXF.
>> 
>> I can add a comment to the code or the commit message, we did this
>> plenty 
>> of times in the past. The broken software is Marquise Technologies MT
>> Mediabase 4.7.2 by the way. I can also rework the patch to keep
>> rejecting 
>> negative values and only allow zero, as that is the only thing I
>> *know* to 
>> exist.
>
> We could also tell Marquise Technologies to fix their software. MXF is
> a living ecosystem
>
>> If we add a new FATE file for every fixed file or workflow, the
>> amount of 
>> FATE samples (and the time fate will run) will increase
>> significantly, I 
>> am not sure that is intended.
>
> "Support any old workflow but don't add tests for them" is a ridiculous
> position. It also prevents refactoring. There are real costs to
> supporting workflows that no one uses. Hence why I mention SLAs. The
> only people interested in MXF are professionals.

And what if a home user gets a file from a professional user? Crippling 
professional use cases from ffmpeg, or make professional features a 
payable option - via SLA or otherwise - is something I cannot support.

>
>> In this case, I could only craft an MXF
>> file, because I don't have access to the software.
>
> Yes we can craft arbitrarily broken files. That doesn't help anyone.
> It's just cargo culting.

Cargo culting would be fixing files which do not exists in the wild.
Files with 0/0 index edit rate do exist. So this patch clearly helps 
anybody who is trying to play those.

I can create a fate test for supporting 0/0 index edit rates if that is 
indeed preferred.

Regards,
Marton