[FFmpeg-devel] [Bulk] [PATCH] avformat/mxfdec: dont truncate packets

[Tomas Härdin]

On Thu, 2014-01-30 at 21:08 +0100, Michael Niedermayer wrote:
> On Thu, Jan 30, 2014 at 07:45:19PM +0100, Tomas Härdin wrote:
> > On Thu, 2014-01-30 at 02:39 +0100, Michael Niedermayer wrote:
> > > On Wed, Jan 29, 2014 at 10:20:48AM +0100, Tomas Härdin wrote:
> > > > On Tue, 2013-11-05 at 07:57 +0000, Tim Nicholson wrote:
> > > > > On 24/10/13 20:28, Michael Niedermayer wrote:
> > > > > > Fixes Ticket2776
> > > > > > 
> > > > > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > > > > ---
> > > > > >  libavformat/mxfdec.c |    1 -
> > > > > >  1 file changed, 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > > > > index d0cbeea..59e5c0d 100644
> > > > > > --- a/libavformat/mxfdec.c
> > > > > > +++ b/libavformat/mxfdec.c
> > > > > > @@ -2285,7 +2285,6 @@ static int mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt)
> > > > > >                                        "KLV for edit unit %i extending into "
> > > > > >                                        "next edit unit",
> > > > > >                                        mxf->current_edit_unit);
> > > > > > -                klv.length = next_ofs - avio_tell(s->pb);
> > > > > >              }
> > > > > >  
> > > > > >              /* check for 8 channels AES3 element */
> > > > > > 
> > > > > 
> > > > > Sorry for a very late review, but I have been snowed under (with a
> > > > > project using ffv1 v3 you may be pleased to hear) and after a quick
> > > > > glance it didn't feel right and I wanted to delve further.
> > > > > 
> > > > > We go to the bother of performing a test to trap (an admittdedly
> > > > > unusual) case, and then if it happens do not introduce the safety net to
> > > > > avoid an error. It would be better to work out why a false positive is
> > > > > occuring with the sample and fix that, otherwise we will reintroduce the
> > > > > issue this segment of code was trying to fix.
> > > > > 
> > > > > Does Tomas have a view? the original code to trap the misreading of OP
> > > > > Atom was his.
> > > > 
> > > > Sorry for the extremely late reply. But yes, that piece of code is there
> > > > to deal with certain bad files. I don't remember if we/I have samples
> > > > for it, but I trust my old self put it there for good reasons.
> > > > 
> > > > As for a proper fix, my MXF is a bit rusty. But I think this ties into a
> > > > thing I had in mind a while back, which was to introduce a huge table
> > > > mapping all EssenceContainer ULs to the corresponding wrapping kind
> > > > (relying on byte 15 of the UL is incorrect). I don't think relying on
> > > > OperationalPattern is enough to determine if the file is clip wrapped or
> > > > frame wrapped, and the MXF specs are vague on this matter (as usual).
> > > > Mistaking clip wrapped essence for being frame wrapped is what that
> > > > piece of code is about.
> > > > 
> > > 
> > > > I think I put some random unfinished code on this mailing list for
> > > > generating the EssenceContainer -> WrappingKind tables described above.
> > > > The problem then is that they will grow the binary by about half a
> > > > megabyte..
> > > 
> > > if the tables can be generated and the generation code is smaller
> > > than the tables then the space problem should be avoidable
> > 
> > IIRC the tables are generated from Excel files via a perl script, so
> > that's runtime generation is not going to work
> 
> adding 500kb for this doesnt sound very nice :(
> 
> is there no other way to find the wraping kind,
> without reling on the ULs ?
> or am i missing something here

Sometimes you can just inspect byte 15 of the UL, but not always.
Perhaps some sort of hybrid solution? Like check a table with special
cases first, then byte 15 if no match was found. Then we can add entries
to that table when we run across samples for them. Or maybe the Excel
file can be put through some clever program that figures out the special
cases. The UL's are highly hierarchical, so some kind of decision tree
could also work

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140203/459c70c6/attachment.asc>