On Wed, 21 Oct 2015 23:45:07 +0200 Tomas Härdin <tomas.har...@codemill.se> wrote:
> On Wed, 2015-10-21 at 18:00 +0200, Alexis Ballier wrote: > > Some files such as those from tickets #2817 & #2776 claim to have > > constant edit unit size but, in fact, have some of them that are > > smaller. This confuses the demuxer that tries to infer the current > > edit unit from the position in the file. By trying to increment the > > current edit unit before rejecting the packet for this reason, we > > try to make it fit into its proper edit unit, which fixes demuxing > > of those files while preserving the check for misprobed OpAtom > > files. Seeking is not accurate but the files provide no way to > > properly find the relevant edit unit. > > > > Fixes tickets #2817 & #2776. > > --- > > libavformat/mxfdec.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c > > index 593604e..526eca6 100644 > > --- a/libavformat/mxfdec.c > > +++ b/libavformat/mxfdec.c > > @@ -2956,6 +2956,18 @@ static int > > mxf_read_packet_old(AVFormatContext *s, AVPacket *pkt) next_ofs = > > mxf_set_current_edit_unit(mxf, klv.offset); > > if (next_ofs >= 0 && next_klv > next_ofs) { > > + /* Samples from tickets #2817 and #2776 claim to > > have > > + * constant edit unit size. However, some of them > > are smaller. > > What does "them" refer to here? The edit units or the KLVs? > > > + * Just after those smaller edit units, > > Right, the edit units. Maybe rework the grammar slightly. > > > + * Just after those smaller edit units, klv.offset > > is still in > > + * the same edit unit according to the > > computations from the > > + * constant edit unit size. If the klv finishes > > after, the next > > + * check would truncate the packet and prevent > > proper demuxing. > > + * Try to increment the current edit unit before > > doing that. */ > > Let's see if I understand this correctly. For say EUBC = 10, there can > still be KLVs that are some size larger than 10, but smaller than > 2*EUBC = 20? So that the next edit unit would extend past the end of > the KLV, and thus be bogus? > > KLV: |header|-------------------|header|--------------| > Edit unit: |0123456789|bogus<10| |0123456789|bgs| > > IIRC with MXF the bogus parts should still count as part of the > essence stream. Maybe I'm missing something. It's simpler than that, and if you don't understand then the comment likely needs improving :) let's see: H = header, V = video, A,B,C = audio tracks, F = fill item mxf file defines a proper edit unit, with EUBC = 10 to be something like: 1234567890 HVVVAFBFCF now, in the samples, in some edit units, video is shorter; mxf spec says it should be padded by fill items, but they're not and look like: 1234567890 HVAFBFCF when continuing to read, we have: 12345678901234567890 HVAFBFCFHVVVAFBFCF | eu 1 || eu 2 | as you can see, 2nd video packet is still in the first edit unit according to EUBC, and extends to next one. that's what the patch is about: try to increment edit unit before rejecting the packet. in 'MXF_DVCAM_not_demuxable.mxf', those smaller video packets seem to correspond to a black frame inserted between two scenes. I've tried hard to get something better, but nothing seemed to work properly; best other option I had was to increment edit unit when seeing a system item, which worked but broke tests and in which I'm not so confident it won't break with other broken files... Alexis. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel