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

Tomas Härdin tomas.hardin at codemill.se
Mon Feb 3 22:14:41 CET 2014


On Mon, 2014-02-03 at 16:32 +0100, Michael Niedermayer wrote:
> On Mon, Feb 03, 2014 at 11:15:43AM +0100, Tomas Härdin wrote:
> > 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
> 
> Can you post the 2 lists here or provide a link to them ?

RP224 is the relevant spec (copy attached):
http://www.smpte-ra.org/mdd/RP224v10-publication-20081215.xls

Two example lines demonstrating how byte 15 often distinguishes clip
wrapping and frame wrapping:

583	Leaf	06.0E.2B.34.04.01.01.01	0D.01.03.01.02.02.01.01	MXF-GC Frame-wrapped IEC-DV 525x59.94I 25Mbps	Identifier for MXF Frame-wrapped IEC-DV compressed data of a 525x59.94I source at 25Mbps	SMPTE 383M
584	Leaf	06.0E.2B.34.04.01.01.01	0D.01.03.01.02.02.01.02	MXF-GC Clip-wrapped IEC-DV 525x59.94I 25Mbps	Identifier for MXF Clip-wrapped IEC-DV compressed data of a 525x59.94I source at 25Mbps	SMPTE 383M
                                                             ^^

I also attached a perl script that converts the Excel file to a C header
(rp224.pl), and the converted C header itself (rp224.h).

> maybe someone can figure out how to seperate them with a minimum of
> code. Might even make a good gsoc qualification task

Good idea

> Also can this not be done without using the UL ?

Nope :)

Example: the OPAtom spec mandates clip wrapping (all audio/video in one
huge KLV chunk). But the OP1a spec is vague on the subject - OP1a files
are usually frame wrapped (audio/video interleaved, like sane formats),
but they can also be clip wrapped since the spec doesn't forbid it.
That's how I interpret the specs at least

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RP224v10-publication-20081215.xls
Type: application/vnd.ms-excel
Size: 765952 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140203/94892879/attachment.xls>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rp224.h
Type: text/x-chdr
Size: 646301 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140203/94892879/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rp224.pl
Type: application/x-perl
Size: 778 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140203/94892879/attachment-0001.bin>
-------------- 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/94892879/attachment.asc>


More information about the ffmpeg-devel mailing list