[FFmpeg-devel] [PATCH v7 1/2] avformat/imf: Demuxer
Pierre-Anthony Lemieux
pal at sandflow.com
Tue Dec 7 20:03:20 EET 2021
On Mon, Dec 6, 2021 at 12:55 PM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> pal at sandflow.com:
> > From: Pierre-Anthony Lemieux <pal at palemieux.com>
> >
> > Signed-off-by: Pierre-Anthony Lemieux <pal at palemieux.com>
> > ---
> >
> > Notes:
> > The IMF demuxer accepts as input an IMF CPL. The assets referenced by the CPL can be
> > contained in multiple deliveries, each defined by an ASSETMAP file:
> >
> > ffmpeg -assetmaps <path of ASSETMAP1>,<path of ASSETMAP>,... -i <path of CPL>
> >
> > If -assetmaps is not specified, FFMPEG looks for a file called ASSETMAP.xml in the same directory as the CPL.
> >
> > EXAMPLE:
> > ffmpeg -i http://ffmpeg-imf-samples-public.s3-website-us-west-1.amazonaws.com/countdown/CPL_f5095caa-f204-4e1c-8a84-7af48c7ae16b.xml out.mp4
> >
> > The Interoperable Master Format (IMF) is a file-based media format for the
> > delivery and storage of professional audio-visual masters.
> > An IMF Composition consists of an XML playlist (the Composition Playlist)
> > and a collection of MXF files (the Track Files). The Composition Playlist (CPL)
> > assembles the Track Files onto a timeline, which consists of multiple tracks.
> > The location of the Track Files referenced by the Composition Playlist is stored
> > in one or more XML documents called Asset Maps. More details at https://www.imfug.com/explainer.
> > The IMF standard was first introduced in 2013 and is managed by the SMPTE.
> >
> > CHANGE NOTES:
> >
> > - reduced line width
> > - use ff_ and FF prefixes for non-local functions and structures
> > - modified copyright header
> > - 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 and
> > ASSETMAP assets
> >
> > MAINTAINERS | 1 +
> > configure | 3 +-
> > doc/demuxers.texi | 19 +
> > libavformat/Makefile | 1 +
> > libavformat/allformats.c | 1 +
> > libavformat/imf.h | 206 +++++++++
> > libavformat/imf_cpl.c | 769 +++++++++++++++++++++++++++++++++
> > libavformat/imfdec.c | 907 +++++++++++++++++++++++++++++++++++++++
> > 8 files changed, 1906 insertions(+), 1 deletion(-)
> > create mode 100644 libavformat/imf.h
> > create mode 100644 libavformat/imf_cpl.c
> > create mode 100644 libavformat/imfdec.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index dcac46003e..7a6972fe1a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -433,6 +433,7 @@ Muxers/Demuxers:
> > idroqdec.c Mike Melanson
> > iff.c Jaikrishnan Menon
> > img2*.c Michael Niedermayer
> > + imf*.c Marc-Antoine Arnaud, Pierre-Anthony Lemieux, Valentin Noël
> > ipmovie.c Mike Melanson
> > ircam* Paul B Mahol
> > iss.c Stefan Gehrer
> > diff --git a/configure b/configure
> > index a98a18abaa..022f17767b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -298,7 +298,7 @@ External library support:
> > --enable-libxvid enable Xvid encoding via xvidcore,
> > native MPEG-4/Xvid encoder exists [no]
> > --enable-libxml2 enable XML parsing using the C library libxml2, needed
> > - for dash demuxing support [no]
> > + for dash and imf demuxing support [no]
> > --enable-libzimg enable z.lib, needed for zscale filter [no]
> > --enable-libzmq enable message passing via libzmq [no]
> > --enable-libzvbi enable teletext support via libzvbi [no]
> > @@ -3400,6 +3400,7 @@ hls_muxer_select="mpegts_muxer"
> > hls_muxer_suggest="gcrypt openssl"
> > image2_alias_pix_demuxer_select="image2_demuxer"
> > image2_brender_pix_demuxer_select="image2_demuxer"
> > +imf_demuxer_deps="libxml2"
> > ipod_muxer_select="mov_muxer"
> > ismv_muxer_select="mov_muxer"
> > ivf_muxer_select="av1_metadata_bsf vp9_superframe_bsf"
> > diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> > index cab8a7072c..614f2e754a 100644
> > --- a/doc/demuxers.texi
> > +++ b/doc/demuxers.texi
> > @@ -267,6 +267,12 @@ which streams to actually receive.
> > Each stream mirrors the @code{id} and @code{bandwidth} properties from the
> > @code{<Representation>} as metadata keys named "id" and "variant_bitrate" respectively.
> >
> > + at section imf
> > +
> > +Interoperable Master Format demuxer.
> > +
> > +This demuxer presents audio and video streams found in an IMF Composition.
> > +
> > @section flv, live_flv, kux
> >
> > Adobe Flash Video Format demuxer.
> > @@ -770,6 +776,19 @@ MJPEG stream. Turning this option on by setting it to 1 will result in a stricte
> > of the boundary value.
> > @end table
> >
> > + at section mxf
> > +
> > +MXF demuxer.
> > +
> > + at table @option
> > +
> > + at item -eia608_extract @var{bool}
> > +Extract EIA-608 captions from SMPTE 436M track
> > +
> > + at item -skip_audio_reordering @var{bool}
> > +This option will disable the audio reordering based on Multi-Channel Audio (MCA) labelling (SMPTE ST-377-4).
> > + at end table
> > +
>
> Why are you adding documentation for mxf in this patch?
Fixed at patch v8. I had applied the MCA patchset on my branch. That
patchset is necessary to process the MXF files used in IMF.
>
> > @section rawvideo
> >
> > Raw video demuxer.
> > diff --git a/libavformat/imfdec.c b/libavformat/imfdec.c
> > new file mode 100644
> > index 0000000000..ec4ad28444
> > --- /dev/null
> > +++ b/libavformat/imfdec.c
> > @@ -0,0 +1,907 @@
> > +/*
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +/*
> > + *
> > + * Copyright (c) Sandflow Consulting LLC
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are met:
> > + *
> > + * * Redistributions of source code must retain the above copyright notice, this
> > + * list of conditions and the following disclaimer.
> > + * * Redistributions in binary form must reproduce the above copyright notice,
> > + * this list of conditions and the following disclaimer in the documentation
> > + * and/or other materials provided with the distribution.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + */
> > +
> > +/**
> > + * Demuxes an IMF Composition
> > + *
> > + * References
> > + * OV 2067-0:2018 - SMPTE Overview Document - Interoperable Master Format
> > + * ST 2067-2:2020 - SMPTE Standard - Interoperable Master Format — Core Constraints
> > + * ST 2067-3:2020 - SMPTE Standard - Interoperable Master Format — Composition Playlist
> > + * ST 2067-5:2020 - SMPTE Standard - Interoperable Master Format — Essence Component
> > + * ST 2067-20:2016 - SMPTE Standard - Interoperable Master Format — Application #2
> > + * ST 2067-21:2020 - SMPTE Standard - Interoperable Master Format — Application #2 Extended
> > + * ST 2067-102:2017 - SMPTE Standard - Interoperable Master Format — Common Image Pixel Color Schemes
> > + * ST 429-9:2007 - SMPTE Standard - D-Cinema Packaging — Asset Mapping and File Segmentation
> > + *
> > + * @author Marc-Antoine Arnaud
> > + * @author Valentin Noel
> > + * @author Nicholas Vanderzwet
> > + * @file
> > + * @ingroup lavu_imf
> > + */
> > +
> > +#include "imf.h"
> > +#include "internal.h"
> > +#include "libavutil/avstring.h"
> > +#include "libavutil/bprint.h"
> > +#include "libavutil/opt.h"
> > +#include "mxf.h"
> > +#include "url.h"
> > +#include <inttypes.h>
> > +#include <libxml/parser.h>
> > +
> > +#define MAX_BPRINT_READ_SIZE (UINT_MAX - 1)
> > +#define DEFAULT_ASSETMAP_SIZE 8 * 1024
> > +#define AVRATIONAL_FORMAT "%d/%d"
> > +#define AVRATIONAL_ARG(rational) rational.num, rational.den
> > +
> > +/**
> > + * IMF Asset locator
> > + */
> > +typedef struct IMFAssetLocator {
> > + FFUUID uuid;
> > + char *absolute_uri;
> > +} IMFAssetLocator;
> > +
> > +/**
> > + * IMF Asset locator map
> > + * Results from the parsing of one or more ASSETMAP XML files
> > + */
> > +typedef struct IMFAssetLocatorMap {
> > + uint32_t asset_count;
> > + IMFAssetLocator *assets;
> > +} IMFAssetLocatorMap;
> > +
> > +typedef struct IMFVirtualTrackResourcePlaybackCtx {
> > + IMFAssetLocator *locator;
> > + FFIMFTrackFileResource *resource;
> > + AVFormatContext *ctx;
> > +} IMFVirtualTrackResourcePlaybackCtx;
> > +
> > +typedef struct IMFVirtualTrackPlaybackCtx {
> > + // Track index in playlist
> > + int32_t index;
> > + // Time counters
> > + AVRational current_timestamp;
> > + AVRational duration;
> > + // Resources
> > + unsigned int resource_count;
> > + unsigned int resources_alloc_sz;
> > + IMFVirtualTrackResourcePlaybackCtx *resources;
> > + // Decoding cursors
> > + uint32_t current_resource_index;
> > + int64_t last_pts;
> > +} 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;
> > + unsigned int track_count;
> > + IMFVirtualTrackPlaybackCtx **tracks;
> > +} IMFContext;
> > +
> > +static int imf_uri_is_url(const char *string)
> > +{
> > + char *substr = strstr(string, "://");
>
> Just because strstr() is not const-correct does not mean that you should
> be, too. Use const char *substr.
Addressed at patch v8. The imf_uri_is* functions have been simplified.
>
> > + return substr != NULL;
> > +}
> > +
> > +static int imf_uri_is_unix_abs_path(const char *string)
> > +{
> > + char *substr = strstr(string, "/");
>
> This is equivalent to strchr(string, '/').
Addressed at patch v8. The imf_uri_is* functions have been simplified.
>
> > + int index = (int)(substr - string);
>
> This might be undefined behaviour string is long (pointer->integer
> conversions are UB if the result can't be represented in the destination
> type).
Addressed at patch v8. The imf_uri_is* functions have been simplified.
>
> > + return index == 0;
>
> This makes the whole function equivalent to "return string[0] == '/';".
Addressed at patch v8. The imf_uri_is* functions have been simplified.
>
> > +}
> > +
> > +static int imf_uri_is_dos_abs_path(const char *string)
> > +{
> > + // Absolute path case: `C:\path\to\somwhere`
> > + char *substr = strstr(string, ":\\");
> > + int index = (int)(substr - string);
> > + if (index == 1)
> > + return 1;
> > +
> > + // Absolute path case: `C:/path/to/somwhere`
> > + substr = strstr(string, ":/");
> > + index = (int)(substr - string);
> > + if (index == 1)
> > + return 1;
> > +
> > + // Network path case: `\\path\to\somwhere`
> > + substr = strstr(string, "\\\\");
> > + index = (int)(substr - string);
> > + return index == 0;
> > +}
> > +
> > +/**
> > + * Parse a ASSETMAP XML file to extract the UUID-URI mapping of assets.
> > + * @param s the current format context, if any (can be NULL).
> > + * @param doc the XML document to be parsed.
> > + * @param asset_map pointer on the IMFAssetLocatorMap to fill.
> > + * @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,
> > + IMFAssetLocatorMap *asset_map,
> > + const char *base_url)
> > +{
> > + xmlNodePtr asset_map_element = NULL;
> > + xmlNodePtr node = NULL;
> > + xmlNodePtr asset_element = NULL;
> > + char *uri;
> > + int ret = 0;
> > + IMFAssetLocator *asset = NULL;
> > +
> > + asset_map_element = xmlDocGetRootElement(doc);
> > +
> > + if (!asset_map_element) {
> > + av_log(s, AV_LOG_ERROR, "Unable to parse asset map XML - missing root node\n");
> > + return AVERROR_INVALIDDATA;
> > + }
> > +
> > + 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);
> > + return AVERROR_INVALIDDATA;
> > + }
> > +
> > + // parse asset locators
> > +
> > + if (!(node = ff_xml_get_child_element_by_name(asset_map_element, "AssetList"))) {
> > + 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));
>
> Leak in case of error: All the absolute_uri strings already contained in
> the array will leak.
Addressed at patch v8. I have gone through the codebase and made sure
that arrays that contain elements referencing heap objects are not
deleted by failed calls to realloc().
>
> > + if (!asset_map->assets) {
> > + av_log(NULL, AV_LOG_PANIC, "Cannot allocate IMF asset locators\n");
> > + return AVERROR(ENOMEM);
> > + }
> > + asset_map->asset_count = 0;
>
> And this implicitly leaks the absolute_uri strings even on success (only
> in case there were any such strings, i.e. if the loop in
> imf_read_header() (that loops over parse_assetmap()) is actually a
> loop). They will either be overwritten below or just not be freed at the
> end.
Addressed at patch v8.
>
> > + asset_element = xmlFirstElementChild(node);
> > + while (asset_element) {
> > + if (av_strcasecmp(asset_element->name, "Asset") != 0)
> > + continue;
> > +
> > + asset = &(asset_map->assets[asset_map->asset_count]);
> > +
> > + if (ff_xml_read_UUID(ff_xml_get_child_element_by_name(asset_element, "Id"), asset->uuid)) {
> > + av_log(s, AV_LOG_ERROR, "Could not parse UUID from asset in asset map.\n");
> > + 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(asset_element, "ChunkList"))) {
> > + av_log(s, AV_LOG_ERROR, "Unable to parse asset map XML - missing ChunkList node\n");
> > + 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");
> > + return AVERROR_INVALIDDATA;
> > + }
> > +
> > + uri = xmlNodeGetContent(ff_xml_get_child_element_by_name(node, "Path"));
> > + if (!imf_uri_is_url(uri) && !imf_uri_is_unix_abs_path(uri) && !imf_uri_is_dos_abs_path(uri))
> > + asset->absolute_uri = av_append_path_component(base_url, uri);
> > + else
> > + asset->absolute_uri = av_strdup(uri);
> > + xmlFree(uri);
> > + if (!asset->absolute_uri) {
> > + av_log(NULL, AV_LOG_PANIC, "Cannot allocate asset locator absolute URI\n");
> > + return AVERROR(ENOMEM);
> > + }
> > +
> > + av_log(s, AV_LOG_DEBUG, "Found asset absolute URI: %s\n", asset->absolute_uri);
> > +
> > + asset_map->asset_count++;
> > + asset_element = xmlNextElementSibling(asset_element);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * Allocate a IMFAssetLocatorMap pointer and return it.
> > + * @return the allocated IMFAssetLocatorMap pointer.
> > + */
> > +static IMFAssetLocatorMap *imf_asset_locator_map_alloc(void)
> > +{
> > + IMFAssetLocatorMap *asset_map;
> > +
> > + asset_map = av_malloc(sizeof(IMFAssetLocatorMap));
> > + if (!asset_map)
> > + return NULL;
> > +
> > + asset_map->assets = NULL;
> > + asset_map->asset_count = 0;
>
> This function could just be replaced by av_mallocz(). And if you did
> this, this whole function would become superfluous.
> But it is superfluous anyway, see below.
Addressed at patch v8. imf_asset_locator_map_alloc() no longer
allocates heap for an IMFAssetLocatorMap structure but just
initializes it.
>
> > + return asset_map;
> > +}
> > +
> > +/**
> > + * Free a IMFAssetLocatorMap pointer.
> > + */
> > +static void imf_asset_locator_map_free(IMFAssetLocatorMap *asset_map)
> > +{
> > + if (!asset_map)
> > + return;
> > +
> > + for (int i = 0; i < asset_map->asset_count; ++i) {
> > + av_free(asset_map->assets[i].absolute_uri);
> > + }
> > +
> > + av_freep(&asset_map->assets);
> > + av_freep(&asset_map);
>
> This is bad style: You are only resetting your local variable pointer to
> NULL, but you leave the pointer in the IMFContext.
Addressed at patch v8.
>
> > +}
> > +
> > +static int parse_assetmap(AVFormatContext *s, const char *url, AVIOContext *in)
> > +{
> > + IMFContext *c = s->priv_data;
> > + struct AVBPrint buf;
> > + AVDictionary *opts = NULL;
> > + xmlDoc *doc = NULL;
> > + const char *base_url;
> > + char *tmp_str = NULL;
> > + int close_in = 0;
> > + int ret;
> > + int64_t filesize;
> > +
> > + av_log(s, AV_LOG_DEBUG, "Asset Map URL: %s\n", url);
> > +
> > + if (!in) {
> > + close_in = 1;
> > +
> > + av_dict_copy(&opts, c->avio_opts, 0);
> > + ret = s->io_open(s, &in, url, AVIO_FLAG_READ, &opts);
> > + av_dict_free(&opts);
> > + if (ret < 0)
> > + return ret;
> > + }
> > +
> > + filesize = avio_size(in);
> > + filesize = filesize > 0 ? filesize : DEFAULT_ASSETMAP_SIZE;
> > +
> > + av_bprint_init(&buf, filesize + 1, AV_BPRINT_SIZE_UNLIMITED);
>
> The AVBPrint API is (currently) limited to unsigned values, ergo it may
> very well be that filesize + 1 can't be represented in an unsigned. That
> being said, I don't know how big asset map files can get.
ASSETMAP files should be smaller than 10 MB in practice.
>
> > +
> > + ret = avio_read_to_bprint(in, &buf, MAX_BPRINT_READ_SIZE);
> > + if (ret < 0 || !avio_feof(in) || (filesize = buf.len) == 0) {
> > + av_log(s, AV_LOG_ERROR, "Unable to read to asset map '%s'\n", url);
> > + if (ret == 0)
> > + ret = AVERROR_INVALIDDATA;
>
> You could just goto cleanup below here; this would save a level of
> indentation below.
Addressed at patch v8.
>
> > + } else {
> > + LIBXML_TEST_VERSION
> > +
> > + tmp_str = strdup(url);
>
> We have av_strdup() (although I don't really know whether this is any
> better than ordinary strdup()).
>
> > + if (!tmp_str) {
> > + av_log(s, AV_LOG_PANIC, "Unable to duplicate string\n");
>
> Excessive logging; also use AV_LOG_ERROR, AV_LOG_PANIC is used for
> "Something went really wrong and we will crash now."
Addressed at patch v8. Replaced all AV_LOG_PANIC with AV_LOG_ERROR.
>
> > + ret = AVERROR(ENOMEM);
> > + goto clean_up;
> > + }
> > + base_url = av_dirname(tmp_str);
> > + if (c->asset_locator_map == NULL) {
> > + c->asset_locator_map = imf_asset_locator_map_alloc();
>
> c->asset_locator_map will only ever have one element; in other words,
> there is no need to allocate it separately at all: Just put it in
> IMFContext.
Addressed at patch v8.
>
> > + if (!c->asset_locator_map) {
> > + av_log(s, AV_LOG_PANIC, "Unable to allocate asset map locator\n");
> > + ret = AVERROR(ENOMEM);
> > + goto clean_up;
> > + }
> > + }
> > +
> > + doc = xmlReadMemory(buf.str, filesize, url, NULL, 0);
> > +
> > + if (!(ret = parse_imf_asset_map_from_xml_dom(s, doc, c->asset_locator_map, base_url)))
> > + av_log(s,
> > + AV_LOG_DEBUG,
> > + "Found %d assets from %s\n",
> > + c->asset_locator_map->asset_count,
> > + url);
> > +
> > + xmlFreeDoc(doc);
> > + }
> > +
> > +clean_up:
> > + if (tmp_str)
> > + free(tmp_str);
> > + if (close_in)
> > + avio_close(in);
> > + av_bprint_finalize(&buf, NULL);
> > +
> > + return ret;
> > +}
> > +
> > +static IMFAssetLocator *find_asset_map_locator(IMFAssetLocatorMap *asset_map, FFUUID uuid)
> > +{
> > + 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;
> > +}
> > +
> > +static int open_track_resource_context(AVFormatContext *s,
> > + IMFVirtualTrackResourcePlaybackCtx *track_resource)
> > +{
> > + IMFContext *c = s->priv_data;
> > + int ret = 0;
> > + int64_t entry_point;
> > + AVDictionary *opts = NULL;
> > +
> > + if (!track_resource->ctx) {
>
> The only way for ctx to be set here is if the context is in an
> inconsistent state (see below).
track_resource->ctx can be released in
get_resource_context_for_timestamp() by a call to
avformat_close_input().
>
> > + track_resource->ctx = avformat_alloc_context();
> > + if (!track_resource->ctx) {
> > + av_log(NULL, AV_LOG_PANIC, "Cannot allocate Track Resource Context\n");
> > + return AVERROR(ENOMEM);
> > + }
> > + }
> > +
> > + if (track_resource->ctx->iformat) {
>
> In particular, this branch here can't happen unless you are in an
> inconsistent state in which the above check was a use-after-free.
>
> > + av_log(s,
> > + AV_LOG_DEBUG,
> > + "Input context already opened for %s.\n",
> > + track_resource->locator->absolute_uri);
> > + return ret;
>
> return 0
Addressed at patch v8.
>
> > + }
> > +
> > + track_resource->ctx->io_open = s->io_open;
> > + track_resource->ctx->io_close = s->io_close;
> > + track_resource->ctx->flags |= s->flags & ~AVFMT_FLAG_CUSTOM_IO;
> > +
> > + if ((ret = ff_copy_whiteblacklists(track_resource->ctx, s)) < 0)
> > + goto cleanup;
>
> avformat_close_input() may only be called if avformat_open_input() has
> been called; without this, you must call avformat_free_context().
Clean-up calls avformat_free_context().
>
> > +
> > + av_dict_copy(&opts, c->avio_opts, 0);
> > + ret = avformat_open_input(&track_resource->ctx,
> > + track_resource->locator->absolute_uri,
> > + NULL,
> > + &opts);
> > + av_dict_free(&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));
> > + goto cleanup;
>
> You can return directly here, as avformat_open_input() frees the ctx on
> failure.
Addressed at patch v8.
>
> > + }
> > +
> > + ret = avformat_find_stream_info(track_resource->ctx, NULL);
> > + if (ret < 0) {
> > + av_log(s,
> > + AV_LOG_ERROR,
> > + "Could not find %s stream information: %s\n",
> > + track_resource->locator->absolute_uri,
> > + av_err2str(ret));
> > + goto cleanup;
> > + }
> > +
> > + // Compare the source timebase to the resource edit rate, considering the first stream of the source file
> > + 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);
> > +
> > + entry_point = (int64_t)track_resource->resource->base.entry_point
> > + * track_resource->resource->base.edit_rate.den
> > + * AV_TIME_BASE
> > + / track_resource->resource->base.edit_rate.num;
> > +
> > + if (entry_point) {
> > + av_log(s,
> > + AV_LOG_DEBUG,
> > + "Seek at resource %s entry point: %ld\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));
> > + goto cleanup;
> > + }
> > + }
> > +
> > + return ret;
> > +cleanup:
> > + avformat_free_context(track_resource->ctx);
>
> This will not reset track_resource->ctx to NULL, leaving track_resource
> in an inconsistent state. This matters when called from
> get_resource_context_for_timestamp() (but not for
> open_track_file_resource() where track_resource lives on the stack).
Addressed at patch v8. track_resource->ctx set to NULL after
avformat_free_context().
>
> > + return ret;
> > +}
> > +
> > +static int open_track_file_resource(AVFormatContext *s,
> > + FFIMFTrackFileResource *track_file_resource,
> > + IMFVirtualTrackPlaybackCtx *track)
> > +{
> > + IMFContext *c = s->priv_data;
> > + IMFAssetLocator *asset_locator;
> > + IMFVirtualTrackResourcePlaybackCtx vt_ctx;
> > + int ret;
> > +
> > + 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_UUID_FORMAT "\n",
> > + UID_ARG(track_file_resource->track_file_uuid));
> > + return AVERROR_INVALIDDATA;
> > + }
> > +
> > + av_log(s,
> > + AV_LOG_DEBUG,
> > + "Found locator for " FF_UUID_FORMAT ": %s\n",
> > + UID_ARG(asset_locator->uuid),
> > + asset_locator->absolute_uri);
> > +
> > + track->resources = av_fast_realloc(track->resources,
> > + &track->resources_alloc_sz,
> > + (track->resource_count + track_file_resource->base.repeat_count)
> > + * sizeof(IMFVirtualTrackResourcePlaybackCtx));
>
> Leak on error.
Addressed at patch v8.
>
> > + if (!track->resources) {
> > + av_log(NULL, AV_LOG_PANIC, "Cannot allocate Virtual Track playback context\n");
> > + return AVERROR(ENOMEM);
> > + }
> > +
> > + for (int i = 0; i < track_file_resource->base.repeat_count; ++i) {
> > + 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));
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int open_virtual_track(AVFormatContext *s,
> > + FFIMFTrackFileVirtualTrack *virtual_track,
> > + int32_t track_index)
> > +{
> > + IMFContext *c = s->priv_data;
> > + IMFVirtualTrackPlaybackCtx *track;
> > + int ret = 0;
> > +
> > + track = av_mallocz(sizeof(IMFVirtualTrackPlaybackCtx));
> > + if (!track) {
> > + av_log(NULL, AV_LOG_PANIC, "Cannot allocate IMF Virtual Track Playback context\n");
>
> These malloc-failure messages are excessive: Just forward the error code.
Addressed at patch v8. I have removed the error message in the case of
unlikely malloc failures, but kept it when the malloc failure might
point to unreasonably large files.
>
> > + return AVERROR(ENOMEM);
> > + }
> > + track->index = track_index;
> > + track->duration = av_make_q(0, 1);
> > +
> > + for (int i = 0; i < virtual_track->resource_count; i++) {
>
> resource_count is unsigned, so the loop variable should be, too.
> This is also true for other loop variables.
Addressed at patch v8.
>
> > + av_log(s,
> > + AV_LOG_DEBUG,
> > + "Open stream from file " FF_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_UUID_FORMAT "\n",
> > + UID_ARG(virtual_track->resources[i].track_file_uuid));
> > + return ret;
>
> track leaks here.
Addressed at patch v8.
>
> > + }
> > + }
> > +
> > + track->current_timestamp = av_make_q(0, track->duration.den);
> > +
> > + c->tracks = av_realloc_f(c->tracks, c->track_count + 1, sizeof(IMFVirtualTrackPlaybackCtx));
> > + if (!c->tracks) {
>
> track leaks here as well as all the c->track_count entries of c->tracks
> and everything that is in them. In general, this means that you can't
> use av_realloc_f() for non-POD structures.
> Furthermore, in this scenario the current code produces c->tracks ==
> NULL with c->track_count > 0, which will lead to a crash in imf_close().
Addressed at patch v8.
>
> > + av_log(NULL, AV_LOG_PANIC, "Cannot allocate Virtual Track playback context\n");
> > + return AVERROR(ENOMEM);
> > + }
> > + c->tracks[c->track_count++] = track;
> > +
> > + return ret;
> > +}
> > +
> > +static void imf_virtual_track_playback_context_free(IMFVirtualTrackPlaybackCtx *track)
> > +{
> > + if (!track)
>
> This check would be redundant if track_count were trustworthy (which it
> should be).
Addressed at patch v8.
>
> > + return;
> > +
> > + for (int i = 0; i < track->resource_count; ++i)
> > + if (track->resources[i].ctx && track->resources[i].ctx->iformat) {
> > + avformat_close_input(&(track->resources[i].ctx));
> > + track->resources[i].ctx = NULL;
>
> avformat_close_input()
Addressed at patch v8.
>
> > + }
> > +
> > + av_freep(&(track->resources));
> > +}
> > +
> > +static int set_context_streams_from_tracks(AVFormatContext *s)
> > +{
> > + IMFContext *c = s->priv_data;
> > +
> > + AVStream *asset_stream;
> > + AVStream *first_resource_stream;
> > +
> > + int ret = 0;
> > +
> > + for (int i = 0; i < c->track_count; ++i) {
> > + // Open the first resource of the track to get stream information
> > + first_resource_stream = c->tracks[i]->resources[0].ctx->streams[0];
> > +
> > + av_log(s, AV_LOG_DEBUG, "Open the first resource of track %d\n", c->tracks[i]->index);
> > +
> > + // Copy stream information
> > + asset_stream = avformat_new_stream(s, NULL);
>
> Unchecked allocation.
Addressed at patch v8.
>
> > + 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");
> > + break;
> > + }
> > + 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)));
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int save_avio_options(AVFormatContext *s)
>
> This whole function is just duplicated from hls and dashdec; it is bad
> enough to have two of these. A third is not acceptable.
Ok. Happy to create a separate patch/start a separate thread. Do you
have suggestion on where the common code should go?
>
> > +{
> > + IMFContext *c = s->priv_data;
> > + static const char *const opts[] = {
> > + "headers", "http_proxy", "user_agent", "cookies", "referer", "rw_timeout", "icy", NULL};
> > + const char *const *opt = opts;
> > + uint8_t *buf;
> > + int ret = 0;
> > +
> > + while (*opt) {
> > + if (av_opt_get(s->pb, *opt, AV_OPT_SEARCH_CHILDREN | AV_OPT_ALLOW_NULL, &buf) >= 0) {
> > + ret = av_dict_set(&c->avio_opts, *opt, buf, AV_DICT_DONT_STRDUP_VAL);
> > + if (ret < 0)
> > + return ret;
> > + }
> > + opt++;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int open_cpl_tracks(AVFormatContext *s)
> > +{
> > + IMFContext *c = s->priv_data;
> > + int32_t track_index = 0;
> > + int ret;
> > +
> > + 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_UUID_FORMAT "\n",
> > + UID_ARG(c->cpl->main_image_2d_track->base.id_uuid));
> > + return ret;
> > + }
> > +
> > + for (int 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,
> > + "Could not open audio track " FF_UUID_FORMAT "\n",
> > + UID_ARG(c->cpl->main_audio_tracks[i].base.id_uuid));
> > + return ret;
> > + }
> > +
> > + return set_context_streams_from_tracks(s);
> > +}
> > +
> > +static int imf_close(AVFormatContext *s);
> > +
> > +static int imf_read_header(AVFormatContext *s)
> > +{
> > + IMFContext *c = s->priv_data;
> > + char *asset_map_path;
> > + int ret;
> > +
> > + c->interrupt_callback = &s->interrupt_callback;
> > + c->base_url = av_dirname(av_strdup(s->url));
>
> Unchecked allocation.
Addressed at patch v8.
>
> > + if ((ret = save_avio_options(s)) < 0)
> > + goto fail;
> > +
> > + av_log(s, AV_LOG_DEBUG, "start parsing IMF CPL: %s\n", s->url);
> > +
> > + if ((ret = ff_parse_imf_cpl(s->pb, &c->cpl)) < 0)
> > + goto fail;
> > +
> > + av_log(s,
> > + AV_LOG_DEBUG,
> > + "parsed IMF CPL: " FF_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");
> > +
> > + // Parse each asset map XML file
> > + asset_map_path = strtok(c->asset_map_paths, ",");
> > + while (asset_map_path != NULL) {
> > + av_log(s, AV_LOG_DEBUG, "start parsing IMF Asset Map: %s\n", asset_map_path);
> > +
> > + if ((ret = parse_assetmap(s, asset_map_path, NULL)) < 0)
> > + goto fail;
> > +
> > + asset_map_path = strtok(NULL, ",");
>
> av_strtok(): strtok is not thread-safe.
Addressed at patch v8.
>
> > + }
> > +
> > + av_log(s, AV_LOG_DEBUG, "parsed IMF Asset Maps\n");
> > +
> > + if ((ret = open_cpl_tracks(s)) != 0)
> > + goto fail;
> > +
> > + av_log(s, AV_LOG_DEBUG, "parsed IMF package\n");
> > + return ret;
>
> return 0;
Addressed at patch v8.
>
> > +
> > +fail:
> > + imf_close(s);
> > + return ret;
>
> This and the forward declaration of imf_close() can be removed with the
> FF_FMT_INIT_CLEANUP flag.
Addressed at patch v8.
>
> > +}
> > +
> > +static IMFVirtualTrackPlaybackCtx *get_next_track_with_minimum_timestamp(AVFormatContext *s)
> > +{
> > + IMFContext *c = s->priv_data;
> > + IMFVirtualTrackPlaybackCtx *track;
> > +
> > + AVRational minimum_timestamp = av_make_q(INT32_MAX, 1);
> > + for (int i = 0; i < c->track_count; ++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]->current_timestamp),
> > + AVRATIONAL_ARG(minimum_timestamp),
> > + AVRATIONAL_ARG(c->tracks[i]->duration));
> > + if (av_cmp_q(c->tracks[i]->current_timestamp, minimum_timestamp) < 0) {
> > + track = c->tracks[i];
> > + minimum_timestamp = track->current_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;
> > +}
> > +
> > +static IMFVirtualTrackResourcePlaybackCtx *get_resource_context_for_timestamp(AVFormatContext *s,
> > + IMFVirtualTrackPlaybackCtx *track)
> > +{
> > + 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 (int 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,
> > + "Found resource %d in track %d to read for timestamp %lf "
> > + "(on cumulated=%lf): entry=%ld, duration=%lu, editrate=" AVRATIONAL_FORMAT
> > + " | edit_unit_duration=%lf\n",
> > + 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);
> > + avformat_close_input(&(track->resources[track->current_resource_index].ctx));
> > + if (open_track_resource_context(s, &(track->resources[i])) != 0)
> > + return NULL;
> > + track->current_resource_index = i;
> > + }
> > + return &(track->resources[track->current_resource_index]);
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > +static int ff_imf_read_packet(AVFormatContext *s, AVPacket *pkt)
>
> Why do you use the prefix for non-static, library internal functions?
Addressed at patch v8. Probably a refactoring error.
>
> > +{
> > + IMFContext *c = s->priv_data;
> > +
> > + IMFVirtualTrackResourcePlaybackCtx *resource_to_read = NULL;
> > + AVRational edit_unit_duration;
> > + int ret = 0;
> > +
> > + IMFVirtualTrackPlaybackCtx *track = get_next_track_with_minimum_timestamp(s);
> > + FFStream *track_stream;
> > +
> > + if (av_cmp_q(track->current_timestamp, track->duration) == 0)
> > + return AVERROR_EOF;
> > +
> > + resource_to_read = get_resource_context_for_timestamp(s, track);
> > +
> > + if (!resource_to_read) {
> > + edit_unit_duration = av_inv_q(track->resources[track->current_resource_index].resource->base.edit_rate);
> > + if (av_cmp_q(av_add_q(track->current_timestamp, edit_unit_duration),
> > + track->duration)
> > + > 0)
> > + return AVERROR_EOF;
> > +
> > + av_log(s, AV_LOG_ERROR, "Could not find IMF track resource to read\n");
> > + return AVERROR_STREAM_NOT_FOUND;
> > + }
> > +
> > + 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);
> > + track_stream = ffstream(s->streams[track->index]);
> > + if (ret >= 0) {
> > + // Update packet info from track
> > + if (pkt->dts < track_stream->cur_dts && track->last_pts > 0)
> > + pkt->dts = track_stream->cur_dts;
> > +
> > + pkt->pts = track->last_pts;
> > + pkt->dts = pkt->dts
> > + - (int64_t)track->resources[track->current_resource_index].resource->base.entry_point;
> > + pkt->stream_index = track->index;
> > +
> > + // Update track cursors
> > + track->current_timestamp = av_add_q(track->current_timestamp,
> > + av_make_q((int)pkt->duration * resource_to_read->ctx->streams[0]->time_base.num,
> > + resource_to_read->ctx->streams[0]->time_base.den));
> > + track->last_pts += pkt->duration;
> > +
> > + 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));
> > + return ret;
> > + }
> > + }
> > +
> > + return AVERROR_EOF;
> > +}
> > +
> > +static int imf_close(AVFormatContext *s)
> > +{
> > + IMFContext *c = s->priv_data;
> > +
> > + av_log(s, AV_LOG_DEBUG, "Close IMF package\n");
> > + av_dict_free(&c->avio_opts);
> > + av_freep(&c->base_url);
> > + imf_asset_locator_map_free(c->asset_locator_map);
> > + ff_imf_cpl_free(c->cpl);
> > +
> > + for (int i = 0; i < c->track_count; ++i) {
> > + imf_virtual_track_playback_context_free(c->tracks[i]);
> > + av_freep(&(c->tracks[i]));
> > + }
> > +
> > + av_freep(&(c->tracks));
> > +
> > + return 0;
> > +}
> > +
> > +static const AVOption imf_options[] = {
> > + {"assetmaps",
> > + "IMF CPL-related asset map comma-separated absolute paths. "
> > + "If not specified, the CPL sibling `ASSETMAP.xml` file is used.",
> > + offsetof(IMFContext, asset_map_paths),
> > + AV_OPT_TYPE_STRING,
> > + {.str = NULL},
> > + 0,
> > + 0,
> > + AV_OPT_FLAG_DECODING_PARAM},
> > + {NULL}};
>
> This is the weirdest way of specifying AVOptions I have ever seen. One
> does not even see that the final '}' does indeed close the first '{’.
> If you don't want to put everything into one gigantic line (as is
> common, but has its own drawbacks), then use designated initializers and
> don't initialize elements to values that they would have anyway (e.g.
> the zeros for min/max are unnecessary).
Addressed at patch v8.
>
> > +
> > +static const AVClass imf_class = {
> > + .class_name = "imf",
> > + .item_name = av_default_item_name,
> > + .option = imf_options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +
> > +const AVInputFormat ff_imf_demuxer = {
> > + .name = "imf",
> > + .long_name = NULL_IF_CONFIG_SMALL("IMF (Interoperable Master Format)"),
> > + .priv_class = &imf_class,
> > + .priv_data_size = sizeof(IMFContext),
> > + .read_header = imf_read_header,
> > + .read_packet = ff_imf_read_packet,
> > + .read_close = imf_close,
> > + .extensions = "xml",
> > + .mime_type = "application/xml,text/xml",
>
> Align this and the AVClass on '='.
Addressed at patch v8.
>
> > +};
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list