[FFmpeg-devel] [PATCH 04/11] libavformat/mxfdec.c: Try to increment current edit before rejecting a klv that spans onto next edit unit.

[Tomas Härdin]

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.

> +                mxf->current_edit_unit++;
> +                next_ofs = mxf_set_current_edit_unit(mxf, klv.offset);

I feel like this should be more of an explicit check, else maybe it can
miss edit units for some corner cases?

Hm-hm.. I'm noticing I'm very confused. This is very typical of MXF.
I'll take another look at this in the morning.

/Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151021/4a65bc4b/attachment.sig>