diff mbox

[FFmpeg-devel,04/12] avformat/mxfdec: compute both essence_offset and essence_length in mxf_compute_essence_containers

Message ID 20180610103650.10155-4-cus@passwd.hu
State Accepted
Commit 6345770eeb974dcf2197ef145a92051125415884
Headers show

Commit Message

Marton Balint June 10, 2018, 10:36 a.m. UTC 
Also compute the correct essence_offset and essence_length for all clip wrapped
essences.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxfdec.c | 108 +++++++++++++++++++++++++++------------------------
 1 file changed, 57 insertions(+), 51 deletions(-)

Comments

Tomas Härdin June 13, 2018, 3:04 p.m. UTC | #1 
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 <cus@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.

> +}
> +
> +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.. :)

The rest looks OK

/Tomas
diff mbox

Patch

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;
+}
+
+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;
+        } 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);
+
+            /* NOTE: op1a_essence_offset may be less than to klv.offset (C0023S01.mxf)  */
+            if (IS_KLV_KEY(p->first_essence_klv.key, mxf_system_item_key_cp) || IS_KLV_KEY(p->first_essence_klv.key, mxf_system_item_key_gc))
+                p->essence_offset = p->first_essence_klv.offset;
+            else
+                p->essence_offset = op1a_essence_offset;
+
+            /* essence container spans to the next partition */
+            if (x < mxf->partitions_count - 1)
+                p->essence_length = mxf->partitions[x+1].this_partition - p->essence_offset;
+
+            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);
+            }
         }
     }
 }
 
-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;
-}
-
 static int is_pcm(enum AVCodecID codec_id)
 {
     /* we only care about "normal" PCM codecs until we get samples */
@@ -3034,32 +3064,8 @@  static int mxf_read_header(AVFormatContext *s)
                 return AVERROR_INVALIDDATA;
             }
 
-            if (!mxf->current_partition->essence_offset) {
-                /* for OP1a we compute essence_offset
-                 * for OPAtom we point essence_offset after the KL (usually op1a_essence_offset + 20 or 25)
-                 * TODO: for OP1a we could eliminate this entire if statement, always stopping parsing at op1a_essence_offset
-                 *       for OPAtom we still need the actual essence_offset though (the KL's length can vary)
-                 */
-                int64_t op1a_essence_offset =
-                    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);
-
-                if (mxf->op == OPAtom) {
-                    /* point essence_offset to the actual data
-                    * OPAtom has all the essence in one big KLV
-                    */
-                    mxf->current_partition->essence_offset = avio_tell(s->pb);
-                    mxf->current_partition->essence_length = klv.length;
-                } else {
-                    /* NOTE: op1a_essence_offset may be less than to klv.offset (C0023S01.mxf)  */
-                    if (IS_KLV_KEY(klv.key, mxf_system_item_key_cp) || IS_KLV_KEY(klv.key, mxf_system_item_key_gc))
-                        mxf->current_partition->essence_offset = klv.offset;
-                    else
-                        mxf->current_partition->essence_offset = op1a_essence_offset;
-                }
-            }
+            if (!mxf->current_partition->first_essence_klv.offset)
+                mxf->current_partition->first_essence_klv = klv;
 
             if (!essence_offset)
                 essence_offset = klv.offset;
@@ -3098,8 +3104,6 @@  static int mxf_read_header(AVFormatContext *s)
     }
     avio_seek(s->pb, essence_offset, SEEK_SET);
 
-    mxf_compute_essence_containers(mxf);
-
     /* we need to do this before computing the index tables
      * to be able to fill in zero IndexDurations with st->duration */
     if ((ret = mxf_parse_structural_metadata(mxf)) < 0)
@@ -3121,6 +3125,8 @@  static int mxf_read_header(AVFormatContext *s)
         goto fail;
     }
 
+    mxf_compute_essence_containers(s);
+
     mxf_handle_small_eubc(s);
 
     return 0;