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

[Tomas Härdin]

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..

/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/20140129/54cb07a6/attachment.asc>