[Ffmpeg-devel] [PATCH] MXF tag parser

Reimar Döffinger Reimar.Doeffinger
Wed Aug 2 14:39:44 CEST 2006


Hello,
the attached patch implements a special mxf tag parsing function to
avoid code duplication.
It also has the side effect of doing more checks and playing my
problematic (i.e. non-conformant) files while also showing a warning
about the problem.
It might be possible to optimize a bit further, esp. some names, but I
think it is already quite helpful, esp. for avoiding copy-and-paste when
adding support for more stuff.
It also changes quite a few types from int to uint32_t, because
1) I think it's more correct
2) checks like "if (tmp_u32 >= UINT_MAX / sizeof(UID))" (before
malloc(tmp_u32 * sizeof(UID)) ) are not correct for signed types AFAICT.

Greetings,
Reimar D?ffinger
-------------- next part --------------
Index: libavformat/mxf.c
===================================================================
--- libavformat/mxf.c	(revision 5888)
+++ libavformat/mxf.c	(working copy)
@@ -64,7 +64,7 @@
     UID data_definition_ul;
     int64_t duration;
     int64_t start_position;
-    int source_track_id;
+    uint32_t source_track_id;
     enum MXFStructuralComponentType type;
 } MXFStructuralComponent;
 
@@ -73,7 +73,7 @@
     UID data_definition_ul;
     MXFStructuralComponent **structural_components;
     UID *structural_components_refs;
-    int structural_components_count;
+    uint32_t structural_components_count;
     int64_t duration;
 } MXFSequence;
 
@@ -81,7 +81,7 @@
     UID uid;
     MXFSequence *sequence; /* mandatory, and only one */
     UID sequence_ref;
-    int track_id;
+    uint32_t track_id;
     uint8_t track_number[4];
     AVRational edit_rate;
 } MXFTrack;
@@ -92,14 +92,14 @@
     UID essence_codec_ul;
     AVRational sample_rate;
     AVRational aspect_ratio;
-    int width;
-    int height;
-    int channels;
-    int bits_per_sample;
+    uint32_t width;
+    uint32_t height;
+    uint32_t channels;
+    uint32_t bits_per_sample;
     struct MXFDescriptor **sub_descriptors;
     UID *sub_descriptors_refs;
     int sub_descriptors_count;
-    int linked_track_id;
+    uint32_t linked_track_id;
 } MXFDescriptor;
 
 typedef struct MXFPackage {
@@ -107,7 +107,7 @@
     UID package_uid;
     MXFTrack **tracks;
     UID *tracks_refs;
-    int tracks_count;
+    uint32_t tracks_count;
     MXFDescriptor *descriptor; /* only one */
     UID descriptor_ref;
     enum MXFPackageType type;
@@ -121,12 +121,12 @@
 typedef struct MXFContext {
     MXFPackage **packages;
     UID *packages_refs;
-    int packages_count;
+    uint32_t packages_count;
     MXFEssenceContainerData **essence_container_data_sets;
     UID *essence_container_data_sets_refs;
-    int essence_container_data_sets_count;
+    uint32_t essence_container_data_sets_count;
     UID *essence_containers_uls; /* Universal Labels SMPTE RP224 */
-    int essence_containers_uls_count;
+    uint32_t essence_containers_uls_count;
     UID operational_pattern_ul;
     UID content_storage_uid;
     AVFormatContext *fc;
@@ -148,6 +148,22 @@
     int (*read)(MXFContext *mxf, KLVPacket *klv);
 } MXFMetadataReadTableEntry;
 
+typedef enum {
+    MXF_TT_BE32, MXF_TT_BE64, MXF_TT_FRAC,
+    MXF_TT_UID, MXF_TT_ID, MXF_TT_BUF4,
+    MXF_TT_UID_ARRAY, MXF_TT_FUNC,
+    MXF_TT_LAST
+} MXFTagType;
+
+typedef int (*MXFTagFunc)(ByteIOContext *, int, int, void *);
+
+typedef struct {
+    int16_t tag;
+    MXFTagType type;
+    void *dest;
+    void *dest2;
+} MXFTagDesc;
+
 /* partial keys to match */
 static const uint8_t mxf_header_partition_pack_key[]       = { 0x06,0x0e,0x2b,0x34,0x02,0x05,0x01,0x01,0x0d,0x01,0x02,0x01,0x01,0x02 };
 static const uint8_t mxf_essence_element_key[]             = { 0x06,0x0e,0x2b,0x34,0x01,0x02,0x01,0x01,0x0d,0x01,0x03,0x01 };
@@ -219,113 +235,144 @@
     return AVERROR_IO;
 }
 
