[FFmpeg-devel] [PATCH 02/10] avformat/mxfdec: fix essence_offset calculation

[Tomas Härdin]

lör 2018-02-17 klockan 22:45 +0100 skrev Marton Balint:
> The reference point for a KAG is the first byte of the key of a Partition Pack.
> 
> Fixes ticket #2817.
> Fixes ticket #5317.
> 
> > Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
>  libavformat/mxfdec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index fcae863ef4..95767ccba4 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -2875,8 +2875,8 @@ static int mxf_read_header(AVFormatContext *s)
>                   *       for OPAtom we still need the actual essence_offset though (the KL's length can vary)
>                   */
>                  int64_t op1a_essence_offset =
> -                    round_to_kag(mxf->current_partition->this_partition +
> -                                 mxf->current_partition->pack_length,       mxf->current_partition->kag_size) +
> +                    mxf->current_partition->this_partition +
> +                    round_to_kag(mxf->current_partition->pack_length,       mxf->current_partition->kag_size) +
>                      round_to_kag(mxf->current_partition->header_byte_count, mxf->current_partition->kag_size) +
>                      round_to_kag(mxf->current_partition->index_byte_count,  mxf->current_partition->kag_size);

This seems like a rather elemental miscalculation, that I wrote even. I
took a look at S377m, it had this to say:

> The first gridline in any partition is the first byte of the key of
> the partition pack that defines that
> partition.

Each partition starts at ThisPartition (plus run-in) so this patch
should be correct. What's perhaps more suspect is the calculation
itself. It should *always* be possible to locate where Op1a essence
starts, by scanning to the first essence KLV. MXF is flexible enough
that having some sketchy calculation for where it *might* be is
probably not correct. One is free to insert any amount of padding

Of course this entire patchset would be unnecessary if we just used
bmxlib instead.

/Tomas