Optimize AssetLocator allocation

Rev Patchset to 7

Author: palemieux

imf-prepare-patches.sh

@@ -2,7 +2,7 @@
 
 set -e
 
-PATCH_VERSION="6"
+PATCH_VERSION="7"
 
 PATCH_NAME="avformat/imf"
 
@@ -82,7 +82,8 @@ CHANGE NOTES:
 - fixed rational initialization
 - removed extraneous call to xmlCleanupParser()
 - fix if/for single line braces
-- replace av_realloc_f with av_fast_realloc when allocating CPL Resources
+- replace av_realloc_f with av_fast_realloc when allocating CPL Resources and
+  ASSETMAP assets
 "
 
 # add tests back to the Makefile

libavformat/imfdec.c

@@ -74,7 +74,7 @@ typedef struct IMFAssetLocator {
  */
 typedef struct IMFAssetLocatorMap {
     uint8_t asset_count;
-    IMFAssetLocator **assets;
+    IMFAssetLocator *assets;
 } IMFAssetLocatorMap;
 
 typedef struct IMFVirtualTrackResourcePlaybackCtx {
@@ -184,34 +184,36 @@ static int parse_imf_asset_map_from_xml_dom(AVFormatContext *s,
         av_log(s, AV_LOG_ERROR, "Unable to parse asset map XML - missing AssetList node\n");
         return AVERROR_INVALIDDATA;
     }
-
+    asset_map->assets = av_realloc_f(asset_map->assets,
+        xmlChildElementCount(node),
+        sizeof(IMFAssetLocator));
+    if (!asset_map->assets) {
+        av_log(NULL, AV_LOG_PANIC, "Cannot allocate IMF asset locators\n");
+        return AVERROR(ENOMEM);
+    }
+    asset_map->asset_count = 0;
     node = xmlFirstElementChild(node);
     while (node) {
         if (av_strcasecmp(node->name, "Asset") != 0)
             continue;
 
-        asset = av_malloc(sizeof(IMFAssetLocator));
-        if (!asset)
-            return AVERROR(ENOMEM);
+        asset = &(asset_map->assets[asset_map->asset_count]);
 
         if (ff_xml_read_UUID(ff_xml_get_child_element_by_name(node, "Id"), asset->uuid)) {
             av_log(s, AV_LOG_ERROR, "Could not parse UUID from asset in asset map.\n");
-            ret = AVERROR_INVALIDDATA;
-            goto clean_up_asset;
+            return AVERROR_INVALIDDATA;
         }
 
         av_log(s, AV_LOG_DEBUG, "Found asset id: " FF_UUID_FORMAT "\n", UID_ARG(asset->uuid));
 
         if (!(node = ff_xml_get_child_element_by_name(node, "ChunkList"))) {
             av_log(s, AV_LOG_ERROR, "Unable to parse asset map XML - missing ChunkList node\n");
-            ret = AVERROR_INVALIDDATA;
-            goto clean_up_asset;
+            return AVERROR_INVALIDDATA;
         }
 
         if (!(node = ff_xml_get_child_element_by_name(node, "Chunk"))) {
             av_log(s, AV_LOG_ERROR, "Unable to parse asset map XML - missing Chunk node\n");
-            ret = AVERROR_INVALIDDATA;
-            goto clean_up_asset;
+            return AVERROR_INVALIDDATA;
         }
 
         uri = xmlNodeGetContent(ff_xml_get_child_element_by_name(node, "Path"));
@@ -222,28 +224,13 @@ static int parse_imf_asset_map_from_xml_dom(AVFormatContext *s,
         xmlFree(uri);
         if (!asset->absolute_uri) {
             av_log(NULL, AV_LOG_PANIC, "Cannot allocate asset locator absolute URI\n");
-            ret = AVERROR(ENOMEM);
-            goto clean_up_asset;
+            return AVERROR(ENOMEM);
         }
 
         av_log(s, AV_LOG_DEBUG, "Found asset absolute URI: %s\n", asset->absolute_uri);
 
-        node = xmlNextElementSibling(node->parent->parent);
-
-        asset_map->assets = av_realloc_f(asset_map->assets,
-            asset_map->asset_count + 1,
-            sizeof(IMFAssetLocator));
-        if (!asset_map->assets) {
-            av_log(NULL, AV_LOG_PANIC, "Cannot allocate IMF asset locators\n");
-            ret = AVERROR(ENOMEM);
-            goto clean_up_asset;
-        }
-        asset_map->assets[asset_map->asset_count++] = asset;
-        continue;
-
-    clean_up_asset:
-        av_freep(&asset);
-        return ret;
+        asset_map->asset_count++;
+        node = xmlNextElementSibling(node);
     }
 
     return ret;
@@ -275,8 +262,7 @@ static void imf_asset_locator_map_free(IMFAssetLocatorMap *asset_map)
         return;
 
     for (int i = 0; i < asset_map->asset_count; ++i) {
-        av_free(asset_map->assets[i]->absolute_uri);
-        av_free(asset_map->assets[i]);
+        av_free(asset_map->assets[i].absolute_uri);
     }
 
     av_freep(&asset_map->assets);
@@ -360,12 +346,9 @@ static int parse_assetmap(AVFormatContext *s, const char *url, AVIOContext *in)
 
 static IMFAssetLocator *find_asset_map_locator(IMFAssetLocatorMap *asset_map, FFUUID uuid)
 {
-    IMFAssetLocator *asset_locator;
-    for (int i = 0; i < asset_map->asset_count; ++i) {
-        asset_locator = asset_map->assets[i];
-        if (memcmp(asset_map->assets[i]->uuid, uuid, 16) == 0)
-            return asset_locator;
-    }
+    for (int i = 0; i < asset_map->asset_count; ++i)
+        if (memcmp(asset_map->assets[i].uuid, uuid, 16) == 0)
+            return &(asset_map->assets[i]);
     return NULL;
 }
 

libavformat/tests/imf.c

@@ -343,26 +343,26 @@ static int test_bad_cpl_parsing(void)
     return 0;
 }
 
-static int check_asset_locator_attributes(IMFAssetLocator *asset, IMFAssetLocator expected_asset)
+static int check_asset_locator_attributes(IMFAssetLocator *asset, IMFAssetLocator *expected_asset)
 {
 
     printf("\tCompare " FF_UUID_FORMAT " to " FF_UUID_FORMAT ".\n",
         UID_ARG(asset->uuid),
-        UID_ARG(expected_asset.uuid));
+        UID_ARG(expected_asset->uuid));
     for (int i = 0; i < 16; ++i) {
-        if (asset->uuid[i] != expected_asset.uuid[i]) {
+        if (asset->uuid[i] != expected_asset->uuid[i]) {
             printf("Invalid asset locator UUID: found " FF_UUID_FORMAT " instead of " FF_UUID_FORMAT " expected.\n",
                 UID_ARG(asset->uuid),
-                UID_ARG(expected_asset.uuid));
+                UID_ARG(expected_asset->uuid));
             return 1;
         }
     }
 
-    printf("\tCompare %s to %s.\n", asset->absolute_uri, expected_asset.absolute_uri);
-    if (strcmp(asset->absolute_uri, expected_asset.absolute_uri) != 0) {
+    printf("\tCompare %s to %s.\n", asset->absolute_uri, expected_asset->absolute_uri);
+    if (strcmp(asset->absolute_uri, expected_asset->absolute_uri) != 0) {
         printf("Invalid asset locator URI: found %s instead of %s expected.\n",
             asset->absolute_uri,
-            expected_asset.absolute_uri);
+            expected_asset->absolute_uri);
         return 1;
     }
 
@@ -414,8 +414,8 @@ static int test_asset_map_parsing(void)
 
     for (int i = 0; i < asset_locator_map->asset_count; ++i) {
         printf("For asset: %d:\n", i);
-        ret = check_asset_locator_attributes(asset_locator_map->assets[i],
-            ASSET_MAP_EXPECTED_LOCATORS[i]);
+        ret = check_asset_locator_attributes(&(asset_locator_map->assets[i]),
+            &(ASSET_MAP_EXPECTED_LOCATORS[i]));
         if (ret > 0)
             goto cleanup;
     }