On Tue, 27 Feb 2018, Marton Balint wrote:


On Thu, 22 Feb 2018, Paul B Mahol wrote:

On 2/22/18, Tomas Haerdin <tjop...@acc.umu.se> wrote:
ons 2018-02-21 klockan 23:06 +0100 skrev Marton Balint:
On Wed, 21 Feb 2018, Tomas Haerdin wrote:

> loer 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 <c...@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

The next patch more or less handles this by checking for a system item
key and explicitly setting the offset if that is found. An essence alone
however might not be the first essence, it is also possible that we
already skipped an unknown essence key, or an unknown system item key, so
I decided to keep the code as is if the first recognized essence is not a
system item.

Sounds reasonable I guess. I'm going to reiterate that I consider
continuing to maintain mxfdec is a mistake. A wrapper around
bmxlib/libMXF would be much easier to maintain

YOu are lazy and evil! People please ignore him!

I plan to apply the series soon, unless there are other comments :)

Applied.

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to