Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType
@ 2024-02-19 23:42 Marton Balint
  2024-02-19 23:42 ` [FFmpeg-devel] [PATCH 2/2] avformat/mxfdec: track metadata sets by their type Marton Balint
  2024-02-29 22:11 ` [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType Marton Balint
  0 siblings, 2 replies; 3+ messages in thread
From: Marton Balint @ 2024-02-19 23:42 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marton Balint

UUIDs do not have to be unique if their type sets them apart, so avoid using
AnyType, since we are only interested in specific types.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxf.h    |  3 +--
 libavformat/mxfdec.c | 54 ++++++++++++++++----------------------------
 2 files changed, 20 insertions(+), 37 deletions(-)

diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index 7dd1681e60..e16843214c 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -30,8 +30,7 @@
 typedef AVUUID UID;
 
 enum MXFMetadataSetType {
-    AnyType,
-    MaterialPackage,
+    MaterialPackage = 1,
     SourcePackage,
     SourceClip,
     TimecodeComponent,
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 446bcf3276..b7b9207a5b 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -1582,7 +1582,7 @@ static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum MXFMe
         return NULL;
     for (i = mxf->metadata_sets_count - 1; i >= 0; i--) {
         if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
-            (type == AnyType || mxf->metadata_sets[i]->type == type)) {
+            (mxf->metadata_sets[i]->type == type)) {
             return mxf->metadata_sets[i];
         }
     }
@@ -2221,22 +2221,17 @@ static int mxf_add_timecode_metadata(AVDictionary **pm, const char *key, AVTimec
 
 static MXFTimecodeComponent* mxf_resolve_timecode_component(MXFContext *mxf, UID *strong_ref)
 {
-    MXFStructuralComponent *component = NULL;
-    MXFPulldownComponent *pulldown = NULL;
+    MXFTimecodeComponent *timecode;
+    MXFPulldownComponent *pulldown;
 
-    component = mxf_resolve_strong_ref(mxf, strong_ref, AnyType);
-    if (!component)
-        return NULL;
+    timecode = mxf_resolve_strong_ref(mxf, strong_ref, TimecodeComponent);
+    if (timecode)
+        return timecode;
 
-    switch (component->meta.type) {
-    case TimecodeComponent:
-        return (MXFTimecodeComponent*)component;
-    case PulldownComponent: /* timcode component may be located on a pulldown component */
-        pulldown = (MXFPulldownComponent*)component;
+    pulldown = mxf_resolve_strong_ref(mxf, strong_ref, PulldownComponent);
+    if (pulldown)
         return mxf_resolve_strong_ref(mxf, &pulldown->input_segment_ref, TimecodeComponent);
-    default:
-        break;
-    }
+
     return NULL;
 }
 
@@ -2280,14 +2275,20 @@ static MXFDescriptor* mxf_resolve_descriptor(MXFContext *mxf, UID *strong_ref, i
     return NULL;
 }
 
-static MXFStructuralComponent* mxf_resolve_essence_group_choice(MXFContext *mxf, MXFEssenceGroup *essence_group)
+static MXFStructuralComponent* mxf_resolve_sourceclip(MXFContext *mxf, UID *strong_ref)
 {
     MXFStructuralComponent *component = NULL;
     MXFPackage *package = NULL;
     MXFDescriptor *descriptor = NULL;
+    MXFEssenceGroup *essence_group;
     int i;
 
-    if (!essence_group || !essence_group->structural_components_count)
+    component = mxf_resolve_strong_ref(mxf, strong_ref, SourceClip);
+    if (component)
+        return component;
+
+    essence_group = mxf_resolve_strong_ref(mxf, strong_ref, EssenceGroup);
+    if (!essence_group)
         return NULL;
 
     /* essence groups contains multiple representations of the same media,
@@ -2304,24 +2305,7 @@ static MXFStructuralComponent* mxf_resolve_essence_group_choice(MXFContext *mxf,
         if (descriptor)
             return component;
     }
-    return NULL;
-}
-
-static MXFStructuralComponent* mxf_resolve_sourceclip(MXFContext *mxf, UID *strong_ref)
-{
-    MXFStructuralComponent *component = NULL;
 
-    component = mxf_resolve_strong_ref(mxf, strong_ref, AnyType);
-    if (!component)
-        return NULL;
-    switch (component->meta.type) {
-        case SourceClip:
-            return component;
-        case EssenceGroup:
-            return mxf_resolve_essence_group_choice(mxf, (MXFEssenceGroup*) component);
-        default:
-            break;
-    }
     return NULL;
 }
 
@@ -3243,7 +3227,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
     { { 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x04,0x04,0x00 }, mxf_read_partition_pack },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x2f,0x00 }, mxf_read_preface_metadata },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x30,0x00 }, mxf_read_identification_metadata },
-    { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x18,0x00 }, mxf_read_content_storage, 0, AnyType },
+    { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x18,0x00 }, mxf_read_content_storage },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x37,0x00 }, mxf_read_package, sizeof(MXFPackage), SourcePackage },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x36,0x00 }, mxf_read_package, sizeof(MXFPackage), MaterialPackage },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x0f,0x00 }, mxf_read_sequence, sizeof(MXFSequence), Sequence },
@@ -3272,7 +3256,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x04,0x01,0x02,0x02,0x00,0x00 }, mxf_read_cryptographic_context, sizeof(MXFCryptoContext), CryptoContext },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x10,0x01,0x00 }, mxf_read_index_table_segment, sizeof(MXFIndexTableSegment), IndexTableSegment },
     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x23,0x00 }, mxf_read_essence_container_data, sizeof(MXFEssenceContainerData), EssenceContainerData },
-    { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 }, NULL, 0, AnyType },
+    { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 }, NULL },
 };
 
 static int mxf_metadataset_init(MXFMetadataSet *ctx, enum MXFMetadataSetType type, MXFPartition *partition)
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [FFmpeg-devel] [PATCH 2/2] avformat/mxfdec: track metadata sets by their type
  2024-02-19 23:42 [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType Marton Balint
@ 2024-02-19 23:42 ` Marton Balint
  2024-02-29 22:11 ` [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType Marton Balint
  1 sibling, 0 replies; 3+ messages in thread
From: Marton Balint @ 2024-02-19 23:42 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Marton Balint

We typically are only interesed in a single type of metadata set, so it is
better to keep them separated instead of always filtering for them.

Also use av_dynarray_add for increasing their array.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxf.h    |   1 +
 libavformat/mxfdec.c | 128 ++++++++++++++++++++-----------------------
 2 files changed, 60 insertions(+), 69 deletions(-)

diff --git a/libavformat/mxf.h b/libavformat/mxf.h
index e16843214c..673703f6bc 100644
--- a/libavformat/mxf.h
+++ b/libavformat/mxf.h
@@ -55,6 +55,7 @@ enum MXFMetadataSetType {
     GroupOfSoundfieldGroupsLabelSubDescriptor,
     FFV1SubDescriptor,
     JPEG2000SubDescriptor,
+    MetadataSetTypeNB
 };
 
 enum MXFFrameLayout {
diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index b7b9207a5b..49387881b7 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -116,9 +116,13 @@ typedef struct MXFPartition {
 typedef struct MXFMetadataSet {
     UID uid;
     uint64_t partition_score;
-    enum MXFMetadataSetType type;
 } MXFMetadataSet;
 
+typedef struct MXFMetadataSetGroup {
+    MXFMetadataSet **metadata_sets;
+    int metadata_sets_count;
+} MXFMetadataSetGroup;
+
 typedef struct MXFCryptoContext {
     MXFMetadataSet meta;
     UID source_container_ul;
@@ -302,8 +306,7 @@ typedef struct MXFContext {
     int packages_count;
     UID *essence_container_data_refs;
     int essence_container_data_count;
-    MXFMetadataSet **metadata_sets;
-    int metadata_sets_count;
+    MXFMetadataSetGroup metadata_set_groups[MetadataSetTypeNB];
     AVFormatContext *fc;
     struct AVAES *aesc;
     uint8_t *local_tags;
@@ -374,10 +377,10 @@ static const uint8_t mxf_mastering_display_uls[4][16] = {
 
 #define IS_KLV_KEY(x, y) (!memcmp(x, y, sizeof(y)))
 
-static void mxf_free_metadataset(MXFMetadataSet **ctx, int freectx)
+static void mxf_free_metadataset(MXFMetadataSet **ctx, enum MXFMetadataSetType type)
 {
     MXFIndexTableSegment *seg;
-    switch ((*ctx)->type) {
+    switch (type) {
     case Descriptor:
     case MultipleDescriptor:
         av_freep(&((MXFDescriptor *)*ctx)->extradata);
@@ -422,9 +425,7 @@ static void mxf_free_metadataset(MXFMetadataSet **ctx, int freectx)
     default:
         break;
     }
-    if (freectx) {
-        av_freep(ctx);
-    }
+    av_freep(ctx);
 }
 
 static int64_t klv_decode_ber_length(AVIOContext *pb)
@@ -915,34 +916,32 @@ static uint64_t partition_score(MXFPartition *p)
     return (score << 60) | ((uint64_t)p->pack_ofs >> 4);
 }
 
-static int mxf_add_metadata_set(MXFContext *mxf, MXFMetadataSet **metadata_set)
+static int mxf_add_metadata_set(MXFContext *mxf, MXFMetadataSet **metadata_set, enum MXFMetadataSetType type)
 {
-    MXFMetadataSet **tmp;
-    enum MXFMetadataSetType type = (*metadata_set)->type;
+    MXFMetadataSetGroup *mg = &mxf->metadata_set_groups[type];
+    int ret;
 
     // Index Table is special because it might be added manually without
     // partition and we iterate thorugh all instances of them. Also some files
     // use the same Instance UID for different index tables...
     if (type != IndexTableSegment) {
-        for (int i = 0; i < mxf->metadata_sets_count; i++) {
-            if (!memcmp((*metadata_set)->uid, mxf->metadata_sets[i]->uid, 16) && type == mxf->metadata_sets[i]->type) {
-                uint64_t old_s = mxf->metadata_sets[i]->partition_score;
+        for (int i = 0; i < mg->metadata_sets_count; i++) {
+            if (!memcmp((*metadata_set)->uid, mg->metadata_sets[i]->uid, 16)) {
+                uint64_t old_s = mg->metadata_sets[i]->partition_score;
                 uint64_t new_s = (*metadata_set)->partition_score;
                 if (old_s > new_s) {
-                     mxf_free_metadataset(metadata_set, 1);
+                     mxf_free_metadataset(metadata_set, type);
                      return 0;
                 }
             }
         }
     }
-    tmp = av_realloc_array(mxf->metadata_sets, mxf->metadata_sets_count + 1, sizeof(*mxf->metadata_sets));
-    if (!tmp) {
-        mxf_free_metadataset(metadata_set, 1);
-        return AVERROR(ENOMEM);
+
+    ret = av_dynarray_add_nofree(&mg->metadata_sets, &mg->metadata_sets_count, *metadata_set);
+    if (ret < 0) {
+        mxf_free_metadataset(metadata_set, type);
+        return ret;
     }
-    mxf->metadata_sets = tmp;
-    mxf->metadata_sets[mxf->metadata_sets_count] = *metadata_set;
-    mxf->metadata_sets_count++;
     return 0;
 }
 
@@ -1576,16 +1575,14 @@ static const MXFCodecUL *mxf_get_codec_ul(const MXFCodecUL *uls, UID *uid)
 
 static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum MXFMetadataSetType type)
 {
-    int i;
+    MXFMetadataSetGroup *mg = &mxf->metadata_set_groups[type];
 
     if (!strong_ref)
         return NULL;
-    for (i = mxf->metadata_sets_count - 1; i >= 0; i--) {
-        if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
-            (mxf->metadata_sets[i]->type == type)) {
-            return mxf->metadata_sets[i];
-        }
-    }
+    for (int i = mg->metadata_sets_count - 1; i >= 0; i--)
+        if (!memcmp(*strong_ref, mg->metadata_sets[i]->uid, 16))
+            return mg->metadata_sets[i];
+
     return NULL;
 }
 
@@ -1742,12 +1739,10 @@ static int mxf_get_sorted_table_segments(MXFContext *mxf, int *nb_sorted_segment
     int i, j, nb_segments = 0;
     MXFIndexTableSegment **unsorted_segments;
     int last_body_sid = -1, last_index_sid = -1, last_index_start = -1;
+    MXFMetadataSetGroup *mg = &mxf->metadata_set_groups[IndexTableSegment];
 
     /* count number of segments, allocate arrays and copy unsorted segments */
-    for (i = 0; i < mxf->metadata_sets_count; i++)
-        if (mxf->metadata_sets[i]->type == IndexTableSegment)
-            nb_segments++;
-
+    nb_segments = mg->metadata_sets_count;
     if (!nb_segments)
         return AVERROR_INVALIDDATA;
 
@@ -1758,15 +1753,13 @@ static int mxf_get_sorted_table_segments(MXFContext *mxf, int *nb_sorted_segment
         return AVERROR(ENOMEM);
     }
 
-    for (i = nb_segments = 0; i < mxf->metadata_sets_count; i++) {
-        if (mxf->metadata_sets[i]->type == IndexTableSegment) {
-            MXFIndexTableSegment *s = (MXFIndexTableSegment*)mxf->metadata_sets[i];
-            if (s->edit_unit_byte_count || s->nb_index_entries)
-                unsorted_segments[nb_segments++] = s;
-            else
-                av_log(mxf->fc, AV_LOG_WARNING, "IndexSID %i segment at %"PRId64" missing EditUnitByteCount and IndexEntryArray\n",
-                       s->index_sid, s->index_start_position);
-        }
+    for (i = nb_segments = 0; i < mg->metadata_sets_count; i++) {
+        MXFIndexTableSegment *s = (MXFIndexTableSegment*)mg->metadata_sets[i];
+        if (s->edit_unit_byte_count || s->nb_index_entries)
+            unsorted_segments[nb_segments++] = s;
+        else
+            av_log(mxf->fc, AV_LOG_WARNING, "IndexSID %i segment at %"PRId64" missing EditUnitByteCount and IndexEntryArray\n",
+                   s->index_sid, s->index_start_position);
     }
 
     if (!nb_segments) {
@@ -2484,9 +2477,10 @@ static int set_language(AVFormatContext *s, const char *rfc5646, AVDictionary **
 
 static MXFMCASubDescriptor *find_mca_link_id(MXFContext *mxf, enum MXFMetadataSetType type, UID *mca_link_id)
 {
-    for (int k = 0; k < mxf->metadata_sets_count; k++) {
-        MXFMCASubDescriptor *group = (MXFMCASubDescriptor*)mxf->metadata_sets[k];
-        if (group->meta.type == type && !memcmp(&group->mca_link_id, mca_link_id, 16))
+    MXFMetadataSetGroup *mg = &mxf->metadata_set_groups[type];
+    for (int k = 0; k < mg->metadata_sets_count; k++) {
+        MXFMCASubDescriptor *group = (MXFMCASubDescriptor*)mg->metadata_sets[k];
+        if (!memcmp(&group->mca_link_id, mca_link_id, 16))
             return group;
     }
     return NULL;
@@ -2622,7 +2616,6 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
     MXFPackage *material_package = NULL;
     int i, j, k, ret;
 
-    av_log(mxf->fc, AV_LOG_TRACE, "metadata sets count %d\n", mxf->metadata_sets_count);
     /* TODO: handle multiple material packages (OP3x) */
     for (i = 0; i < mxf->packages_count; i++) {
         material_package = mxf_resolve_strong_ref(mxf, &mxf->packages_refs[i], MaterialPackage);
@@ -2807,13 +2800,11 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
         /* HACK: replacing the original key with mxf_encrypted_essence_container
          * is not allowed according to s429-6, try to find correct information anyway */
         if (IS_KLV_KEY(essence_container_ul, mxf_encrypted_essence_container)) {
+            MXFMetadataSetGroup *mg = &mxf->metadata_set_groups[CryptoContext];
             av_log(mxf->fc, AV_LOG_INFO, "broken encrypted mxf file\n");
-            for (k = 0; k < mxf->metadata_sets_count; k++) {
-                MXFMetadataSet *metadata = mxf->metadata_sets[k];
-                if (metadata->type == CryptoContext) {
-                    essence_container_ul = &((MXFCryptoContext *)metadata)->source_container_ul;
-                    break;
-                }
+            if (mg->metadata_sets_count) {
+                MXFMetadataSet *metadata = mg->metadata_sets[0];
+                essence_container_ul = &((MXFCryptoContext *)metadata)->source_container_ul;
             }
         }
 
@@ -3261,7 +3252,6 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
 
 static int mxf_metadataset_init(MXFMetadataSet *ctx, enum MXFMetadataSetType type, MXFPartition *partition)
 {
-    ctx->type = type;
     ctx->partition_score = partition_score(partition);
     switch (type){
     case MultipleDescriptor:
@@ -3300,7 +3290,7 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF
         UID uid = {0};
         if (next < 0 || next > INT64_MAX - size) {
             if (meta) {
-                mxf_free_metadataset(&meta, 1);
+                mxf_free_metadataset(&meta, type);
             }
             return next < 0 ? next : AVERROR_INVALIDDATA;
         }
@@ -3326,7 +3316,7 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF
             avio_read(pb, meta->uid, 16);
         } else if ((ret = read_child(ctx, pb, tag, size, uid, -1)) < 0) {
             if (meta) {
-                mxf_free_metadataset(&meta, 1);
+                mxf_free_metadataset(&meta, type);
             }
             return ret;
         }
@@ -3335,7 +3325,7 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF
          * it extending past the end of the KLV though (zzuf5.mxf). */
         if (avio_tell(pb) > klv_end) {
             if (meta) {
-                mxf_free_metadataset(&meta, 1);
+                mxf_free_metadataset(&meta, type);
             }
 
             av_log(mxf->fc, AV_LOG_ERROR,
@@ -3345,7 +3335,7 @@ static int mxf_read_local_tags(MXFContext *mxf, KLVPacket *klv, MXFMetadataReadF
         } else if (avio_tell(pb) <= next)   /* only seek forward, else this can loop for a long time */
             avio_seek(pb, next, SEEK_SET);
     }
-    return meta ? mxf_add_metadata_set(mxf, &meta) : 0;
+    return meta ? mxf_add_metadata_set(mxf, &meta, type) : 0;
 }
 
 /**
@@ -3601,17 +3591,16 @@ static int mxf_handle_missing_index_segment(MXFContext *mxf, AVStream *st)
     int essence_partition_count = 0;
     int edit_unit_byte_count = 0;
     int i, ret;
+    MXFMetadataSetGroup *mg = &mxf->metadata_set_groups[IndexTableSegment];
 
     if (!track || track->wrapping != ClipWrapped)
         return 0;
 
     /* check if track already has an IndexTableSegment */
-    for (i = 0; i < mxf->metadata_sets_count; i++) {
-        if (mxf->metadata_sets[i]->type == IndexTableSegment) {
-            MXFIndexTableSegment *s = (MXFIndexTableSegment*)mxf->metadata_sets[i];
-            if (s->body_sid == track->body_sid)
-                return 0;
-        }
+    for (i = 0; i < mg->metadata_sets_count; i++) {
+        MXFIndexTableSegment *s = (MXFIndexTableSegment*)mg->metadata_sets[i];
+        if (s->body_sid == track->body_sid)
+            return 0;
     }
 
     /* find the essence partition */
@@ -3643,7 +3632,7 @@ static int mxf_handle_missing_index_segment(MXFContext *mxf, AVStream *st)
     if (!(segment = av_mallocz(sizeof(*segment))))
         return AVERROR(ENOMEM);
 
-    if ((ret = mxf_add_metadata_set(mxf, (MXFMetadataSet**)&segment)))
+    if ((ret = mxf_add_metadata_set(mxf, (MXFMetadataSet**)&segment, IndexTableSegment)))
         return ret;
 
     /* Make sure we have nonzero unique index_sid, body_sid will be ok, because
@@ -3651,7 +3640,6 @@ static int mxf_handle_missing_index_segment(MXFContext *mxf, AVStream *st)
     if (!track->index_sid)
         track->index_sid = track->body_sid;
 
-    segment->meta.type = IndexTableSegment;
     /* stream will be treated as small EditUnitByteCount */
     segment->edit_unit_byte_count = edit_unit_byte_count;
     segment->index_start_position = 0;
@@ -4120,12 +4108,14 @@ static int mxf_read_close(AVFormatContext *s)
     for (i = 0; i < s->nb_streams; i++)
         s->streams[i]->priv_data = NULL;
 
-    for (i = 0; i < mxf->metadata_sets_count; i++) {
-        mxf_free_metadataset(mxf->metadata_sets + i, 1);
+    for (int type = 0; type < FF_ARRAY_ELEMS(mxf->metadata_set_groups); type++) {
+        MXFMetadataSetGroup *mg = &mxf->metadata_set_groups[type];
+        for (i = 0; i < mg->metadata_sets_count; i++)
+            mxf_free_metadataset(mg->metadata_sets + i, type);
+        mg->metadata_sets_count = 0;
+        av_freep(&mg->metadata_sets);
     }
-    mxf->metadata_sets_count = 0;
     av_freep(&mxf->partitions);
-    av_freep(&mxf->metadata_sets);
     av_freep(&mxf->aesc);
     av_freep(&mxf->local_tags);
 
-- 
2.35.3

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType
  2024-02-19 23:42 [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType Marton Balint
  2024-02-19 23:42 ` [FFmpeg-devel] [PATCH 2/2] avformat/mxfdec: track metadata sets by their type Marton Balint
@ 2024-02-29 22:11 ` Marton Balint
  1 sibling, 0 replies; 3+ messages in thread
From: Marton Balint @ 2024-02-29 22:11 UTC (permalink / raw)
  To: FFmpeg development discussions and patches



On Tue, 20 Feb 2024, Marton Balint wrote:

> UUIDs do not have to be unique if their type sets them apart, so avoid using
> AnyType, since we are only interested in specific types.

Ping for the series, will apply in 1-2 days.

Regards,
Marton

>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
> libavformat/mxf.h    |  3 +--
> libavformat/mxfdec.c | 54 ++++++++++++++++----------------------------
> 2 files changed, 20 insertions(+), 37 deletions(-)
>
> diff --git a/libavformat/mxf.h b/libavformat/mxf.h
> index 7dd1681e60..e16843214c 100644
> --- a/libavformat/mxf.h
> +++ b/libavformat/mxf.h
> @@ -30,8 +30,7 @@
> typedef AVUUID UID;
>
> enum MXFMetadataSetType {
> -    AnyType,
> -    MaterialPackage,
> +    MaterialPackage = 1,
>     SourcePackage,
>     SourceClip,
>     TimecodeComponent,
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index 446bcf3276..b7b9207a5b 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -1582,7 +1582,7 @@ static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum MXFMe
>         return NULL;
>     for (i = mxf->metadata_sets_count - 1; i >= 0; i--) {
>         if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
> -            (type == AnyType || mxf->metadata_sets[i]->type == type)) {
> +            (mxf->metadata_sets[i]->type == type)) {
>             return mxf->metadata_sets[i];
>         }
>     }
> @@ -2221,22 +2221,17 @@ static int mxf_add_timecode_metadata(AVDictionary **pm, const char *key, AVTimec
>
> static MXFTimecodeComponent* mxf_resolve_timecode_component(MXFContext *mxf, UID *strong_ref)
> {
> -    MXFStructuralComponent *component = NULL;
> -    MXFPulldownComponent *pulldown = NULL;
> +    MXFTimecodeComponent *timecode;
> +    MXFPulldownComponent *pulldown;
>
> -    component = mxf_resolve_strong_ref(mxf, strong_ref, AnyType);
> -    if (!component)
> -        return NULL;
> +    timecode = mxf_resolve_strong_ref(mxf, strong_ref, TimecodeComponent);
> +    if (timecode)
> +        return timecode;
>
> -    switch (component->meta.type) {
> -    case TimecodeComponent:
> -        return (MXFTimecodeComponent*)component;
> -    case PulldownComponent: /* timcode component may be located on a pulldown component */
> -        pulldown = (MXFPulldownComponent*)component;
> +    pulldown = mxf_resolve_strong_ref(mxf, strong_ref, PulldownComponent);
> +    if (pulldown)
>         return mxf_resolve_strong_ref(mxf, &pulldown->input_segment_ref, TimecodeComponent);
> -    default:
> -        break;
> -    }
> +
>     return NULL;
> }
>
> @@ -2280,14 +2275,20 @@ static MXFDescriptor* mxf_resolve_descriptor(MXFContext *mxf, UID *strong_ref, i
>     return NULL;
> }
>
> -static MXFStructuralComponent* mxf_resolve_essence_group_choice(MXFContext *mxf, MXFEssenceGroup *essence_group)
> +static MXFStructuralComponent* mxf_resolve_sourceclip(MXFContext *mxf, UID *strong_ref)
> {
>     MXFStructuralComponent *component = NULL;
>     MXFPackage *package = NULL;
>     MXFDescriptor *descriptor = NULL;
> +    MXFEssenceGroup *essence_group;
>     int i;
>
> -    if (!essence_group || !essence_group->structural_components_count)
> +    component = mxf_resolve_strong_ref(mxf, strong_ref, SourceClip);
> +    if (component)
> +        return component;
> +
> +    essence_group = mxf_resolve_strong_ref(mxf, strong_ref, EssenceGroup);
> +    if (!essence_group)
>         return NULL;
>
>     /* essence groups contains multiple representations of the same media,
> @@ -2304,24 +2305,7 @@ static MXFStructuralComponent* mxf_resolve_essence_group_choice(MXFContext *mxf,
>         if (descriptor)
>             return component;
>     }
> -    return NULL;
> -}
> -
> -static MXFStructuralComponent* mxf_resolve_sourceclip(MXFContext *mxf, UID *strong_ref)
> -{
> -    MXFStructuralComponent *component = NULL;
>
> -    component = mxf_resolve_strong_ref(mxf, strong_ref, AnyType);
> -    if (!component)
> -        return NULL;
> -    switch (component->meta.type) {
> -        case SourceClip:
> -            return component;
> -        case EssenceGroup:
> -            return mxf_resolve_essence_group_choice(mxf, (MXFEssenceGroup*) component);
> -        default:
> -            break;
> -    }
>     return NULL;
> }
>
> @@ -3243,7 +3227,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x04,0x04,0x00 }, mxf_read_partition_pack },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x2f,0x00 }, mxf_read_preface_metadata },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x30,0x00 }, mxf_read_identification_metadata },
> -    { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x18,0x00 }, mxf_read_content_storage, 0, AnyType },
> +    { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x18,0x00 }, mxf_read_content_storage },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x37,0x00 }, mxf_read_package, sizeof(MXFPackage), SourcePackage },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x36,0x00 }, mxf_read_package, sizeof(MXFPackage), MaterialPackage },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x0f,0x00 }, mxf_read_sequence, sizeof(MXFSequence), Sequence },
> @@ -3272,7 +3256,7 @@ static const MXFMetadataReadTableEntry mxf_metadata_read_table[] = {
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x04,0x01,0x02,0x02,0x00,0x00 }, mxf_read_cryptographic_context, sizeof(MXFCryptoContext), CryptoContext },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x10,0x01,0x00 }, mxf_read_index_table_segment, sizeof(MXFIndexTableSegment), IndexTableSegment },
>     { { 0x06,0x0e,0x2b,0x34,0x02,0x53,0x01,0x01,0x0d,0x01,0x01,0x01,0x01,0x01,0x23,0x00 }, mxf_read_essence_container_data, sizeof(MXFEssenceContainerData), EssenceContainerData },
> -    { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 }, NULL, 0, AnyType },
> +    { { 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00 }, NULL },
> };
>
> static int mxf_metadataset_init(MXFMetadataSet *ctx, enum MXFMetadataSetType type, MXFPartition *partition)
> -- 
> 2.35.3
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-02-29 22:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-19 23:42 [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType Marton Balint
2024-02-19 23:42 ` [FFmpeg-devel] [PATCH 2/2] avformat/mxfdec: track metadata sets by their type Marton Balint
2024-02-29 22:11 ` [FFmpeg-devel] [PATCH 1/2] avformat/mxfdec: remove resolve_strong_ref usage with AnyType Marton Balint

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git