From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id D2B7841A15 for ; Wed, 16 Feb 2022 18:15:53 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 2341768B255; Wed, 16 Feb 2022 20:15:51 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 0D01A68A4C5 for ; Wed, 16 Feb 2022 20:15:45 +0200 (EET) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 0AB1A240179 for ; Wed, 16 Feb 2022 19:15:43 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id loZhhSQJN38G for ; Wed, 16 Feb 2022 19:15:41 +0100 (CET) Received: from lain.red.khirnov.net (lain.red.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.red.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 7FD14240175 for ; Wed, 16 Feb 2022 19:15:41 +0100 (CET) Received: by lain.red.khirnov.net (Postfix, from userid 1000) id 83F7F1601AD; Wed, 16 Feb 2022 19:15:37 +0100 (CET) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: <20220113020913.870-6-jamrial@gmail.com> References: <20220113015101.4-1-jamrial@gmail.com> <20220113020913.870-6-jamrial@gmail.com> Mail-Followup-To: FFmpeg development discussions and patches Date: Wed, 16 Feb 2022 19:15:37 +0100 Message-ID: <164503533751.19727.8561650103287228754@lain.red.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 275/281] avfilter: convert to new channel layout API X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Quoting James Almer (2022-01-13 03:09:07) > diff --git a/libavfilter/af_channelsplit.c b/libavfilter/af_channelsplit.c > index 1a2519dd32..31453823c5 100644 > --- a/libavfilter/af_channelsplit.c > +++ b/libavfilter/af_channelsplit.c > @@ -36,7 +36,7 @@ > typedef struct ChannelSplitContext { > const AVClass *class; > > - uint64_t channel_layout; > + AVChannelLayout channel_layout; > char *channel_layout_str; > char *channels_str; > > @@ -57,11 +57,11 @@ AVFILTER_DEFINE_CLASS(channelsplit); > static av_cold int init(AVFilterContext *ctx) > { > ChannelSplitContext *s = ctx->priv; > - uint64_t channel_layout; > + AVChannelLayout channel_layout = { 0 }; > int nb_channels; > int all = 0, ret = 0, i; > > - if (!(s->channel_layout = av_get_channel_layout(s->channel_layout_str))) { > + if ((ret = av_channel_layout_from_string(&s->channel_layout, s->channel_layout_str)) < 0) { > av_log(ctx, AV_LOG_ERROR, "Error parsing channel layout '%s'.\n", > s->channel_layout_str); > ret = AVERROR(EINVAL); > @@ -70,27 +70,32 @@ static av_cold int init(AVFilterContext *ctx) > > > if (!strcmp(s->channels_str, "all")) { > - nb_channels = av_get_channel_layout_nb_channels(s->channel_layout); > + nb_channels = s->channel_layout.nb_channels; > channel_layout = s->channel_layout; > all = 1; > } else { > - if ((ret = av_get_extended_channel_layout(s->channels_str, &channel_layout, &nb_channels)) < 0) > + if ((ret = av_channel_layout_from_string(&channel_layout, s->channels_str)) < 0) > return ret; > } > > for (i = 0; i < nb_channels; i++) { > - uint64_t channel = av_channel_layout_extract_channel(channel_layout, i); > - AVFilterPad pad = { 0 }; > + int channel = av_channel_layout_channel_from_index(&channel_layout, i); > + char buf[64]; > + AVFilterPad pad = { .flags = AVFILTERPAD_FLAG_FREE_NAME }; > > + av_channel_name(buf, sizeof(buf), channel); > pad.type = AVMEDIA_TYPE_AUDIO; > - pad.name = av_get_channel_name(channel); > + pad.name = av_strdup(buf); > + if (!pad.name) > + return AVERROR(ENOMEM); > > if (all) { > s->map[i] = i; Should either make map dynamically allocated or check that there are fewer than 64 channels in the layout. > } else { > - if ((ret = av_get_channel_layout_channel_index(s->channel_layout, channel)) < 0) { > + if ((ret = av_channel_layout_index_from_channel(&s->channel_layout, channel)) < 0) { > av_log(ctx, AV_LOG_ERROR, "Channel name '%s' not present in channel layout '%s'.\n", > - av_get_channel_name(channel), s->channel_layout_str); > + pad.name, s->channel_layout_str); > + av_freep(&pad.name); > return ret; > } > > @@ -115,15 +120,18 @@ static int query_formats(AVFilterContext *ctx) > (ret = ff_set_common_all_samplerates(ctx)) < 0) > return ret; > > - if ((ret = ff_add_channel_layout(&in_layouts, s->channel_layout)) < 0 || > + if ((ret = ff_add_channel_layout(&in_layouts, &s->channel_layout)) < 0 || > (ret = ff_channel_layouts_ref(in_layouts, &ctx->inputs[0]->outcfg.channel_layouts)) < 0) > return ret; > > for (i = 0; i < ctx->nb_outputs; i++) { > + AVChannelLayout channel_layout = { 0 }; > AVFilterChannelLayouts *out_layouts = NULL; > - uint64_t channel = av_channel_layout_extract_channel(s->channel_layout, s->map[i]); > + int channel = av_channel_layout_channel_from_index(&s->channel_layout, s->map[i]); > > - if ((ret = ff_add_channel_layout(&out_layouts, channel)) < 0 || > + if ((channel < 0) || > + (ret = av_channel_layout_from_mask(&channel_layout, 1ULL << channel)) < 0 || > + (ret = ff_add_channel_layout(&out_layouts, &channel_layout)) < 0 || > (ret = ff_channel_layouts_ref(out_layouts, &ctx->outputs[i]->incfg.channel_layouts)) < 0) > return ret; > } > @@ -139,6 +147,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf) > > for (i = 0; i < ctx->nb_outputs; i++) { > AVFrame *buf_out = av_frame_clone(buf); > + int channel = av_channel_layout_channel_from_index(&buf->ch_layout, s->map[i]); > > if (!buf_out) { > ret = AVERROR(ENOMEM); > @@ -146,9 +155,16 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *buf) > } > > buf_out->data[0] = buf_out->extended_data[0] = buf_out->extended_data[s->map[i]]; > + ret = av_channel_layout_from_mask(&buf_out->ch_layout, 1ULL << channel); potential invalid shift? > diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c > index 7362bcdab5..acb2d7db51 100644 > --- a/libavfilter/avfilter.c > +++ b/libavfilter/avfilter.c > @@ -204,6 +204,7 @@ void avfilter_link_free(AVFilterLink **link) > > ff_framequeue_free(&(*link)->fifo); > ff_frame_pool_uninit((FFFramePool**)&(*link)->frame_pool); > + av_channel_layout_uninit(&(*link)->ch_layout); > > av_freep(link); > } > @@ -405,7 +406,7 @@ void ff_tlog_link(void *ctx, AVFilterLink *link, int end) > end ? "\n" : ""); > } else { > char buf[128]; > - av_get_channel_layout_string(buf, sizeof(buf), -1, link->channel_layout); > + av_channel_layout_describe(&link->ch_layout, buf, sizeof(buf)); > > ff_tlog(ctx, > "link[%p r:%d cl:%s fmt:%s %s->%s]%s", > @@ -1036,11 +1037,21 @@ int ff_filter_frame(AVFilterLink *link, AVFrame *frame) > av_log(link->dst, AV_LOG_ERROR, "Format change is not supported\n"); > goto error; > } > - if (frame->channels != link->channels) { > +#if FF_API_OLD_CHANNEL_LAYOUT > +FF_DISABLE_DEPRECATION_WARNINGS > + if (frame->channels != link->ch_layout.nb_channels) { > av_log(link->dst, AV_LOG_ERROR, "Channel count change is not supported\n"); > goto error; > } > - if (frame->channel_layout != link->channel_layout) { > + if (frame->channel_layout && frame->channel_layout != link->channel_layout) { > + av_log(link->dst, AV_LOG_ERROR, "Channel layout change is not supported\n"); > + goto error; > + } > +FF_ENABLE_DEPRECATION_WARNINGS > + if (av_channel_layout_check(&frame->ch_layout) && av_channel_layout_compare(&frame->ch_layout, &link->ch_layout)) { > +#else > + if (av_channel_layout_compare(&frame->ch_layout, &link->ch_layout)) { > +#endif Is all this still needed? Presumably after this patch all sources make sure the old and new fields are in sync, so we could just assert here, or not check at all. > diff --git a/libavfilter/src_movie.c b/libavfilter/src_movie.c > index b89a680883..3ebd279df7 100644 > --- a/libavfilter/src_movie.c > +++ b/libavfilter/src_movie.c > @@ -189,23 +189,24 @@ static int guess_channel_layout(MovieStream *st, int st_index, void *log_ctx) > { > AVCodecParameters *dec_par = st->st->codecpar; > char buf[256]; > - int64_t chl = av_get_default_channel_layout(dec_par->channels); > + AVChannelLayout chl = { 0 }; > > - if (!chl) { > + av_channel_layout_default(&chl, dec_par->ch_layout.nb_channels); > + > + if (!KNOWN(&chl)) { > av_log(log_ctx, AV_LOG_ERROR, > "Channel layout is not set in stream %d, and could not " > "be guessed from the number of channels (%d)\n", > - st_index, dec_par->channels); > + st_index, dec_par->ch_layout.nb_channels); > return AVERROR(EINVAL); Should this still be an error? Unspec layouts should now be properly supported by (almost?) everything. -- 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-request@ffmpeg.org with subject "unsubscribe".