From 73f146c51bbe1ab61deb94df8be13992177a6600 Mon Sep 17 00:00:00 2001 From: Zane van Iperen Date: Sat, 1 Jan 2022 01:33:39 +1000 Subject: [PATCH 1/2] avformat/imf: cosmetics --- libavformat/imf_cpl.c | 78 +++++++------ libavformat/imfdec.c | 249 ++++++++++++++++-------------------------- 2 files changed, 130 insertions(+), 197 deletions(-) diff --git a/libavformat/imf_cpl.c b/libavformat/imf_cpl.c index f2ad9c05d6..8d97960ca1 100644 --- a/libavformat/imf_cpl.c +++ b/libavformat/imf_cpl.c @@ -77,23 +77,10 @@ int ff_imf_xml_read_uuid(xmlNodePtr element, uint8_t uuid[16]) int ret = 0; element_text = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1); - scanf_ret = sscanf(element_text, - FF_IMF_UUID_FORMAT, - &uuid[0], - &uuid[1], - &uuid[2], - &uuid[3], - &uuid[4], - &uuid[5], - &uuid[6], - &uuid[7], - &uuid[8], - &uuid[9], - &uuid[10], - &uuid[11], - &uuid[12], - &uuid[13], - &uuid[14], + scanf_ret = sscanf(element_text, FF_IMF_UUID_FORMAT, + &uuid[ 0], &uuid[ 1], &uuid[ 2], &uuid[ 3], &uuid[ 4], + &uuid[ 5], &uuid[ 6], &uuid[ 7], &uuid[ 8], &uuid[ 9], + &uuid[10], &uuid[11], &uuid[12], &uuid[13], &uuid[14], &uuid[15]); if (scanf_ret != 16) { av_log(NULL, AV_LOG_ERROR, "Invalid UUID\n"); @@ -190,9 +177,8 @@ static int fill_content_title(xmlNodePtr cpl_element, FFIMFCPL *cpl) av_log(NULL, AV_LOG_ERROR, "ContentTitle element not found in the IMF CPL\n"); return AVERROR_INVALIDDATA; } - cpl->content_title_utf8 = xmlNodeListGetString(cpl_element->doc, - element->xmlChildrenNode, - 1); + + cpl->content_title_utf8 = xmlNodeListGetString(cpl_element->doc, element->xmlChildrenNode, 1); return 0; } @@ -231,6 +217,7 @@ static int fill_marker(xmlNodePtr marker_elem, FFIMFMarker *marker) av_log(NULL, AV_LOG_ERROR, "Offset element not found in a Marker\n"); return AVERROR_INVALIDDATA; } + if ((ret = ff_imf_xml_read_uint32(element, &marker->offset))) return ret; @@ -239,13 +226,14 @@ static int fill_marker(xmlNodePtr marker_elem, FFIMFMarker *marker) av_log(NULL, AV_LOG_ERROR, "Label element not found in a Marker\n"); return AVERROR_INVALIDDATA; } + if (!(marker->label_utf8 = xmlNodeListGetString(element->doc, element->xmlChildrenNode, 1))) { av_log(NULL, AV_LOG_ERROR, "Empty Label element found in a Marker\n"); return AVERROR_INVALIDDATA; } + if (!(marker->scope_utf8 = xmlGetNoNsProp(element, "scope"))) { - marker->scope_utf8 - = xmlCharStrdup("http://www.smpte-ra.org/schemas/2067-3/2013#standard-markers"); + marker->scope_utf8 = xmlCharStrdup("http://www.smpte-ra.org/schemas/2067-3/2013#standard-markers"); if (!marker->scope_utf8) { xmlFree(marker->label_utf8); return AVERROR(ENOMEM); @@ -283,6 +271,7 @@ static int fill_base_resource(xmlNodePtr resource_elem, FFIMFBaseResource *resou av_log(NULL, AV_LOG_ERROR, "IntrinsicDuration element missing from Resource\n"); return AVERROR_INVALIDDATA; } + if ((ret = ff_imf_xml_read_uint32(element, &resource->duration))) { av_log(NULL, AV_LOG_ERROR, "Invalid IntrinsicDuration element found in a Resource\n"); return ret; @@ -305,8 +294,7 @@ static int fill_base_resource(xmlNodePtr resource_elem, FFIMFBaseResource *resou } static int fill_trackfile_resource(xmlNodePtr tf_resource_elem, - FFIMFTrackFileResource *tf_resource, - FFIMFCPL *cpl) + FFIMFTrackFileResource *tf_resource, FFIMFCPL *cpl) { xmlNodePtr element = NULL; int ret = 0; @@ -329,8 +317,7 @@ static int fill_trackfile_resource(xmlNodePtr tf_resource_elem, } static int fill_marker_resource(xmlNodePtr marker_resource_elem, - FFIMFMarkerResource *marker_resource, - FFIMFCPL *cpl) + FFIMFMarkerResource *marker_resource, FFIMFCPL *cpl) { xmlNodePtr element = NULL; int ret = 0; @@ -354,8 +341,7 @@ static int fill_marker_resource(xmlNodePtr marker_resource_elem, marker_resource->markers = tmp; imf_marker_init(&marker_resource->markers[marker_resource->marker_count]); - ret = fill_marker(element, - &marker_resource->markers[marker_resource->marker_count]); + ret = fill_marker(element, &marker_resource->markers[marker_resource->marker_count]); marker_resource->marker_count++; if (ret) return ret; @@ -382,12 +368,13 @@ static int push_marker_sequence(xmlNodePtr marker_sequence_elem, FFIMFCPL *cpl) av_log(NULL, AV_LOG_ERROR, "TrackId element missing from Sequence\n"); return AVERROR_INVALIDDATA; } + if (ff_imf_xml_read_uuid(track_id_elem, uuid)) { av_log(NULL, AV_LOG_ERROR, "Invalid TrackId element found in Sequence\n"); return AVERROR_INVALIDDATA; } - av_log(NULL, - AV_LOG_DEBUG, + + av_log(NULL, AV_LOG_DEBUG, "Processing IMF CPL Marker Sequence for Virtual Track " FF_IMF_UUID_FORMAT "\n", UID_ARG(uuid)); @@ -396,6 +383,7 @@ static int push_marker_sequence(xmlNodePtr marker_sequence_elem, FFIMFCPL *cpl) cpl->main_markers_track = av_malloc(sizeof(FFIMFMarkerVirtualTrack)); if (!cpl->main_markers_track) return AVERROR(ENOMEM); + imf_marker_virtual_track_init(cpl->main_markers_track); memcpy(cpl->main_markers_track->base.id_uuid, uuid, sizeof(uuid)); @@ -413,6 +401,7 @@ static int push_marker_sequence(xmlNodePtr marker_sequence_elem, FFIMFCPL *cpl) if (resource_elem_count > UINT32_MAX || cpl->main_markers_track->resource_count > UINT32_MAX - resource_elem_count) return AVERROR(ENOMEM); + tmp = av_realloc_array(cpl->main_markers_track->resources, cpl->main_markers_track->resource_count + resource_elem_count, sizeof(FFIMFMarkerResource)); @@ -470,10 +459,12 @@ static int push_main_audio_sequence(xmlNodePtr audio_sequence_elem, FFIMFCPL *cp av_log(NULL, AV_LOG_ERROR, "TrackId element missing from audio sequence\n"); return AVERROR_INVALIDDATA; } + if ((ret = ff_imf_xml_read_uuid(track_id_elem, uuid))) { av_log(NULL, AV_LOG_ERROR, "Invalid TrackId element found in audio sequence\n"); return ret; } + av_log(NULL, AV_LOG_DEBUG, "Processing IMF CPL Audio Sequence for Virtual Track " FF_IMF_UUID_FORMAT "\n", @@ -513,8 +504,7 @@ static int push_main_audio_sequence(xmlNodePtr audio_sequence_elem, FFIMFCPL *cp if (resource_elem_count > UINT32_MAX || vt->resource_count > UINT32_MAX - resource_elem_count) return AVERROR(ENOMEM); - tmp = av_fast_realloc(vt->resources, - &vt->resources_alloc_sz, + tmp = av_fast_realloc(vt->resources, &vt->resources_alloc_sz, (vt->resource_count + resource_elem_count) * sizeof(FFIMFTrackFileResource)); if (!tmp) { @@ -526,9 +516,7 @@ static int push_main_audio_sequence(xmlNodePtr audio_sequence_elem, FFIMFCPL *cp resource_elem = xmlFirstElementChild(resource_list_elem); while (resource_elem) { imf_trackfile_resource_init(&vt->resources[vt->resource_count]); - ret = fill_trackfile_resource(resource_elem, - &vt->resources[vt->resource_count], - cpl); + ret = fill_trackfile_resource(resource_elem, &vt->resources[vt->resource_count], cpl); vt->resource_count++; if (ret) { av_log(NULL, AV_LOG_ERROR, "Invalid Resource\n"); @@ -562,6 +550,7 @@ static int push_main_image_2d_sequence(xmlNodePtr image_sequence_elem, FFIMFCPL av_log(NULL, AV_LOG_ERROR, "TrackId element missing from audio sequence\n"); return AVERROR_INVALIDDATA; } + if ((ret = ff_imf_xml_read_uuid(track_id_elem, uuid))) { av_log(NULL, AV_LOG_ERROR, "Invalid TrackId element found in audio sequence\n"); return ret; @@ -579,8 +568,8 @@ static int push_main_image_2d_sequence(xmlNodePtr image_sequence_elem, FFIMFCPL av_log(NULL, AV_LOG_ERROR, "Multiple MainImage virtual tracks found\n"); return AVERROR_INVALIDDATA; } - av_log(NULL, - AV_LOG_DEBUG, + + av_log(NULL, AV_LOG_DEBUG, "Processing IMF CPL Main Image Sequence for Virtual Track " FF_IMF_UUID_FORMAT "\n", UID_ARG(uuid)); @@ -595,6 +584,7 @@ static int push_main_image_2d_sequence(xmlNodePtr image_sequence_elem, FFIMFCPL || (cpl->main_image_2d_track->resource_count + resource_elem_count) > INT_MAX / sizeof(FFIMFTrackFileResource)) return AVERROR(ENOMEM); + tmp = av_fast_realloc(cpl->main_image_2d_track->resources, &cpl->main_image_2d_track->resources_alloc_sz, (cpl->main_image_2d_track->resource_count + resource_elem_count) @@ -658,8 +648,7 @@ static int fill_virtual_tracks(xmlNodePtr cpl_element, FFIMFCPL *cpl) ret = push_main_audio_sequence(sequence_elem, cpl); else - av_log(NULL, - AV_LOG_INFO, + av_log(NULL, AV_LOG_INFO, "The following Sequence is not supported and is ignored: %s\n", sequence_elem->name); @@ -715,6 +704,7 @@ static void imf_marker_free(FFIMFMarker *marker) { if (!marker) return; + xmlFree(marker->label_utf8); xmlFree(marker->scope_utf8); } @@ -723,8 +713,10 @@ static void imf_marker_resource_free(FFIMFMarkerResource *rsrc) { if (!rsrc) return; + for (uint32_t i = 0; i < rsrc->marker_count; i++) imf_marker_free(&rsrc->markers[i]); + av_freep(&rsrc->markers); } @@ -732,8 +724,10 @@ static void imf_marker_virtual_track_free(FFIMFMarkerVirtualTrack *vt) { if (!vt) return; + for (uint32_t i = 0; i < vt->resource_count; i++) imf_marker_resource_free(&vt->resources[i]); + av_freep(&vt->resources); } @@ -741,6 +735,7 @@ static void imf_trackfile_virtual_track_free(FFIMFTrackFileVirtualTrack *vt) { if (!vt) return; + av_freep(&vt->resources); } @@ -759,9 +754,9 @@ FFIMFCPL *ff_imf_cpl_alloc(void) { FFIMFCPL *cpl; - cpl = av_malloc(sizeof(FFIMFCPL)); - if (!cpl) + if (!(cpl = av_malloc(sizeof(FFIMFCPL)))) return NULL; + imf_cpl_init(cpl); return cpl; } @@ -803,6 +798,7 @@ int ff_imf_parse_cpl(AVIOContext *in, FFIMFCPL **cpl) filesize = filesize > 0 ? filesize : 8192; av_bprint_init(&buf, filesize + 1, AV_BPRINT_SIZE_UNLIMITED); ret = avio_read_to_bprint(in, &buf, UINT_MAX - 1); + if (ret < 0 || !avio_feof(in) || buf.len == 0) { av_log(NULL, AV_LOG_ERROR, "Cannot read IMF CPL\n"); if (ret == 0) diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c index d67c9b8898..5bf1f77618 100644 --- a/libavformat/imfdec.c +++ b/libavformat/imfdec.c @@ -89,36 +89,36 @@ typedef struct IMFAssetLocator { * Results from the parsing of one or more ASSETMAP XML files */ typedef struct IMFAssetLocatorMap { - uint32_t asset_count; + uint32_t asset_count; IMFAssetLocator *assets; } IMFAssetLocatorMap; typedef struct IMFVirtualTrackResourcePlaybackCtx { - IMFAssetLocator *locator; + IMFAssetLocator *locator; FFIMFTrackFileResource *resource; - AVFormatContext *ctx; + AVFormatContext *ctx; } IMFVirtualTrackResourcePlaybackCtx; typedef struct IMFVirtualTrackPlaybackCtx { - int32_t index; /**< Track index in playlist */ - AVRational current_timestamp; /**< Current temporal position */ - AVRational duration; /**< Overall duration */ - uint32_t resource_count; /**< Number of resources */ - unsigned int resources_alloc_sz; /**< Size of the buffer holding the resource */ - IMFVirtualTrackResourcePlaybackCtx *resources; /**< Buffer holding the resources */ - uint32_t current_resource_index; /**< Current resource */ - int64_t last_pts; /**< Last timestamp */ + int32_t index; /**< Track index in playlist */ + AVRational current_timestamp; /**< Current temporal position */ + AVRational duration; /**< Overall duration */ + uint32_t resource_count; /**< Number of resources */ + unsigned int resources_alloc_sz; /**< Size of the buffer holding the resource */ + IMFVirtualTrackResourcePlaybackCtx *resources; /**< Buffer holding the resources */ + uint32_t current_resource_index; /**< Current resource */ + int64_t last_pts; /**< Last timestamp */ } IMFVirtualTrackPlaybackCtx; typedef struct IMFContext { - const AVClass *class; - const char *base_url; - char *asset_map_paths; - AVIOInterruptCB *interrupt_callback; - AVDictionary *avio_opts; - FFIMFCPL *cpl; - IMFAssetLocatorMap asset_locator_map; - uint32_t track_count; + const AVClass *class; + const char *base_url; + char *asset_map_paths; + AVIOInterruptCB *interrupt_callback; + AVDictionary *avio_opts; + FFIMFCPL *cpl; + IMFAssetLocatorMap asset_locator_map; + uint32_t track_count; IMFVirtualTrackPlaybackCtx **tracks; } IMFContext; @@ -157,8 +157,7 @@ static int imf_uri_is_dos_abs_path(const char *string) * @param base_url the url of the asset map XML file, if any (can be NULL). * @return a negative value in case of error, 0 otherwise. */ -static int parse_imf_asset_map_from_xml_dom(AVFormatContext *s, - xmlDocPtr doc, +static int parse_imf_asset_map_from_xml_dom(AVFormatContext *s, xmlDocPtr doc, IMFAssetLocatorMap *asset_map, const char *base_url) { @@ -179,11 +178,8 @@ static int parse_imf_asset_map_from_xml_dom(AVFormatContext *s, } if (asset_map_element->type != XML_ELEMENT_NODE || av_strcasecmp(asset_map_element->name, "AssetMap")) { - av_log(s, - AV_LOG_ERROR, - "Unable to parse asset map XML - wrong root node name[%s] type[%d]\n", - asset_map_element->name, - (int)asset_map_element->type); + av_log(s, AV_LOG_ERROR, "Unable to parse asset map XML - wrong root node name[%s] type[%d]\n", + asset_map_element->name, (int)asset_map_element->type); return AVERROR_INVALIDDATA; } @@ -192,12 +188,13 @@ 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; } + elem_count = xmlChildElementCount(node); if (elem_count > UINT32_MAX || asset_map->asset_count > UINT32_MAX - elem_count) return AVERROR(ENOMEM); - tmp = av_realloc_array(asset_map->assets, - elem_count + asset_map->asset_count, + + tmp = av_realloc_array(asset_map->assets, elem_count + asset_map->asset_count, sizeof(IMFAssetLocator)); if (!tmp) { av_log(s, AV_LOG_ERROR, "Cannot allocate IMF asset locators\n"); @@ -235,6 +232,7 @@ static int parse_imf_asset_map_from_xml_dom(AVFormatContext *s, else asset->absolute_uri = av_strdup(uri); xmlFree(uri); + if (!asset->absolute_uri) return AVERROR(ENOMEM); @@ -261,7 +259,7 @@ static void imf_asset_locator_map_init(IMFAssetLocatorMap *asset_map) */ static void imf_asset_locator_map_deinit(IMFAssetLocatorMap *asset_map) { - for (uint32_t i = 0; i < asset_map->asset_count; ++i) + for (uint32_t i = 0; i < asset_map->asset_count; i++) av_freep(&asset_map->assets[i].absolute_uri); av_freep(&asset_map->assets); @@ -309,11 +307,7 @@ static int parse_assetmap(AVFormatContext *s, const char *url) ret = parse_imf_asset_map_from_xml_dom(s, doc, &c->asset_locator_map, base_url); if (!ret) - av_log(s, - AV_LOG_DEBUG, - "Found %d assets from %s\n", - c->asset_locator_map.asset_count, - url); + av_log(s, AV_LOG_DEBUG, "Found %d assets from %s\n", c->asset_locator_map.asset_count, url); xmlFreeDoc(doc); @@ -327,15 +321,14 @@ static int parse_assetmap(AVFormatContext *s, const char *url) static IMFAssetLocator *find_asset_map_locator(IMFAssetLocatorMap *asset_map, FFIMFUUID uuid) { - for (uint32_t i = 0; i < asset_map->asset_count; ++i) { + for (uint32_t 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; } -static int open_track_resource_context(AVFormatContext *s, - IMFVirtualTrackResourcePlaybackCtx *track_resource) +static int open_track_resource_context(AVFormatContext *s, IMFVirtualTrackResourcePlaybackCtx *track_resource) { IMFContext *c = s->priv_data; int ret = 0; @@ -343,10 +336,7 @@ static int open_track_resource_context(AVFormatContext *s, AVDictionary *opts = NULL; if (track_resource->ctx) { - av_log(s, - AV_LOG_DEBUG, - "Input context already opened for %s.\n", - track_resource->locator->absolute_uri); + av_log(s, AV_LOG_DEBUG, "Input context already opened for %s.\n", track_resource->locator->absolute_uri); return 0; } @@ -368,16 +358,10 @@ static int open_track_resource_context(AVFormatContext *s, if ((ret = av_dict_copy(&opts, c->avio_opts, 0)) < 0) goto cleanup; - ret = avformat_open_input(&track_resource->ctx, - track_resource->locator->absolute_uri, - NULL, - &opts); + ret = avformat_open_input(&track_resource->ctx, track_resource->locator->absolute_uri, NULL, &opts); if (ret < 0) { - av_log(s, - AV_LOG_ERROR, - "Could not open %s input context: %s\n", - track_resource->locator->absolute_uri, - av_err2str(ret)); + av_log(s, AV_LOG_ERROR, "Could not open %s input context: %s\n", + track_resource->locator->absolute_uri, av_err2str(ret)); goto cleanup; } av_dict_free(&opts); @@ -387,13 +371,9 @@ static int open_track_resource_context(AVFormatContext *s, */ if (av_cmp_q(track_resource->ctx->streams[0]->time_base, av_inv_q(track_resource->resource->base.edit_rate))) - av_log(s, - AV_LOG_WARNING, - "Incoherent source stream timebase %d/%d regarding resource edit rate: %d/%d", - track_resource->ctx->streams[0]->time_base.num, - track_resource->ctx->streams[0]->time_base.den, - track_resource->resource->base.edit_rate.den, - track_resource->resource->base.edit_rate.num); + av_log(s, AV_LOG_WARNING, "Incoherent source stream timebase %d/%d regarding resource edit rate: %d/%d", + track_resource->ctx->streams[0]->time_base.num, track_resource->ctx->streams[0]->time_base.den, + track_resource->resource->base.edit_rate.den, track_resource->resource->base.edit_rate.num); entry_point = (int64_t)track_resource->resource->base.entry_point * track_resource->resource->base.edit_rate.den @@ -401,19 +381,12 @@ static int open_track_resource_context(AVFormatContext *s, / track_resource->resource->base.edit_rate.num; if (entry_point) { - av_log(s, - AV_LOG_DEBUG, - "Seek at resource %s entry point: %" PRIu32 "\n", - track_resource->locator->absolute_uri, - track_resource->resource->base.entry_point); + av_log(s, AV_LOG_DEBUG, "Seek at resource %s entry point: %" PRIu32 "\n", + track_resource->locator->absolute_uri, track_resource->resource->base.entry_point); ret = avformat_seek_file(track_resource->ctx, -1, entry_point, entry_point, entry_point, 0); if (ret < 0) { - av_log(s, - AV_LOG_ERROR, - "Could not seek at %" PRId64 "on %s: %s\n", - entry_point, - track_resource->locator->absolute_uri, - av_err2str(ret)); + av_log(s, AV_LOG_ERROR, "Could not seek at %" PRId64 "on %s: %s\n", entry_point, + track_resource->locator->absolute_uri, av_err2str(ret)); avformat_close_input(&track_resource->ctx); return ret; } @@ -437,46 +410,45 @@ static int open_track_file_resource(AVFormatContext *s, void *tmp; int ret; - asset_locator = find_asset_map_locator(&c->asset_locator_map, track_file_resource->track_file_uuid); + asset_locator = find_asset_map_locator(&c->asset_locator_map, + track_file_resource->track_file_uuid); if (!asset_locator) { - av_log(s, - AV_LOG_ERROR, - "Could not find asset locator for UUID: " FF_IMF_UUID_FORMAT "\n", + av_log(s, AV_LOG_ERROR, "Could not find asset locator for UUID: " FF_IMF_UUID_FORMAT "\n", UID_ARG(track_file_resource->track_file_uuid)); return AVERROR_INVALIDDATA; } - av_log(s, - AV_LOG_DEBUG, - "Found locator for " FF_IMF_UUID_FORMAT ": %s\n", - UID_ARG(asset_locator->uuid), - asset_locator->absolute_uri); + av_log(s, AV_LOG_DEBUG, "Found locator for " FF_IMF_UUID_FORMAT ": %s\n", + UID_ARG(asset_locator->uuid), asset_locator->absolute_uri); if (track->resource_count > UINT32_MAX - track_file_resource->base.repeat_count || (track->resource_count + track_file_resource->base.repeat_count) > INT_MAX / sizeof(IMFVirtualTrackResourcePlaybackCtx)) return AVERROR(ENOMEM); - tmp = av_fast_realloc(track->resources, - &track->resources_alloc_sz, - (track->resource_count + track_file_resource->base.repeat_count) - * sizeof(IMFVirtualTrackResourcePlaybackCtx)); + + tmp = av_fast_realloc(track->resources, &track->resources_alloc_sz, + (track->resource_count + track_file_resource->base.repeat_count) * + sizeof(IMFVirtualTrackResourcePlaybackCtx)); if (!tmp) return AVERROR(ENOMEM); track->resources = tmp; - for (uint32_t i = 0; i < track_file_resource->base.repeat_count; ++i) { + for (uint32_t i = 0; i < track_file_resource->base.repeat_count; i++) { IMFVirtualTrackResourcePlaybackCtx vt_ctx; vt_ctx.locator = asset_locator; vt_ctx.resource = track_file_resource; vt_ctx.ctx = NULL; + if ((ret = open_track_resource_context(s, &vt_ctx)) != 0) return ret; + track->resources[track->resource_count++] = vt_ctx; - track->duration = av_add_q(track->duration, - av_make_q((int)track_file_resource->base.duration - * track_file_resource->base.edit_rate.den, - track_file_resource->base.edit_rate.num)); + track->duration = av_add_q( + track->duration, + av_make_q((int)track_file_resource->base.duration * track_file_resource->base.edit_rate.den, + track_file_resource->base.edit_rate.num) + ); } return 0; @@ -484,7 +456,7 @@ static int open_track_file_resource(AVFormatContext *s, static void imf_virtual_track_playback_context_deinit(IMFVirtualTrackPlaybackCtx *track) { - for (uint32_t i = 0; i < track->resource_count; ++i) + for (uint32_t i = 0; i < track->resource_count; i++) avformat_close_input(&track->resources[i].ctx); av_freep(&track->resources); @@ -501,19 +473,16 @@ static int open_virtual_track(AVFormatContext *s, if (!(track = av_mallocz(sizeof(IMFVirtualTrackPlaybackCtx)))) return AVERROR(ENOMEM); + track->index = track_index; track->duration = av_make_q(0, 1); for (uint32_t i = 0; i < virtual_track->resource_count; i++) { - av_log(s, - AV_LOG_DEBUG, - "Open stream from file " FF_IMF_UUID_FORMAT ", stream %d\n", - UID_ARG(virtual_track->resources[i].track_file_uuid), - i); + av_log(s, AV_LOG_DEBUG, "Open stream from file " FF_IMF_UUID_FORMAT ", stream %d\n", + UID_ARG(virtual_track->resources[i].track_file_uuid), i); + if ((ret = open_track_file_resource(s, &virtual_track->resources[i], track)) != 0) { - av_log(s, - AV_LOG_ERROR, - "Could not open image track resource " FF_IMF_UUID_FORMAT "\n", + av_log(s, AV_LOG_ERROR, "Could not open image track resource " FF_IMF_UUID_FORMAT "\n", UID_ARG(virtual_track->resources[i].track_file_uuid)); goto clean_up; } @@ -525,11 +494,13 @@ static int open_virtual_track(AVFormatContext *s, ret = AVERROR(ENOMEM); goto clean_up; } + tmp = av_realloc_array(c->tracks, c->track_count + 1, sizeof(IMFVirtualTrackPlaybackCtx *)); if (!tmp) { ret = AVERROR(ENOMEM); goto clean_up; } + c->tracks = tmp; c->tracks[c->track_count++] = track; @@ -546,7 +517,7 @@ static int set_context_streams_from_tracks(AVFormatContext *s) IMFContext *c = s->priv_data; int ret = 0; - for (uint32_t i = 0; i < c->track_count; ++i) { + for (uint32_t i = 0; i < c->track_count; i++) { AVStream *asset_stream; AVStream *first_resource_stream; @@ -560,18 +531,17 @@ static int set_context_streams_from_tracks(AVFormatContext *s) av_log(s, AV_LOG_ERROR, "Could not create stream\n"); return AVERROR(ENOMEM); } + asset_stream->id = i; ret = avcodec_parameters_copy(asset_stream->codecpar, first_resource_stream->codecpar); if (ret < 0) { av_log(s, AV_LOG_ERROR, "Could not copy stream parameters\n"); return ret; } - avpriv_set_pts_info(asset_stream, - first_resource_stream->pts_wrap_bits, - first_resource_stream->time_base.num, + + avpriv_set_pts_info(asset_stream, first_resource_stream->pts_wrap_bits, first_resource_stream->time_base.num, first_resource_stream->time_base.den); - asset_stream->duration = (int64_t)av_q2d(av_mul_q(c->tracks[i]->duration, - av_inv_q(asset_stream->time_base))); + asset_stream->duration = (int64_t)av_q2d(av_mul_q(c->tracks[i]->duration, av_inv_q(asset_stream->time_base))); } return 0; @@ -585,18 +555,15 @@ static int open_cpl_tracks(AVFormatContext *s) if (c->cpl->main_image_2d_track) { if ((ret = open_virtual_track(s, c->cpl->main_image_2d_track, track_index++)) != 0) { - av_log(s, - AV_LOG_ERROR, - "Could not open image track " FF_IMF_UUID_FORMAT "\n", + av_log(s, AV_LOG_ERROR, "Could not open image track " FF_IMF_UUID_FORMAT "\n", UID_ARG(c->cpl->main_image_2d_track->base.id_uuid)); return ret; } } - for (uint32_t i = 0; i < c->cpl->main_audio_track_count; ++i) { + for (uint32_t i = 0; i < c->cpl->main_audio_track_count; i++) { if ((ret = open_virtual_track(s, &c->cpl->main_audio_tracks[i], track_index++)) != 0) { - av_log(s, - AV_LOG_ERROR, + av_log(s, AV_LOG_ERROR, "Could not open audio track " FF_IMF_UUID_FORMAT "\n", UID_ARG(c->cpl->main_audio_tracks[i].base.id_uuid)); return ret; @@ -630,10 +597,7 @@ static int imf_read_header(AVFormatContext *s) if ((ret = ff_imf_parse_cpl(s->pb, &c->cpl)) < 0) return ret; - av_log(s, - AV_LOG_DEBUG, - "parsed IMF CPL: " FF_IMF_UUID_FORMAT "\n", - UID_ARG(c->cpl->id_uuid)); + av_log(s, AV_LOG_DEBUG, "parsed IMF CPL: " FF_IMF_UUID_FORMAT "\n", UID_ARG(c->cpl->id_uuid)); if (!c->asset_map_paths) { c->asset_map_paths = av_append_path_component(c->base_url, "ASSETMAP.xml"); @@ -673,13 +637,11 @@ static IMFVirtualTrackPlaybackCtx *get_next_track_with_minimum_timestamp(AVForma AVRational minimum_timestamp = av_make_q(INT32_MAX, 1); for (uint32_t i = c->track_count; i > 0; i--) { - av_log(s, - AV_LOG_DEBUG, + av_log(s, AV_LOG_DEBUG, "Compare track %d timestamp " AVRATIONAL_FORMAT " to minimum " AVRATIONAL_FORMAT " (over duration: " AVRATIONAL_FORMAT - ")\n", - i, + ")\n", i, AVRATIONAL_ARG(c->tracks[i - 1]->current_timestamp), AVRATIONAL_ARG(minimum_timestamp), AVRATIONAL_ARG(c->tracks[i - 1]->duration)); @@ -690,12 +652,8 @@ static IMFVirtualTrackPlaybackCtx *get_next_track_with_minimum_timestamp(AVForma } } - av_log(s, - AV_LOG_DEBUG, - "Found next track to read: %d (timestamp: %lf / %lf)\n", - track->index, - av_q2d(track->current_timestamp), - av_q2d(minimum_timestamp)); + av_log(s, AV_LOG_DEBUG, "Found next track to read: %d (timestamp: %lf / %lf)\n", + track->index, av_q2d(track->current_timestamp), av_q2d(minimum_timestamp)); return track; } @@ -705,42 +663,34 @@ static IMFVirtualTrackResourcePlaybackCtx *get_resource_context_for_timestamp(AV AVRational edit_unit_duration = av_inv_q(track->resources[0].resource->base.edit_rate); AVRational cumulated_duration = av_make_q(0, edit_unit_duration.den); - av_log(s, - AV_LOG_DEBUG, - "Looking for track %d resource for timestamp = %lf / %lf\n", - track->index, - av_q2d(track->current_timestamp), - av_q2d(track->duration)); - for (uint32_t i = 0; i < track->resource_count; ++i) { + av_log(s, AV_LOG_DEBUG, "Looking for track %d resource for timestamp = %lf / %lf\n", + track->index, av_q2d(track->current_timestamp), av_q2d(track->duration)); + for (uint32_t i = 0; i < track->resource_count; i++) { cumulated_duration = av_add_q(cumulated_duration, av_make_q((int)track->resources[i].resource->base.duration * edit_unit_duration.num, edit_unit_duration.den)); if (av_cmp_q(av_add_q(track->current_timestamp, edit_unit_duration), cumulated_duration) <= 0) { - av_log(s, - AV_LOG_DEBUG, + av_log(s, AV_LOG_DEBUG, "Found resource %d in track %d to read for timestamp %lf " "(on cumulated=%lf): entry=%" PRIu32 ", duration=%" PRIu32 ", editrate=" AVRATIONAL_FORMAT " | edit_unit_duration=%lf\n", - i, - track->index, - av_q2d(track->current_timestamp), - av_q2d(cumulated_duration), + i, track->index, + av_q2d(track->current_timestamp), av_q2d(cumulated_duration), track->resources[i].resource->base.entry_point, track->resources[i].resource->base.duration, AVRATIONAL_ARG(track->resources[i].resource->base.edit_rate), av_q2d(edit_unit_duration)); if (track->current_resource_index != i) { - av_log(s, - AV_LOG_DEBUG, - "Switch resource on track %d: re-open context\n", - track->index); + av_log(s, AV_LOG_DEBUG, "Switch resource on track %d: re-open context\n", track->index); + if (open_track_resource_context(s, &(track->resources[i])) != 0) return NULL; + avformat_close_input(&(track->resources[track->current_resource_index].ctx)); track->current_resource_index = i; } @@ -780,18 +730,9 @@ static int imf_read_packet(AVFormatContext *s, AVPacket *pkt) while (!ff_check_interrupt(c->interrupt_callback) && !ret) { ret = av_read_frame(resource_to_read->ctx, pkt); - av_log(s, - AV_LOG_DEBUG, - "Got packet: pts=%" PRId64 - ", dts=%" PRId64 - ", duration=%" PRId64 - ", stream_index=%d, pos=%" PRId64 - "\n", - pkt->pts, - pkt->dts, - pkt->duration, - pkt->stream_index, - pkt->pos); + av_log(s, AV_LOG_DEBUG, "Got packet: pts=%" PRId64 ", dts=%" PRId64 + ", duration=%" PRId64 ", stream_index=%d, pos=%" PRId64 "\n", + pkt->pts, pkt->dts, pkt->duration, pkt->stream_index, pkt->pos); track_stream = ffstream(s->streams[track->index]); if (ret >= 0) { @@ -814,11 +755,7 @@ static int imf_read_packet(AVFormatContext *s, AVPacket *pkt) return 0; } else if (ret != AVERROR_EOF) { - av_log(s, - AV_LOG_ERROR, - "Could not get packet from track %d: %s\n", - track->index, - av_err2str(ret)); + av_log(s, AV_LOG_ERROR, "Could not get packet from track %d: %s\n", track->index, av_err2str(ret)); return ret; } } @@ -836,7 +773,7 @@ static int imf_close(AVFormatContext *s) imf_asset_locator_map_deinit(&c->asset_locator_map); ff_imf_cpl_free(c->cpl); - for (uint32_t i = 0; i < c->track_count; ++i) { + for (uint32_t i = 0; i < c->track_count; i++) { imf_virtual_track_playback_context_deinit(c->tracks[i]); av_freep(&c->tracks[i]); } From 092434a08326cfaab30c1d8b417b6c7843c10e05 Mon Sep 17 00:00:00 2001 From: Pierre-Anthony Lemieux Date: Sun, 2 Jan 2022 14:54:20 -0800 Subject: [PATCH 2/2] Futher style improvements --- libavformat/imf.h | 46 ++++++++++++++++++++--------------------- libavformat/imfdec.c | 11 ++++------ libavformat/tests/imf.c | 32 ++++++++++++---------------- 3 files changed, 40 insertions(+), 49 deletions(-) diff --git a/libavformat/imf.h b/libavformat/imf.h index 62c4468ce9..058600ce48 100644 --- a/libavformat/imf.h +++ b/libavformat/imf.h @@ -73,10 +73,10 @@ typedef uint8_t FFIMFUUID[16]; * IMF Composition Playlist Base Resource */ typedef struct FFIMFBaseResource { - AVRational edit_rate; /**< BaseResourceType/EditRate */ - uint32_t entry_point; /**< BaseResourceType/EntryPoint */ - uint32_t duration; /**< BaseResourceType/Duration */ - uint32_t repeat_count; /**< BaseResourceType/RepeatCount */ + AVRational edit_rate; /**< BaseResourceType/EditRate */ + uint32_t entry_point; /**< BaseResourceType/EntryPoint */ + uint32_t duration; /**< BaseResourceType/Duration */ + uint32_t repeat_count; /**< BaseResourceType/RepeatCount */ } FFIMFBaseResource; /** @@ -84,16 +84,16 @@ typedef struct FFIMFBaseResource { */ typedef struct FFIMFTrackFileResource { FFIMFBaseResource base; - FFIMFUUID track_file_uuid; /**< TrackFileResourceType/TrackFileId */ + FFIMFUUID track_file_uuid; /**< TrackFileResourceType/TrackFileId */ } FFIMFTrackFileResource; /** * IMF Marker */ typedef struct FFIMFMarker { - xmlChar *label_utf8; /**< Marker/Label */ - xmlChar *scope_utf8; /**< Marker/Label/\@scope */ - uint32_t offset; /**< Marker/Offset */ + xmlChar *label_utf8; /**< Marker/Label */ + xmlChar *scope_utf8; /**< Marker/Label/\@scope */ + uint32_t offset; /**< Marker/Offset */ } FFIMFMarker; /** @@ -101,8 +101,8 @@ typedef struct FFIMFMarker { */ typedef struct FFIMFMarkerResource { FFIMFBaseResource base; - uint32_t marker_count; /**< Number of Marker elements */ - FFIMFMarker *markers; /**< Marker elements */ + uint32_t marker_count; /**< Number of Marker elements */ + FFIMFMarker *markers; /**< Marker elements */ } FFIMFMarkerResource; /** @@ -116,10 +116,10 @@ typedef struct FFIMFBaseVirtualTrack { * IMF Composition Playlist Virtual Track that consists of Track File Resources */ typedef struct FFIMFTrackFileVirtualTrack { - FFIMFBaseVirtualTrack base; - uint32_t resource_count; /**< Number of Resource elements present in the Virtual Track */ - FFIMFTrackFileResource *resources; /**< Resource elements of the Virtual Track */ - unsigned int resources_alloc_sz; /**< Size of the resources buffer */ + FFIMFBaseVirtualTrack base; + uint32_t resource_count; /**< Number of Resource elements present in the Virtual Track */ + FFIMFTrackFileResource *resources; /**< Resource elements of the Virtual Track */ + unsigned int resources_alloc_sz; /**< Size of the resources buffer */ } FFIMFTrackFileVirtualTrack; /** @@ -127,21 +127,21 @@ typedef struct FFIMFTrackFileVirtualTrack { */ typedef struct FFIMFMarkerVirtualTrack { FFIMFBaseVirtualTrack base; - uint32_t resource_count; /**< Number of Resource elements present in the Virtual Track */ - FFIMFMarkerResource *resources; /**< Resource elements of the Virtual Track */ + uint32_t resource_count; /**< Number of Resource elements present in the Virtual Track */ + FFIMFMarkerResource *resources; /**< Resource elements of the Virtual Track */ } FFIMFMarkerVirtualTrack; /** * IMF Composition Playlist */ typedef struct FFIMFCPL { - FFIMFUUID id_uuid; /**< CompositionPlaylist/Id element */ - xmlChar *content_title_utf8; /**< CompositionPlaylist/ContentTitle element */ - AVRational edit_rate; /**< CompositionPlaylist/EditRate element */ - FFIMFMarkerVirtualTrack *main_markers_track; /**< Main Marker Virtual Track */ - FFIMFTrackFileVirtualTrack *main_image_2d_track; /**< Main Image Virtual Track */ - uint32_t main_audio_track_count; /**< Number of Main Audio Virtual Tracks */ - FFIMFTrackFileVirtualTrack *main_audio_tracks; /**< Main Audio Virtual Tracks */ + FFIMFUUID id_uuid; /**< CompositionPlaylist/Id element */ + xmlChar *content_title_utf8; /**< CompositionPlaylist/ContentTitle element */ + AVRational edit_rate; /**< CompositionPlaylist/EditRate element */ + FFIMFMarkerVirtualTrack *main_markers_track; /**< Main Marker Virtual Track */ + FFIMFTrackFileVirtualTrack *main_image_2d_track; /**< Main Image Virtual Track */ + uint32_t main_audio_track_count; /**< Number of Main Audio Virtual Tracks */ + FFIMFTrackFileVirtualTrack *main_audio_tracks; /**< Main Audio Virtual Tracks */ } FFIMFCPL; /** diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c index 5bf1f77618..aad441a659 100644 --- a/libavformat/imfdec.c +++ b/libavformat/imfdec.c @@ -81,7 +81,7 @@ */ typedef struct IMFAssetLocator { FFIMFUUID uuid; - char *absolute_uri; + char *absolute_uri; } IMFAssetLocator; /** @@ -639,12 +639,9 @@ static IMFVirtualTrackPlaybackCtx *get_next_track_with_minimum_timestamp(AVForma for (uint32_t i = c->track_count; i > 0; i--) { av_log(s, AV_LOG_DEBUG, "Compare track %d timestamp " AVRATIONAL_FORMAT - " to minimum " AVRATIONAL_FORMAT - " (over duration: " AVRATIONAL_FORMAT - ")\n", i, - AVRATIONAL_ARG(c->tracks[i - 1]->current_timestamp), - AVRATIONAL_ARG(minimum_timestamp), - AVRATIONAL_ARG(c->tracks[i - 1]->duration)); + " to minimum " AVRATIONAL_FORMAT " (over duration: " AVRATIONAL_FORMAT ")\n", + i, AVRATIONAL_ARG(c->tracks[i - 1]->current_timestamp), + AVRATIONAL_ARG(minimum_timestamp), AVRATIONAL_ARG(c->tracks[i - 1]->duration)); if (av_cmp_q(c->tracks[i - 1]->current_timestamp, minimum_timestamp) <= 0) { track = c->tracks[i - 1]; diff --git a/libavformat/tests/imf.c b/libavformat/tests/imf.c index 142aa04261..308b1eeb84 100644 --- a/libavformat/tests/imf.c +++ b/libavformat/tests/imf.c @@ -452,17 +452,17 @@ typedef struct PathTypeTestStruct { } PathTypeTestStruct; static const PathTypeTestStruct PATH_TYPE_TEST_STRUCTS[11] = { - {.path = "file://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, - {.path = "http://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, + {.path = "file://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, + {.path = "http://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, {.path = "https://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, - {.path = "s3://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, - {.path = "ftp://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, - {.path = "/path/to/somewhere", .is_url = 0, .is_unix_absolute_path = 1, .is_dos_absolute_path = 0}, - {.path = "path/to/somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, - {.path = "C:\\path\\to\\somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 1}, - {.path = "C:/path/to/somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 1}, - {.path = "\\\\path\\to\\somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 1}, - {.path = "path\\to\\somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, + {.path = "s3://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, + {.path = "ftp://path/to/somewhere", .is_url = 1, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, + {.path = "/path/to/somewhere", .is_url = 0, .is_unix_absolute_path = 1, .is_dos_absolute_path = 0}, + {.path = "path/to/somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, + {.path = "C:\\path\\to\\somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 1}, + {.path = "C:/path/to/somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 1}, + {.path = "\\\\path\\to\\somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 1}, + {.path = "path\\to\\somewhere", .is_url = 0, .is_unix_absolute_path = 0, .is_dos_absolute_path = 0}, }; static int test_path_type_functions(void) @@ -473,27 +473,21 @@ static int test_path_type_functions(void) if (imf_uri_is_url(path_type.path) != path_type.is_url) { fprintf(stderr, "URL comparison test failed for '%s', got %d instead of expected %d\n", - path_type.path, - path_type.is_url, - !path_type.is_url); + path_type.path, path_type.is_url, !path_type.is_url); goto fail; } if (imf_uri_is_unix_abs_path(path_type.path) != path_type.is_unix_absolute_path) { fprintf(stderr, "Unix absolute path comparison test failed for '%s', got %d instead of expected %d\n", - path_type.path, - path_type.is_unix_absolute_path, - !path_type.is_unix_absolute_path); + path_type.path, path_type.is_unix_absolute_path, !path_type.is_unix_absolute_path); goto fail; } if (imf_uri_is_dos_abs_path(path_type.path) != path_type.is_dos_absolute_path) { fprintf(stderr, "DOS absolute path comparison test failed for '%s', got %d instead of expected %d\n", - path_type.path, - path_type.is_dos_absolute_path, - !path_type.is_dos_absolute_path); + path_type.path, path_type.is_dos_absolute_path, !path_type.is_dos_absolute_path); goto fail; } }