On Sun, 2015-10-25 at 21:43 +0100, Tomas Härdin wrote: > On Thu, 2015-10-22 at 19:47 +0200, Alexis Ballier wrote: > > 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. > > Ah, that makes it a lot clearer :) > > > 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... > > Yeah, breaking existing tests is obviously not OK. But increasing > current_edit_unit like that seems a bit too suspect. > > What your patch seems to end up doing with that > max_set_current_edit_unit() call is call mxf_edit_unit_absolute_offset() > like: > > mxf_edit_unit_absolute_offset(mxf, t, mxf->current_edit_unit + 1, NULL, > &next_ofs, 0) > > Maybe you could make use of just that function call (with > mxf->current_edit_unit + *2*), instead of potentially messing > current_edit_unit up for some corner cases..
Any progress on this? More information needed perhaps? /Tomas
signature.asc
Description: This is a digitally signed message part
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel