ons 2018-06-13 klockan 21:58 +0200 skrev Marton Balint:
>
> On Wed, 13 Jun 2018, Tomas Härdin wrote:
>
> > sön 2018-06-10 klockan 12:36 +0200 skrev Marton Balint:
> > > Also compute the correct essence_offset and essence_length for all clip
> > > wrapped
> > > essences.
> > >
> > > > Signed-off-by: Marton Balint <c...@passwd.hu>
> > >
> > > ---
> > > libavformat/mxfdec.c | 108
> > > +++++++++++++++++++++++++++------------------------
> > > 1 file changed, 57 insertions(+), 51 deletions(-)
> > >
> > > diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> > > index 1ab34f4873..67b0028e88 100644
> > > --- a/libavformat/mxfdec.c
> > > +++ b/libavformat/mxfdec.c
> > > @@ -98,6 +98,7 @@ typedef struct MXFPartition {
> > > int pack_length;
> > > int64_t pack_ofs; ///< absolute offset of pack in
> > > file, including run-in
> > > int64_t body_offset;
> > > + KLVPacket first_essence_klv;
> > > } MXFPartition;
> > >
> > > typedef struct MXFCryptoContext {
> > > @@ -2782,47 +2783,76 @@ static int
> > > mxf_parse_handle_partition_or_eof(MXFContext *mxf)
> > > return mxf->parsing_backward ? mxf_seek_to_previous_partition(mxf) :
> > > 1;
> > > }
> > >
> > > +static int64_t round_to_kag(int64_t position, int kag_size)
> > > +{
> > > + /* TODO: account for run-in? the spec isn't clear whether KAG should
> > > account for it */
> > > + /* NOTE: kag_size may be any integer between 1 - 2^10 */
> > > + int64_t ret = (position / kag_size) * kag_size;
> > > + return ret == position ? ret : ret + kag_size;
> >
> > This should probably just be
> >
> > return ((position + kag_size - 1) / kag_size) * kag_size;
> >
> > but maybe there's some fine point I'm overlooking. What happens if
> > kag_size is a smallish value like 16?
> >
> > A check for position > INT64_MAX - kag_size might also be appropriate.
>
> Well, this is your code as well :), I just moved it to avoid a forward
> declaration.
>
> >
> > > +}
> > > +
> > > +static MXFWrappingScheme mxf_get_wrapping_by_body_sid(AVFormatContext
> > > *s, int body_sid)
> > > +{
> > > + for (int i = 0; i < s->nb_streams; i++) {
> > > + MXFTrack *track = s->streams[i]->priv_data;
> > > + if (track && track->body_sid == body_sid && track->wrapping !=
> > > UnknownWrapped)
> > > + return track->wrapping;
> > > + }
> > > + return UnknownWrapped;
> > > +}
> > > +
> > > /**
> > > * Figures out the proper offset and length of the essence container in
> > > each partition
> > > */
> > > -static void mxf_compute_essence_containers(MXFContext *mxf)
> > > +static void mxf_compute_essence_containers(AVFormatContext *s)
> > > {
> > > + MXFContext *mxf = s->priv_data;
> > > int x;
> > >
> > > - /* everything is already correct */
> > > - if (mxf->op == OPAtom)
> > > - return;
> > > -
> > > for (x = 0; x < mxf->partitions_count; x++) {
> > > MXFPartition *p = &mxf->partitions[x];
> > > + MXFWrappingScheme wrapping;
> > >
> > > if (!p->body_sid)
> > > continue; /* BodySID == 0 -> no essence */
> > >
> > > - if (x >= mxf->partitions_count - 1)
> > > - break; /* FooterPartition - can't compute length
> > > (and we don't need to) */
> > > + /* for non clip-wrapped essences we compute essence_offset
> > > + * for clip wrapped essences we point essence_offset after the
> > > KL (usually klv.offset + 20 or 25)
> > > + */
> > >
> > > - /* essence container spans to the next partition */
> > > - p->essence_length = mxf->partitions[x+1].this_partition -
> > > p->essence_offset;
> > > + wrapping = (mxf->op == OPAtom) ? ClipWrapped :
> > > mxf_get_wrapping_by_body_sid(s, p->body_sid);
> > >
> > > - if (p->essence_length < 0) {
> > > - /* next ThisPartition < essence_offset */
> > > - p->essence_length = 0;
> > > - av_log(mxf->fc, AV_LOG_ERROR,
> > > - "partition %i: bad ThisPartition = %"PRIX64"\n",
> > > - x+1, mxf->partitions[x+1].this_partition);
> > > + if (wrapping == ClipWrapped) {
> > > + p->essence_offset = p->first_essence_klv.next_klv -
> > > p->first_essence_klv.length;
> > > + p->essence_length = p->first_essence_klv.length;
> >
> > Much nicer
> >
> > > + } else {
> > > + int64_t op1a_essence_offset =
> > > + p->this_partition +
> > > + round_to_kag(p->pack_length, p->kag_size) +
> > > + round_to_kag(p->header_byte_count, p->kag_size) +
> > > + round_to_kag(p->index_byte_count, p->kag_size);
> >
> > Is this really always the case? I guess with OP1a it isn't a huge
> > concern since the demuxer will find the next essence packet anyway. But
> > still..
> >
> > I'm also fairly sure this is my code originally so.. :)
>
> I think this tends to work well, if kag_size is not guessed. However, if
> it _is_ guessed, then there might be problems, and having a 0 kagsize is
> valid (only means an unknown KAG). So I am inclined to simply
> use p->first_essence_klv.offset unconditionally, as you suggested in
>
> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225629.html
Yes, past me seems to have the right idea :)
> See the attached patch.
I think we should still parse kag_size even if we don't end up using it
for much. Especially since there's special cases for Sony files in
there. Did we have a sample for that? I forget.
Are there cases where we might not run across an essence KLV during
scanning but where there would be one anyway? I suspect not, not even
if we're reading an unseekable file
/Tomas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
