On Wed, 10 Apr 2024, Tomas Härdin wrote:
tis 2024-04-09 klockan 22:58 +0200 skrev Marton Balint:
On Tue, 9 Apr 2024, Tomas Härdin wrote:
> mån 2024-04-08 klockan 21:46 +0200 skrev Marton Balint:
> >
> >
> > On Mon, 8 Apr 2024, Tomas Härdin wrote:
> >
> > > tor 2024-04-04 klockan 00:51 +0200 skrev Michael Niedermayer:
> > > > Fixes: Assertion b >=0 failed at libavutil/mathematics.c:62
> > > > Fixes: 67811/clusterfuzz-testcase-minimized-
> > > > ffmpeg_dem_MXF_fuzzer-
> > > > 5108429687422976
> > > >
> > > > Found-by: continuous fuzzing process
> > > > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > Signed-off-by: Michael Niedermayer <mich...@niedermayer.cc>
> > > > ---
> > > > libavformat/mxfdec.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > index 04de4c1d5e3..233d614f783 100644
> > > > --- a/libavformat/mxfdec.c
> > > > +++ b/libavformat/mxfdec.c
> > > > @@ -1264,6 +1264,9 @@ static int
> > > > mxf_read_index_table_segment(void
> > > > *arg, AVIOContext *pb, int tag, int
> > > > case 0x3F0B:
> > > > segment->index_edit_rate.num = avio_rb32(pb);
> > > > segment->index_edit_rate.den = avio_rb32(pb);
> > > > + if (segment->index_edit_rate.num <= 0 ||
> > > > + segment->index_edit_rate.den <= 0)
> > > > + return AVERROR_INVALIDDATA;
> > >
> > > mxf_compute_index_tables() has a check for index_edit_rate that
> > > you
> > > probably want to remove as well. It was introduced in c6fff3d,
> > > but
> > > the
> > > files it supposedly fixes aren't in FATE. We shouldn't
> > > encourage
> > > broken
> > > muxers.
> >
> > I don't quite get what FATE has to do with it. And the samples
> > mentioned
> > in the patch has valid index segment edit rates, only they are
> > different
> > from the track edit rate, and the patch was intended to fix that
> > case.
>
> Then why does it check against 0/0?
Probably to avoid divison by zero.
I think it's safe to say that EditRates with zero in the numerator or
denominator are not allowed. We currently default to 25/1 in this case
for Tracks, but I am skeptical of this since it encourages broken
muxers.
In general, I don't like the idea of rejecting everything which is not
following the standard to the letter. Decoding and demuxing should be
based on the "Robustness principle", as in being liberal in what we accept
and strict in what we generate.
I am also not sure about your reasoning that rejecting files will force
vendors to fix their muxers, because the users will have to pay the price
for this approach. Users may well already have their archives full of
non-compliant files, their camera, phone, whatever is likely out of
warranty/support, so they might not even be in a position to request
anything from vendors.
Sure, I get it, some issues cannot be worked around easily, and I am not
saying that everything must be supported with huge hacks if needed. But an
effort should be made to not break existing real files, and support what
we reasonably can.
As for IndexEditRate, here's what ST 377-1:2019 has to say:
Edit Rate copied from the Essence Tracks of the
Essence Container
[Note: SMPTE RP 210 definition Specifies the
indexing rate in hertz]
It's possible to encode a file that does not specify IndexEditRate, but
this is not allowed since the field is marked Required in Table 26.
mxfdec.c will default to 0/0 since the segment is calloc'd. Michael's
fix won't work if one changes the IndexEditRate local tag in the file
to something else, say FFFF instead of 3F0B.
Yes, you are right about this. To be honest, I'd rather fix the invalid
index edit rate issue by dropping the invalid segments when sorting them.
That should work for both the explicitly and implicitly invalid index edit
rates.
In short, IndexEditRate MUST be set and it MUST equal the EditRate of
the associated Essence Track (confusingly called source_track in the
code). Section 11 is even more explicit:
An Index Table shall be used to index a single Essence Container.
Each Index Table shall index Edit Units
stored Essence of the Essence Container. The Edit Unit rate of an
Index Table is defined by the Edit Rate of the
Essence Tracks of the Package that describes the Essence Container
that the Index Table indexes.
EditRate MAY be different between MaterialPackage and FilePackage
however. This is a consequence of MXF's AAF heritage. MXF is really an
NLE format.
Section 11.6.2 "Look-up Algorithm for Conversion of Index Position to
Stream Offset" is also of relevance. It doesn't make use of
IndexEditRate at all.
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