-static int mxf_read_metadata_preface(MXFContext *mxf, KLVPacket *klv)
-{
-    ByteIOContext *pb = &mxf->fc->pb;
-    int bytes_read = 0;
-
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* SMPTE 336M Table 8 KLV specified length, 0x53 */
-
-        switch (tag) {
-        case 0x3B03:
-            get_buffer(pb, mxf->content_storage_uid, 16);
-            break;
-        case 0x3B09:
-            get_buffer(pb, mxf->operational_pattern_ul, 16);
-            break;
-        case 0x3B0A:
-            mxf->essence_containers_uls_count = get_be32(pb);
-            if (mxf->essence_containers_uls_count >= UINT_MAX / sizeof(UID))
-                return -1;
-            mxf->essence_containers_uls = av_malloc(mxf->essence_containers_uls_count * sizeof(UID));
-            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
-            get_buffer(pb, mxf->essence_containers_uls, mxf->essence_containers_uls_count * sizeof(UID));
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
+/**
+ * \brief allocates an array with the same number of elements as another one
+ * \param count number of element, set to 0 if allocation fails
+ * \param array existing array, will be freed and set to NULL if allocation fails
+ * \param newarray array to allocate
+ * \param elsize size of one element in new array
+ * \return 0 if allocation failed, 1 otherwise
+ */
+static int alloc_associated_array(uint32_t *count, void **array, void **newarray, uint32_t elsize) {
+    if (*count) {
+        if (*count >= UINT_MAX / elsize) goto fail;
+        *newarray = av_mallocz(*count * elsize);
+        if (!*newarray) goto fail;
     }
+    return 1;
+fail:
+    *newarray = NULL;
+    av_freep(array);
+    *count = 0;
     return 0;
 }
 
-static int mxf_read_metadata_content_storage(MXFContext *mxf, KLVPacket *klv)
-{
+static void mxf_read_tags(MXFContext *mxf, const MXFTagDesc *tags, int64_t maxlen) {
     ByteIOContext *pb = &mxf->fc->pb;
-    int bytes_read = 0;
-
-    while (bytes_read < klv->length) {
+    while (maxlen > 0) {
+        uint32_t tmp_u32 = 0;
+        UID *tmp_puid = NULL;
+        MXFTagFunc tmp_func;
+        int bad = 0;
+        MXFTagDesc *tagd;
         int tag = get_be16(pb);
         int size = get_be16(pb); /* SMPTE 336M Table 8 KLV specified length, 0x53 */
-
-        dprintf("tag 0x%04X, size %d\n", tag, size);
-        switch (tag) {
-        case 0x1901:
-            mxf->packages_count = get_be32(pb);
-            if (mxf->packages_count >= UINT_MAX / sizeof(UID) ||
-                mxf->packages_count >= UINT_MAX / sizeof(*mxf->packages))
-                return -1;
-            mxf->packages_refs = av_malloc(mxf->packages_count * sizeof(UID));
-            mxf->packages = av_mallocz(mxf->packages_count * sizeof(*mxf->packages));
-            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
-            get_buffer(pb, mxf->packages_refs, mxf->packages_count * sizeof(UID));
-            break;
-        case 0x1902:
-            mxf->essence_container_data_sets_count = get_be32(pb);
-            if (mxf->essence_container_data_sets_count >= UINT_MAX / sizeof(UID) ||
-                mxf->essence_container_data_sets_count >= UINT_MAX / sizeof(*mxf->essence_container_data_sets))
-                return -1;
-            mxf->essence_container_data_sets_refs = av_malloc(mxf->essence_container_data_sets_count * sizeof(UID));
-            mxf->essence_container_data_sets = av_mallocz(mxf->essence_container_data_sets_count * sizeof(*mxf->essence_container_data_sets));
-            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
-            get_buffer(pb, mxf->essence_container_data_sets_refs, mxf->essence_container_data_sets_count * sizeof(UID));
-            break;
-        default:
-            url_fskip(pb, size);
+        int skip = 0;
+        maxlen -= 4;
+        for (tagd = tags; tagd->type != MXF_TT_LAST; tagd++) {
+            if (tagd->tag == tag)
+                break;
         }
-        bytes_read += size + 4;
+        switch (tagd->type) {
+            case MXF_TT_BE32:
+                if (size != 4) {skip = size; break;}
+                *(uint32_t *)tagd->dest = get_be32(pb);
+                break;
+            case MXF_TT_BE64:
+                if (size != 8) {skip = size; break;}
+                *(uint64_t *)tagd->dest = get_be64(pb);
+                break;
+            case MXF_TT_FRAC:
+                if (size != 8) {skip = size; break;}
+                ((AVRational *)tagd->dest)->num = get_be32(pb);
+                ((AVRational *)tagd->dest)->den = get_be32(pb);
+                break;
+            case MXF_TT_UID:
+                if (size != 16) {skip = size; break;}
+                get_buffer(pb, tagd->dest, 16);
+                break;
+            case MXF_TT_ID:
+                if (size != 32) {skip = size; break;}
+                url_fskip(pb, 16);
+                get_buffer(pb, tagd->dest, 16);
+                break;
+            case MXF_TT_BUF4:
+                if (size != 4) {skip = size; break;}
+                get_buffer(pb, tagd->dest, 4);
+                break;
+            case MXF_TT_UID_ARRAY:
+                if (size < 8) {skip = size; break;}
+                tmp_u32 = get_be32(pb);
+                if (tmp_u32 >= UINT_MAX / sizeof(UID)) {skip = size - 4; break;}
+                if (get_be32(pb) != sizeof(UID)) {skip = size - 8; break;}
+                if (tmp_u32 * sizeof(UID) != size - 8) {skip = size - 8; break;}
+                if (tmp_u32) {
+                    tmp_puid = av_malloc(tmp_u32 * sizeof(UID));
+                    get_buffer(pb, tmp_puid, tmp_u32 * sizeof(UID));
+                }
+                *(uint32_t *)tagd->dest = tmp_u32;
+                *(UID **)tagd->dest2 = tmp_puid;
+                break;
+            case MXF_TT_FUNC:
+                tmp_func = (MXFTagFunc)tagd->dest;
+                skip = tmp_func(pb, tag, size, tagd->dest2);
+                break;
+            default:
+                url_fskip(pb, size);
+                break;
+        }
+        if (!size || skip) {
+            av_log(mxf->fc, AV_LOG_INFO, "bad tag %x of size %i, skipping %i\n", tag, size, skip);
+            url_fskip(pb, skip);
+        }
+        maxlen -= size;
     }
+}
+
+static int mxf_read_metadata_preface(MXFContext *mxf, KLVPacket *klv)
+{
+    MXFTagDesc tags[] = {
+        {0x3B03, MXF_TT_UID,       &mxf->content_storage_uid, NULL},
+        {0x3B09, MXF_TT_UID,       &mxf->operational_pattern_ul, NULL},
+        {0x3B0A, MXF_TT_UID_ARRAY, &mxf->essence_containers_uls_count, &mxf->essence_containers_uls},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
+    mxf_read_tags(mxf, tags, klv->length);
     return 0;
 }
 
+static int mxf_read_metadata_content_storage(MXFContext *mxf, KLVPacket *klv)
+{
+    int res;
+    MXFTagDesc tags[] = {
+        {0x1901, MXF_TT_UID_ARRAY, &mxf->packages_count, &mxf->packages_refs},
+        {0x1902, MXF_TT_UID_ARRAY, &mxf->essence_container_data_sets_count, &mxf->essence_container_data_sets_refs},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
+    mxf_read_tags(mxf, tags, klv->length);
+    res = alloc_associated_array(&mxf->packages_count, &mxf->packages_refs,
+            &mxf->packages, sizeof(*mxf->packages));
+    res &= alloc_associated_array(&mxf->essence_container_data_sets_count,
+             &mxf->essence_container_data_sets_refs, &mxf->essence_container_data_sets,
+             sizeof(*mxf->essence_container_data_sets));
+    return res ? 0 : -1;
+}
+
 static int mxf_read_metadata_source_clip(MXFContext *mxf, KLVPacket *klv)
 {
-    ByteIOContext *pb = &mxf->fc->pb;
     MXFStructuralComponent *source_clip = av_mallocz(sizeof(*source_clip));
-    int bytes_read = 0;
+    MXFTagDesc tags[] = {
+        {0x3C0A, MXF_TT_UID,       &source_clip->uid, NULL},
+        {0x0202, MXF_TT_BE64,      &source_clip->duration, NULL},
+        {0x1201, MXF_TT_BE64,      &source_clip->start_position, NULL},
+        {0x1101, MXF_TT_ID,        &source_clip->source_package_uid, NULL},
+        {0x1102, MXF_TT_BE32,      &source_clip->source_track_id, NULL},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
     int i, j, k;
 
     source_clip->type = SourceClip;
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* SMPTE 336M Table 8 KLV specified length, 0x53 */
-
-        dprintf("tag 0x%04X, size %d\n", tag, size);
-        switch (tag) {
-        case 0x3C0A:
-            get_buffer(pb, source_clip->uid, 16);
-            break;
-        case 0x0202:
-            source_clip->duration = get_be64(pb);
-            break;
-        case 0x1201:
-            source_clip->start_position = get_be64(pb);
-            break;
-        case 0x1101:
-            /* UMID, only get last 16 bytes */
-            url_fskip(pb, 16);
-            get_buffer(pb, source_clip->source_package_uid, 16);
-            break;
-        case 0x1102:
-            source_clip->source_track_id = get_be32(pb);
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
-    }
+    mxf_read_tags(mxf, tags, klv->length);
     for (i = 0; i < mxf->packages_count; i++) {
         if (mxf->packages[i]) {
             for (j = 0; j < mxf->packages[i]->tracks_count; j++) {
@@ -345,35 +392,21 @@
 
 static int mxf_read_metadata_material_package(MXFContext *mxf, KLVPacket *klv)
 {
-    ByteIOContext *pb = &mxf->fc->pb;
     MXFPackage *package = av_mallocz(sizeof(*package));
-    int bytes_read = 0;
+    MXFTagDesc tags[] = {
+        {0x3C0A, MXF_TT_UID,       &package->uid, NULL},
+        {0x4403, MXF_TT_UID_ARRAY, &package->tracks_count, &package->tracks_refs},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
     int i;
 
     package->type = MaterialPackage;
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* KLV specified by 0x53 */
+    mxf_read_tags(mxf, tags, klv->length);
 
-        switch (tag) {
-        case 0x3C0A:
-            get_buffer(pb, package->uid, 16);
-            break;
-        case 0x4403:
-            package->tracks_count = get_be32(pb);
-            if (package->tracks_count >= UINT_MAX / sizeof(UID) ||
-                package->tracks_count >= UINT_MAX / sizeof(*package->tracks))
-                return -1;
-            package->tracks_refs = av_malloc(package->tracks_count * sizeof(UID));
-            package->tracks = av_mallocz(package->tracks_count * sizeof(*package->tracks));
-            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
-            get_buffer(pb, package->tracks_refs, package->tracks_count * sizeof(UID));
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
-    }
+    if (!alloc_associated_array(&package->tracks_count, &package->tracks_refs,
+           &package->tracks, sizeof(*package->tracks)))
+        return -1;
+
     for (i = 0; i < mxf->packages_count; i++) {
         if (!memcmp(mxf->packages_refs[i], package->uid, 16)) {
             mxf->packages[i] = package;
@@ -385,38 +418,21 @@
 
 static int mxf_read_metadata_track(MXFContext *mxf, KLVPacket *klv)
 {
-    ByteIOContext *pb = &mxf->fc->pb;
     MXFTrack *track = av_mallocz(sizeof(*track));
-    int bytes_read = 0;
+    AVRational edit = {0, 0};
+    MXFTagDesc tags[] = {
+        {0x3C0A, MXF_TT_UID,       &track->uid, NULL},
+        {0x4801, MXF_TT_BE32,      &track->track_id, NULL},
+        {0x4804, MXF_TT_BUF4,      &track->track_number, NULL},
+        {0x4B01, MXF_TT_FRAC,      &edit, NULL},
+        {0x4803, MXF_TT_UID,       &track->sequence_ref, NULL},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
     int i, j;
 
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* KLV specified by 0x53 */
-
-        dprintf("tag 0x%04X, size %d\n", tag, size);
-        switch (tag) {
-        case 0x3C0A:
-            get_buffer(pb, track->uid, 16);
-            break;
-        case 0x4801:
-            track->track_id = get_be32(pb);
-            break;
-        case 0x4804:
-            get_buffer(pb, track->track_number, 4);
-            break;
-        case 0x4B01:
-            track->edit_rate.den = get_be32(pb);
-            track->edit_rate.num = get_be32(pb);
-            break;
-        case 0x4803:
-            get_buffer(pb, track->sequence_ref, 16);
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
-    }
+    mxf_read_tags(mxf, tags, klv->length);
+    track->edit_rate.den = edit.num;
+    track->edit_rate.num = edit.den;
     for (i = 0; i < mxf->packages_count; i++) {
         if (mxf->packages[i]) {
             for (j = 0; j < mxf->packages[i]->tracks_count; j++) {
@@ -432,41 +448,22 @@
 
 static int mxf_read_metadata_sequence(MXFContext *mxf, KLVPacket *klv)
 {
-    ByteIOContext *pb = &mxf->fc->pb;
     MXFSequence *sequence = av_mallocz(sizeof(*sequence));
-    int bytes_read = 0;
+    MXFTagDesc tags[] = {
+        {0x3C0A, MXF_TT_UID,       &sequence->uid, NULL},
+        {0x0202, MXF_TT_BE64,      &sequence->duration, NULL},
+        {0x0201, MXF_TT_UID,       &sequence->data_definition_ul, NULL},
+        {0x1001, MXF_TT_UID_ARRAY, &sequence->structural_components_count, &sequence->structural_components_refs},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
     int i, j;
 
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* KLV specified by 0x53 */
+    mxf_read_tags(mxf, tags, klv->length);
+    if (!alloc_associated_array(&sequence->structural_components_count,
+           &sequence->structural_components_refs, &sequence->structural_components,
+           sizeof(*sequence->structural_components)))
+        return -1;
 
-        dprintf("tag 0x%04X, size %d\n", tag, size);
-        switch (tag) {
-        case 0x3C0A:
-            get_buffer(pb, sequence->uid, 16);
-            break;
-        case 0x0202:
-            sequence->duration = get_be64(pb);
-            break;
-        case 0x0201:
-            get_buffer(pb, sequence->data_definition_ul, 16);
-            break;
-        case 0x1001:
-            sequence->structural_components_count = get_be32(pb);
-            if (sequence->structural_components_count >= UINT_MAX / sizeof(UID) ||
-                sequence->structural_components_count >= UINT_MAX / sizeof(*sequence->structural_components))
-                return -1;
-            sequence->structural_components_refs = av_malloc(sequence->structural_components_count * sizeof(UID));
-            sequence->structural_components = av_mallocz(sequence->structural_components_count * sizeof(*sequence->structural_components));
-            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
-            get_buffer(pb, sequence->structural_components_refs, sequence->structural_components_count * sizeof(UID));
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
-    }
     for (i = 0; i < mxf->packages_count; i++) {
         if (mxf->packages[i]) {
             for (j = 0; j < mxf->packages[i]->tracks_count; j++) {
@@ -484,44 +481,22 @@
 
 static int mxf_read_metadata_source_package(MXFContext *mxf, KLVPacket *klv)
 {
-    ByteIOContext *pb = &mxf->fc->pb;
     MXFPackage *package = av_mallocz(sizeof(*package));
-    int bytes_read = 0;
+    MXFTagDesc tags[] = {
+        {0x3C0A, MXF_TT_UID,       &package->uid, NULL},
+        {0x4403, MXF_TT_UID_ARRAY, &package->tracks_count, &package->tracks_refs},
+        {0x4401, MXF_TT_ID,        &package->package_uid, NULL},
+        {0x4701, MXF_TT_UID,       &package->descriptor_ref, NULL},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
     int i;
 
     package->type = SourcePackage;
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* KLV specified by 0x53 */
+    mxf_read_tags(mxf, tags, klv->length);
+    if (!alloc_associated_array(&package->tracks_count, &package->tracks_refs,
+           &package->tracks, sizeof(*package->tracks)))
+        return -1;
 
-        dprintf("tag 0x%04X, size %d\n", tag, size);
-        switch (tag) {
-        case 0x3C0A:
-            get_buffer(pb, package->uid, 16);
-            break;
-        case 0x4403:
-            package->tracks_count = get_be32(pb);
-            if (package->tracks_count >= UINT_MAX / sizeof(UID) ||
-                package->tracks_count >= UINT_MAX / sizeof(*package->tracks))
-                return -1;
-            package->tracks_refs = av_malloc(package->tracks_count * sizeof(UID));
-            package->tracks = av_mallocz(package->tracks_count * sizeof(*package->tracks));
-            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
-            get_buffer(pb, package->tracks_refs, package->tracks_count * sizeof(UID));
-            break;
-        case 0x4401:
-            /* UMID, only get last 16 bytes */
-            url_fskip(pb, 16);
-            get_buffer(pb, package->package_uid, 16);
-            break;
-        case 0x4701:
-            get_buffer(pb, package->descriptor_ref, 16);
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
-    }
     for (i = 0; i < mxf->packages_count; i++) {
         if (!memcmp(mxf->packages_refs[i], package->uid, 16)) {
             mxf->packages[i] = package;
@@ -533,35 +508,20 @@
 
 static int mxf_read_metadata_multiple_descriptor(MXFContext *mxf, KLVPacket *klv)
 {
-    ByteIOContext *pb = &mxf->fc->pb;
     MXFDescriptor *descriptor = av_mallocz(sizeof(*descriptor));
-    int bytes_read = 0;
+    MXFTagDesc tags[] = {
+        {0x3C0A, MXF_TT_UID,       &descriptor->uid, NULL},
+        {0x3F01, MXF_TT_UID_ARRAY, &descriptor->sub_descriptors_count, &descriptor->sub_descriptors_refs},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
     int i;
 
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* KLV specified by 0x53 */
+    mxf_read_tags(mxf, tags, klv->length);
+    if (!alloc_associated_array(&descriptor->sub_descriptors_count,
+           &descriptor->sub_descriptors_refs,
+           &descriptor->sub_descriptors, sizeof(*descriptor->sub_descriptors)))
+        return -1;
 
-        dprintf("tag 0x%04X, size %d\n", tag, size);
-        switch (tag) {
-        case 0x3C0A:
-            get_buffer(pb, descriptor->uid, 16);
-            break;
-        case 0x3F01:
-            descriptor->sub_descriptors_count = get_be32(pb);
-            if (descriptor->sub_descriptors_count >= UINT_MAX / sizeof(UID) ||
-                descriptor->sub_descriptors_count >= UINT_MAX / sizeof(*descriptor->sub_descriptors))
-                return -1;
-            descriptor->sub_descriptors_refs = av_malloc(descriptor->sub_descriptors_count * sizeof(UID));
-            descriptor->sub_descriptors = av_mallocz(descriptor->sub_descriptors_count * sizeof(*descriptor->sub_descriptors));
-            url_fskip(pb, 4); /* useless size of objects, always 16 according to specs */
-            get_buffer(pb, descriptor->sub_descriptors_refs, descriptor->sub_descriptors_count * sizeof(UID));
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
-    }
     for (i = 0; i < mxf->packages_count; i++) {
         if (mxf->packages[i]) {
             if (!memcmp(mxf->packages[i]->descriptor_ref, descriptor->uid, 16)) {
@@ -573,8 +533,9 @@
     return -1;
 }
 
-static void mxf_read_metadata_pixel_layout(ByteIOContext *pb, MXFDescriptor *descriptor)
+static int mxf_read_metadata_pixel_layout(ByteIOContext *pb, int tag, int size, void *dest)
 {
+    MXFDescriptor *descriptor = dest;
     int code;
 
     do {
@@ -594,64 +555,30 @@
             get_byte(pb);
         }
     } while (code != 0); /* SMPTE 377M E.2.46 */
+    return 0;
 }
 
 static int mxf_read_metadata_generic_descriptor(MXFContext *mxf, KLVPacket *klv)
 {
-    ByteIOContext *pb = &mxf->fc->pb;
     MXFDescriptor *descriptor = av_mallocz(sizeof(*descriptor));
-    int bytes_read = 0;
+    MXFTagDesc tags[] = {
+        {0x3C0A, MXF_TT_UID,       &descriptor->uid, NULL},
+        {0x3004, MXF_TT_UID,       &descriptor->essence_container_ul, NULL},
+        {0x3006, MXF_TT_BE32,      &descriptor->linked_track_id, NULL},
+        {0x3201, MXF_TT_UID,       &descriptor->essence_codec_ul, NULL}, /* PictureEssenceCoding */
+        {0x3203, MXF_TT_BE32,      &descriptor->width, NULL},
+        {0x3202, MXF_TT_BE32,      &descriptor->height, NULL},
+        {0x320E, MXF_TT_FRAC,      &descriptor->aspect_ratio, NULL},
+        {0x3D03, MXF_TT_FRAC,      &descriptor->sample_rate, NULL},
+        {0x3D06, MXF_TT_UID,       &descriptor->essence_codec_ul, NULL}, /* SoundEssenceCompression */
+        {0x3D07, MXF_TT_BE32,      &descriptor->channels, NULL},
+        {0x3D01, MXF_TT_BE32,      &descriptor->bits_per_sample, NULL},
+        {0x3401, MXF_TT_FUNC,      mxf_read_metadata_pixel_layout, descriptor},
+        {0x0000, MXF_TT_LAST,      NULL, NULL},
+    };
     int i, j;
 
-    while (bytes_read < klv->length) {
-        int tag = get_be16(pb);
-        int size = get_be16(pb); /* KLV specified by 0x53 */
-
-        dprintf("tag 0x%04X, size %d\n", tag, size);
-        switch (tag) {
-        case 0x3C0A:
-            get_buffer(pb, descriptor->uid, 16);
-            break;
-        case 0x3004:
-            get_buffer(pb, descriptor->essence_container_ul, 16);
-            break;
-        case 0x3006:
-            descriptor->linked_track_id = get_be32(pb);
-            break;
-        case 0x3201: /* PictureEssenceCoding */
-            get_buffer(pb, descriptor->essence_codec_ul, 16);
-            break;
-        case 0x3203:
-            descriptor->width = get_be32(pb);
-            break;
-        case 0x3202:
-            descriptor->height = get_be32(pb);
-            break;
-        case 0x320E:
-            descriptor->aspect_ratio.num = get_be32(pb);
-            descriptor->aspect_ratio.den = get_be32(pb);
-            break;
-        case 0x3D03:
-            descriptor->sample_rate.num = get_be32(pb);
-            descriptor->sample_rate.den = get_be32(pb);
-            break;
-        case 0x3D06: /* SoundEssenceCompression */
-            get_buffer(pb, descriptor->essence_codec_ul, 16);
-            break;
-        case 0x3D07:
-            descriptor->channels = get_be32(pb);
-            break;
-        case 0x3D01:
-            descriptor->bits_per_sample = get_be32(pb);
-            break;
-        case 0x3401:
-            mxf_read_metadata_pixel_layout(pb, descriptor);
-            break;
-        default:
-            url_fskip(pb, size);
-        }
-        bytes_read += size + 4;
-    }
+    mxf_read_tags(mxf, tags, klv->length);
     for (i = 0; i < mxf->packages_count; i++) {
         if (mxf->packages[i]) {
             if (!memcmp(mxf->packages[i]->descriptor_ref, descriptor->uid, 16)) {



More information about the ffmpeg-devel mailing list