On Tue, Dec 14, 2021 at 2:33 AM Anton Khirnov <an...@khirnov.net> wrote:
>
> Quoting p...@sandflow.com (2021-12-13 06:43:35)
> > +static int parse_assetmap(AVFormatContext *s, const char *url, AVIOContext
> > *in)
>
> in is always NULL, might as well make it a local variable
Addressed by v11 of the patchset.
>
> > +{
> > + 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);
> > +
> > + ret = avio_read_to_bprint(in, &buf, MAX_BPRINT_READ_SIZE);
> > + if (ret < 0 || !avio_feof(in) || (filesize = buf.len) == 0) {
>
> Stealth assignments in if () clauses are sinful and a recipe for bugs.
> Especially when a part of several statements. Please don't.
Addressed by v11 of the patchset.
>
> > +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) {
> > + track_resource->ctx = avformat_alloc_context();
> > + if (!track_resource->ctx)
> > + return AVERROR(ENOMEM);
> > + }
> > +
> > + if (track_resource->ctx->iformat) {
> > + av_log(s,
> > + AV_LOG_DEBUG,
> > + "Input context already opened for %s.\n",
> > + track_resource->locator->absolute_uri);
> > + return 0;
> > + }
> > +
> > + 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;
> > +
> > + 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));
> > + return ret;
> > + }
> > +
> > + 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;
> > + }
>
> Is this really necessary? This might potentially take a long time and
> invoke decoders and so should be avoided unless you really need it.
Addressed by v11 of the patchset.
By restricting Track Files to MXF, I think we can avoid
avformat_find_stream_info() since the MXF demuxer fills in
streams->time_base on opening.
>
> --
> Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
