[Ffmpeg-devel] [PATCH] respect type when resolving MXF strong ref

[Reimar Döffinger]

Hello,
I think this should really be done, since at least in one case we were
even writing into the thing returned by the resolving function without
even checking the type.
Does attached patch look good to you?

Greetings,
Reimar D?ffinger
-------------- next part --------------
Index: libavformat/mxf.c
===================================================================
--- libavformat/mxf.c	(revision 7439)
+++ libavformat/mxf.c	(working copy)
@@ -50,6 +50,7 @@
 typedef uint8_t UID[16];
 
 enum MXFMetadataSetType {
+    AnyType,
     MaterialPackage,
     SourcePackage,
     SourceClip,
@@ -741,14 +742,15 @@
     return uls->type;
 }
 
-static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref)
+static void *mxf_resolve_strong_ref(MXFContext *mxf, UID *strong_ref, enum MXFMetadataSetType type)
 {
     int i;
 
     if (!strong_ref)
         return NULL;
     for (i = 0; i < mxf->metadata_sets_count; i++) {
-        if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16)) {
+        if (!memcmp(*strong_ref, mxf->metadata_sets[i]->uid, 16) &&
+            (type == AnyType || mxf->metadata_sets[i]->type == type)) {
             return mxf->metadata_sets[i];
         }
     }
@@ -764,14 +954,8 @@
     dprintf("metadata sets count %d\n", mxf->metadata_sets_count);
     /* TODO: handle multiple material packages (OP3x) */
     for (i = 0; i < mxf->packages_count; i++) {
-        if (!(temp_package = mxf_resolve_strong_ref(mxf, &mxf->packages_refs[i]))) {
-            av_log(mxf->fc, AV_LOG_ERROR, "could not resolve package strong ref\n");
-            return -1;
-        }
-        if (temp_package->type == MaterialPackage) {
-            material_package = temp_package;
-            break;
-        }
+        material_package = mxf_resolve_strong_ref(mxf, &mxf->packages_refs[i], MaterialPackage);
+        if (material_package) break;
     }
     if (!material_package) {
         av_log(mxf->fc, AV_LOG_ERROR, "no material package found\n");
@@ -789,12 +973,12 @@
         const MXFCodecUL *container_ul = NULL;
         AVStream *st;
 
-        if (!(material_track = mxf_resolve_strong_ref(mxf, &material_package->tracks_refs[i]))) {
+        if (!(material_track = mxf_resolve_strong_ref(mxf, &material_package->tracks_refs[i], Track))) {
             av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track strong ref\n");
             continue;
         }
 
-        if (!(material_track->sequence = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref))) {
+        if (!(material_track->sequence = mxf_resolve_strong_ref(mxf, &material_track->sequence_ref, Sequence))) {
             av_log(mxf->fc, AV_LOG_ERROR, "could not resolve material track sequence strong ref\n");
             return -1;
         }
@@ -802,15 +986,19 @@
         /* TODO: handle multiple source clips */
         for (j = 0; j < material_track->sequence->structural_components_count; j++) {
             /* TODO: handle timecode component */
-            component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j]);
-            if (!component || component->type != SourceClip)
+            component = mxf_resolve_strong_ref(mxf, &material_track->sequence->structural_components_refs[j], SourceClip);
+            if (!component)
                 continue;
 
             for (k = 0; k < mxf->packages_count; k++) {
-                if (!(temp_package = mxf_resolve_strong_ref(mxf, &mxf->packages_refs[k]))) {
+                if (!(temp_package = mxf_resolve_strong_ref(mxf, &mxf->packages_refs[k], AnyType))) {
                     av_log(mxf->fc, AV_LOG_ERROR, "could not resolve source track strong ref\n");
                     return -1;
                 }
+                if (temp_package->type != MaterialPackage && temp_package->type != SourcePackage) {
+                    av_log(mxf->fc, AV_LOG_ERROR, "source track strong ref resolved to bad type\n");
+                    return -1;
+                }
                 if (!memcmp(temp_package->package_uid, component->source_package_uid, 16)) {
                     source_package = temp_package;
                     break;
@@ -821,7 +1009,7 @@
                 break;
             }
             for (k = 0; k < source_package->tracks_count; k++) {
-                if (!(temp_track = mxf_resolve_strong_ref(mxf, &source_package->tracks_refs[k]))) {
+                if (!(temp_track = mxf_resolve_strong_ref(mxf, &source_package->tracks_refs[k], Track))) {
                     av_log(mxf->fc, AV_LOG_ERROR, "could not resolve source track strong ref\n");
                     return -1;
                 }
@@ -846,7 +1034,7 @@
         st->start_time = component->start_position;
         av_set_pts_info(st, 64, material_track->edit_rate.num, material_track->edit_rate.den);
 
-        if (!(source_track->sequence = mxf_resolve_strong_ref(mxf, &source_track->sequence_ref))) {
+        if (!(source_track->sequence = mxf_resolve_strong_ref(mxf, &source_track->sequence_ref, Sequence))) {
             av_log(mxf->fc, AV_LOG_ERROR, "could not resolve source track sequence strong ref\n");
             return -1;
         }
@@ -856,11 +1044,11 @@
 #endif
         st->codec->codec_type = mxf_get_codec_type(mxf_data_definition_uls, &source_track->sequence->data_definition_ul);
 
-        source_package->descriptor = mxf_resolve_strong_ref(mxf, &source_package->descriptor_ref);
+        source_package->descriptor = mxf_resolve_strong_ref(mxf, &source_package->descriptor_ref, Descriptor);
         if (source_package->descriptor) {
             if (source_package->descriptor->type == MultipleDescriptor) {
                 for (j = 0; j < source_package->descriptor->sub_descriptors_count; j++) {
-                    MXFDescriptor *sub_descriptor = mxf_resolve_strong_ref(mxf, &source_package->descriptor->sub_descriptors_refs[j]);
+                    MXFDescriptor *sub_descriptor = mxf_resolve_strong_ref(mxf, &source_package->descriptor->sub_descriptors_refs[j], Descriptor);
 
                     if (!sub_descriptor) {
                         av_log(mxf->fc, AV_LOG_ERROR, "could not resolve sub descriptor strong ref\n");