[FFmpeg-devel] [PATCH] lavf/mxfenc: Bump EDIT_UNITS_PER_BODY

[Tomas Härdin]

mån 2023-02-13 klockan 22:05 +0100 skrev Marton Balint:
> 
> I think we have a pretty good idea that MPEG2 in MXF usually means
> some 
> broadcast realted use, therefore intent of RDD9 compliance by default
> is 
> not insane at all for MPEG2 essence. Can we please keep it as default
> for 
> MPEG2?

If the intent is to follow RDD9 then more things need to be done, among
them erroring out if gop_size > 15. EDIT_UNITS_PER_BODY is also too big
for 24/1.001 Hz essence. Finally the logic around line 2988 is wrong
since it will easily and consistently writes index table segments > 10
seconds:

> if (!mxf->edit_unit_byte_count &&
>     (!mxf->edit_units_count || mxf->edit_units_count >
> EDIT_UNITS_PER_BODY) &&
>     !(ie.flags & 0x33)) { // I-frame, GOP start
> 

This will only work for NTSC because (250+15)/30*1.001 < 8.9s
assuming gop_size <= 15, and it will be > 10s for PAL and 24/1.001 Hz.
The reallocation logic is likely there to compensate for this
wrongness.

The simplest way to remain compliant should therefore be:

* check that gop_size <= 15
* change the above condition to mxf->edit_units_count > 239-gop_size
(maybe -1)

The simplest for the latter is just > 223. If ever the muxer gets a
series of packages that then exceeds the limits set out in RDD 9-2006
then it should complain loudly and terminate so that users don't
accidentally write non-compliant files.

For the allocation stuff, we should make room for 301 EditUnits. If
ever the muxer finds the need to insert a 302nd EditUnit when muxing
MPEG2 then it should error out.

Of course a lot of this could likely be avoided if we just used BMX
instead.

> 
> > 
> > > 
> > > There is also code which allocates the index entries in 
> > > EDIT_UNITS_PER_BODY increments, that probably should be replaced
> > > with
> > > av_dynarray2_add...
> > 
> > That (re)allocation happens at most twice for assuming GOP size <
> > EDIT_UNITS_PER_BODY
> > 
> > The reason I bring this all up is because opening MXF files muxed
> > by
> > lavf over HTTP is slow as hell. This will be true for any other
> > storage
> > where seeking incurs a non-trivial cost. HDDs come to mind. Perhaps
> > that is really an mxfdec problem, but then parsing becomes even
> > hairier
> > than it already is. We'd have to binary search the file when
> > seeking,
> > hoping to find the necessary index table segments on-the-fly..
> 
> A sane muxer duplicates all index table segments in the footer, so a
> smart 
> demuxer can read the whole index from there.

True, though that requires using a dynarray. It also requires smarts
that I don't think mxfdec currently has.

> Yes, mxfdec problem. I have a 
> WIP patch somewhere fixing that.

That would be lovely

/Tomas