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 11E35411A6 for ; Tue, 15 Feb 2022 11:50:14 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3D46368B2AF; Tue, 15 Feb 2022 13:50:12 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 03A6768B1BA for ; Tue, 15 Feb 2022 13:50:06 +0200 (EET) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 7AF29240179 for ; Tue, 15 Feb 2022 12:50:05 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id kj4IoW9Y7Xvh for ; Tue, 15 Feb 2022 12:50:04 +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 94CA2240175 for ; Tue, 15 Feb 2022 12:50:04 +0100 (CET) Received: by lain.red.khirnov.net (Postfix, from userid 1000) id D068F1601AD; Tue, 15 Feb 2022 12:50:04 +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: Tue, 15 Feb 2022 12:50:04 +0100 Message-ID: <164492580482.19727.5709579514076305797@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_aformat.c b/libavfilter/af_aformat.c > index ed3c75311a..96704e041c 100644 > --- a/libavfilter/af_aformat.c > +++ b/libavfilter/af_aformat.c > @@ -104,9 +104,36 @@ static av_cold int init(AVFilterContext *ctx) > ff_add_format, av_get_sample_fmt, AV_SAMPLE_FMT_NONE, "sample format"); > PARSE_FORMATS(s->sample_rates_str, int, s->sample_rates, ff_add_format, > get_sample_rate, 0, "sample rate"); > - PARSE_FORMATS(s->channel_layouts_str, uint64_t, s->channel_layouts, > - ff_add_channel_layout, av_get_channel_layout, 0, > - "channel layout"); > + { > + AVChannelLayout fmt = { 0 }; > + const char *cur = s->channel_layouts_str; > + int ret; > + > + if (s->channel_layouts_str && strchr(s->channel_layouts_str, ',')) { > + av_log(ctx, AV_LOG_WARNING, "This syntax is deprecated, use '|' to " > + "separate channel layout.\n"); It might be unclear to the user what "this syntax" refers to, maybe make it "Using ',' to separate channel layouts is deprecated" > diff --git a/libavfilter/af_biquads.c b/libavfilter/af_biquads.c > index ee42b2a034..2a0747a037 100644 > --- a/libavfilter/af_biquads.c > +++ b/libavfilter/af_biquads.c > @@ -125,7 +125,7 @@ typedef struct BiquadsContext { > double frequency; > double width; > double mix; > - uint64_t channels; > + AVChannelLayout ch_layout; > int normalize; > int order; > > @@ -716,11 +716,11 @@ static int config_filter(AVFilterLink *outlink, int reset) > s->b2 *= factor; > } > > - s->cache = av_realloc_f(s->cache, sizeof(ChanCache), inlink->channels); > + s->cache = av_realloc_f(s->cache, sizeof(ChanCache), inlink->ch_layout.nb_channels); > if (!s->cache) > return AVERROR(ENOMEM); > if (reset) > - memset(s->cache, 0, sizeof(ChanCache) * inlink->channels); > + memset(s->cache, 0, sizeof(ChanCache) * inlink->ch_layout.nb_channels); > > switch (s->transform_type) { > case DI: > @@ -838,12 +838,14 @@ static int filter_channel(AVFilterContext *ctx, void *arg, int jobnr, int nb_job > AVFrame *buf = td->in; > AVFrame *out_buf = td->out; > BiquadsContext *s = ctx->priv; > - const int start = (buf->channels * jobnr) / nb_jobs; > - const int end = (buf->channels * (jobnr+1)) / nb_jobs; > + const int start = (buf->ch_layout.nb_channels * jobnr) / nb_jobs; > + const int end = (buf->ch_layout.nb_channels * (jobnr+1)) / nb_jobs; > int ch; > > for (ch = start; ch < end; ch++) { > - if (!((av_channel_layout_extract_channel(inlink->channel_layout, ch) & s->channels))) { > + if (!(av_channel_layout_channel_from_index(&inlink->ch_layout, ch) >= 0 && > + av_channel_layout_channel_from_index(&s->ch_layout, ch) >= 0)) { This doesn't look right. The original code tests whether the channel with index ch in inlink->channel_layout is present in s->channels. The new code tests whether the channel with index ch exists in both inlink->ch_layout and s->ch_layout (the test is also broken, because it should compare against AV_CHAN_NONE). Same applies to af_speechnorm > diff --git a/libavfilter/af_headphone.c b/libavfilter/af_headphone.c > index b2030dbbbb..2fe36368c1 100644 > --- a/libavfilter/af_headphone.c > +++ b/libavfilter/af_headphone.c > @@ -85,13 +85,13 @@ typedef struct HeadphoneContext { > uint64_t mapping[64]; > } HeadphoneContext; > > -static int parse_channel_name(const char *arg, uint64_t *rchannel) > +static int parse_channel_name(const char *arg, int *rchannel) enum AVChannel everywhere? > { > - uint64_t layout = av_get_channel_layout(arg); > + int channel = av_channel_from_string(arg); > > - if (av_get_channel_layout_nb_channels(layout) != 1) > + if (channel < 0) > return AVERROR(EINVAL); > - *rchannel = layout; > + *rchannel = channel; > return 0; > } > > @@ -103,14 +103,14 @@ static void parse_map(AVFilterContext *ctx) > > p = s->map; > while ((arg = av_strtok(p, "|", &tokenizer))) { > - uint64_t out_channel; > + int out_channel; > > p = NULL; > if (parse_channel_name(arg, &out_channel)) { > av_log(ctx, AV_LOG_WARNING, "Failed to parse \'%s\' as channel name.\n", arg); > continue; > } > - if (used_channels & out_channel) { > + if (used_channels & (1ULL << out_channel)) { should check that out_channel < 64 > diff --git a/libavfilter/af_surround.c b/libavfilter/af_surround.c > index ccd85148e9..71d713e4ed 100644 > --- a/libavfilter/af_surround.c > +++ b/libavfilter/af_surround.c > @@ -92,8 +92,8 @@ typedef struct AudioSurroundContext { > float lowcut; > float highcut; > > - uint64_t out_channel_layout; > - uint64_t in_channel_layout; > + AVChannelLayout out_channel_layout; > + AVChannelLayout in_channel_layout; > int nb_in_channels; > int nb_out_channels; > > @@ -171,7 +171,7 @@ static int query_formats(AVFilterContext *ctx) > return ret; > > layouts = NULL; > - ret = ff_add_channel_layout(&layouts, s->out_channel_layout); > + ret = ff_add_channel_layout(&layouts, &s->out_channel_layout); > if (ret) > return ret; > > @@ -180,7 +180,7 @@ static int query_formats(AVFilterContext *ctx) > return ret; > > layouts = NULL; > - ret = ff_add_channel_layout(&layouts, s->in_channel_layout); > + ret = ff_add_channel_layout(&layouts, &s->in_channel_layout); > if (ret) > return ret; > > @@ -197,12 +197,12 @@ static int config_input(AVFilterLink *inlink) > AudioSurroundContext *s = ctx->priv; > int ch; > > - s->rdft = av_calloc(inlink->channels, sizeof(*s->rdft)); > + s->rdft = av_calloc(inlink->ch_layout.nb_channels, sizeof(*s->rdft)); > if (!s->rdft) > return AVERROR(ENOMEM); > - s->nb_in_channels = inlink->channels; > + s->nb_in_channels = inlink->ch_layout.nb_channels; > > - for (ch = 0; ch < inlink->channels; ch++) { > + for (ch = 0; ch < inlink->ch_layout.nb_channels; ch++) { > s->rdft[ch] = av_rdft_init(ff_log2(s->buf_size), DFT_R2C); > if (!s->rdft[ch]) > return AVERROR(ENOMEM); > @@ -212,31 +212,31 @@ static int config_input(AVFilterLink *inlink) > return AVERROR(ENOMEM); > for (ch = 0; ch < s->nb_in_channels; ch++) > s->input_levels[ch] = s->level_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_FRONT_CENTER); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_FRONT_CENTER); > if (ch >= 0) > s->input_levels[ch] *= s->fc_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_FRONT_LEFT); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_FRONT_LEFT); > if (ch >= 0) > s->input_levels[ch] *= s->fl_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_FRONT_RIGHT); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_FRONT_RIGHT); > if (ch >= 0) > s->input_levels[ch] *= s->fr_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_SIDE_LEFT); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_SIDE_LEFT); > if (ch >= 0) > s->input_levels[ch] *= s->sl_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_SIDE_RIGHT); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_SIDE_RIGHT); > if (ch >= 0) > s->input_levels[ch] *= s->sr_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_BACK_LEFT); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_BACK_LEFT); > if (ch >= 0) > s->input_levels[ch] *= s->bl_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_BACK_RIGHT); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_BACK_RIGHT); > if (ch >= 0) > s->input_levels[ch] *= s->br_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_BACK_CENTER); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_BACK_CENTER); > if (ch >= 0) > s->input_levels[ch] *= s->bc_in; > - ch = av_get_channel_layout_channel_index(inlink->channel_layout, AV_CH_LOW_FREQUENCY); > + ch = av_channel_layout_index_from_channel(&inlink->ch_layout, AV_CHAN_LOW_FREQUENCY); > if (ch >= 0) Make all those compare to AV_CHAN_NONE -- 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".