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 9107C41127 for ; Mon, 14 Feb 2022 15:49:55 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 1B82E68B06C; Mon, 14 Feb 2022 17:49:52 +0200 (EET) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 43DFF68AFAF for ; Mon, 14 Feb 2022 17:49:46 +0200 (EET) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id 4A4FD240179 for ; Mon, 14 Feb 2022 16:49:45 +0100 (CET) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavisd-new, port 10024) with ESMTP id gUgRu9T4xewM for ; Mon, 14 Feb 2022 16:49:44 +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 0B9D3240175 for ; Mon, 14 Feb 2022 16:49:44 +0100 (CET) Received: by lain.red.khirnov.net (Postfix, from userid 1000) id 3BDC41601AD; Mon, 14 Feb 2022 16:49:44 +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: Mon, 14 Feb 2022 16:49:44 +0100 Message-ID: <164485378421.19727.12778582756705145993@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_apulsator.c b/libavfilter/af_apulsator.c > index c2a8de0e0b..c3ca752035 100644 > --- a/libavfilter/af_apulsator.c > +++ b/libavfilter/af_apulsator.c > @@ -192,7 +192,7 @@ static int query_formats(AVFilterContext *ctx) > > if ((ret = ff_add_format (&formats, AV_SAMPLE_FMT_DBL )) < 0 || > (ret = ff_set_common_formats (ctx , formats )) < 0 || > - (ret = ff_add_channel_layout (&layout , AV_CH_LAYOUT_STEREO)) < 0 || > + (ret = ff_add_channel_layout (&layout , &(AVChannelLayout)AV_CHANNEL_LAYOUT_MONO)) < 0 || completion fail? > diff --git a/libavfilter/af_sofalizer.c b/libavfilter/af_sofalizer.c > index 20b717bdf8..246ddeffb9 100644 > --- a/libavfilter/af_sofalizer.c > +++ b/libavfilter/af_sofalizer.c > @@ -187,25 +187,15 @@ static int preload_sofa(AVFilterContext *ctx, char *filename, int *samplingrate) > > static int parse_channel_name(AVFilterContext *ctx, char **arg, int *rchannel) > { > - int len, i, channel_id = 0; > - int64_t layout, layout0; > + int len, channel_id = 0; > char buf[8] = {0}; > > /* try to parse a channel name, e.g. "FL" */ > if (av_sscanf(*arg, "%7[A-Z]%n", buf, &len)) { > - layout0 = layout = av_get_channel_layout(buf); > - /* channel_id <- first set bit in layout */ > - for (i = 32; i > 0; i >>= 1) { > - if (layout >= 1LL << i) { > - channel_id += i; > - layout >>= i; > - } > - } > - /* reject layouts that are not a single channel */ > - if (channel_id >= 64 || layout0 != 1LL << channel_id) { You should keep the range check, because the id gets used as index into vspkrpos[64]. > - av_log(ctx, AV_LOG_WARNING, "Failed to parse \'%s\' as channel name.\n", buf); > - return AVERROR(EINVAL); > - } > + channel_id = av_channel_from_string(buf); > + if (channel_id < 0) > + return channel_id; > + > *rchannel = channel_id; > *arg += len; > return 0; > @@ -221,7 +211,7 @@ static int parse_channel_name(AVFilterContext *ctx, char **arg, int *rchannel) > return AVERROR(EINVAL); > } > > -static void parse_speaker_pos(AVFilterContext *ctx, int64_t in_channel_layout) > +static void parse_speaker_pos(AVFilterContext *ctx) > { > SOFAlizerContext *s = ctx->priv; > char *arg, *tokenizer, *p, *args = av_strdup(s->speakers_pos); > @@ -256,10 +246,10 @@ static int get_speaker_pos(AVFilterContext *ctx, > float *speaker_azim, float *speaker_elev) > { > struct SOFAlizerContext *s = ctx->priv; > - uint64_t channels_layout = ctx->inputs[0]->channel_layout; > + AVChannelLayout *channel_layout = &ctx->inputs[0]->ch_layout; > float azim[64] = { 0 }; > float elev[64] = { 0 }; > - int m, ch, n_conv = ctx->inputs[0]->channels; /* get no. input channels */ > + int m, ch, n_conv = ctx->inputs[0]->ch_layout.nb_channels; /* get no. input channels */ > > if (n_conv < 0 || n_conv > 64) > return AVERROR(EINVAL); > @@ -267,45 +257,45 @@ static int get_speaker_pos(AVFilterContext *ctx, > s->lfe_channel = -1; > > if (s->speakers_pos) > - parse_speaker_pos(ctx, channels_layout); > + parse_speaker_pos(ctx); > > /* set speaker positions according to input channel configuration: */ > for (m = 0, ch = 0; ch < n_conv && m < 64; m++) { > - uint64_t mask = channels_layout & (1ULL << m); > - > - switch (mask) { > - case AV_CH_FRONT_LEFT: azim[ch] = 30; break; > - case AV_CH_FRONT_RIGHT: azim[ch] = 330; break; > - case AV_CH_FRONT_CENTER: azim[ch] = 0; break; > - case AV_CH_LOW_FREQUENCY: > - case AV_CH_LOW_FREQUENCY_2: s->lfe_channel = ch; break; > - case AV_CH_BACK_LEFT: azim[ch] = 150; break; > - case AV_CH_BACK_RIGHT: azim[ch] = 210; break; > - case AV_CH_BACK_CENTER: azim[ch] = 180; break; > - case AV_CH_SIDE_LEFT: azim[ch] = 90; break; > - case AV_CH_SIDE_RIGHT: azim[ch] = 270; break; > - case AV_CH_FRONT_LEFT_OF_CENTER: azim[ch] = 15; break; > - case AV_CH_FRONT_RIGHT_OF_CENTER: azim[ch] = 345; break; > - case AV_CH_TOP_CENTER: azim[ch] = 0; > + int chan = av_channel_layout_channel_from_index(channel_layout, m); > + > + switch (chan) { > + case AV_CHAN_FRONT_LEFT: azim[ch] = 30; break; > + case AV_CHAN_FRONT_RIGHT: azim[ch] = 330; break; > + case AV_CHAN_FRONT_CENTER: azim[ch] = 0; break; > + case AV_CHAN_LOW_FREQUENCY: > + case AV_CHAN_LOW_FREQUENCY_2: s->lfe_channel = ch; break; > + case AV_CHAN_BACK_LEFT: azim[ch] = 150; break; > + case AV_CHAN_BACK_RIGHT: azim[ch] = 210; break; > + case AV_CHAN_BACK_CENTER: azim[ch] = 180; break; > + case AV_CHAN_SIDE_LEFT: azim[ch] = 90; break; > + case AV_CHAN_SIDE_RIGHT: azim[ch] = 270; break; > + case AV_CHAN_FRONT_LEFT_OF_CENTER: azim[ch] = 15; break; > + case AV_CHAN_FRONT_RIGHT_OF_CENTER: azim[ch] = 345; break; > + case AV_CHAN_TOP_CENTER: azim[ch] = 0; > elev[ch] = 90; break; > - case AV_CH_TOP_FRONT_LEFT: azim[ch] = 30; > + case AV_CHAN_TOP_FRONT_LEFT: azim[ch] = 30; > elev[ch] = 45; break; > - case AV_CH_TOP_FRONT_CENTER: azim[ch] = 0; > + case AV_CHAN_TOP_FRONT_CENTER: azim[ch] = 0; > elev[ch] = 45; break; > - case AV_CH_TOP_FRONT_RIGHT: azim[ch] = 330; > + case AV_CHAN_TOP_FRONT_RIGHT: azim[ch] = 330; > elev[ch] = 45; break; > - case AV_CH_TOP_BACK_LEFT: azim[ch] = 150; > + case AV_CHAN_TOP_BACK_LEFT: azim[ch] = 150; > elev[ch] = 45; break; > - case AV_CH_TOP_BACK_RIGHT: azim[ch] = 210; > + case AV_CHAN_TOP_BACK_RIGHT: azim[ch] = 210; > elev[ch] = 45; break; > - case AV_CH_TOP_BACK_CENTER: azim[ch] = 180; > + case AV_CHAN_TOP_BACK_CENTER: azim[ch] = 180; > elev[ch] = 45; break; > - case AV_CH_WIDE_LEFT: azim[ch] = 90; break; > - case AV_CH_WIDE_RIGHT: azim[ch] = 270; break; > - case AV_CH_SURROUND_DIRECT_LEFT: azim[ch] = 90; break; > - case AV_CH_SURROUND_DIRECT_RIGHT: azim[ch] = 270; break; > - case AV_CH_STEREO_LEFT: azim[ch] = 90; break; > - case AV_CH_STEREO_RIGHT: azim[ch] = 270; break; > + case AV_CHAN_WIDE_LEFT: azim[ch] = 90; break; > + case AV_CHAN_WIDE_RIGHT: azim[ch] = 270; break; > + case AV_CHAN_SURROUND_DIRECT_LEFT: azim[ch] = 90; break; > + case AV_CHAN_SURROUND_DIRECT_RIGHT: azim[ch] = 270; break; > + case AV_CHAN_STEREO_LEFT: azim[ch] = 90; break; > + case AV_CHAN_STEREO_RIGHT: azim[ch] = 270; break; > case 0: break; case 0 makes no sense now > default: > return AVERROR(EINVAL); > @@ -316,7 +306,7 @@ static int get_speaker_pos(AVFilterContext *ctx, > elev[ch] = s->vspkrpos[m].elev; > } > > - if (mask) > + if (chan) and this check should be removed I think > diff --git a/libavfilter/buffersink.h b/libavfilter/buffersink.h > index 69ed0f29a8..ae90ae87fa 100644 > --- a/libavfilter/buffersink.h > +++ b/libavfilter/buffersink.h > @@ -46,7 +46,7 @@ > * - av_buffersink_get_h(), > * - av_buffersink_get_sample_aspect_ratio(), > * - av_buffersink_get_channels(), > - * - av_buffersink_get_channel_layout(), > + * - av_buffersink_get_ch_layout(), should mention that the layout should be uninited by the caller > diff --git a/libavfilter/f_ebur128.c b/libavfilter/f_ebur128.c > index 88d6a1fe46..2a6edba3d8 100644 > --- a/libavfilter/f_ebur128.c > +++ b/libavfilter/f_ebur128.c > @@ -422,7 +422,7 @@ static int config_audio_output(AVFilterLink *outlink) > int i; > AVFilterContext *ctx = outlink->src; > EBUR128Context *ebur128 = ctx->priv; > - const int nb_channels = av_get_channel_layout_nb_channels(outlink->channel_layout); > + const int nb_channels = outlink->ch_layout.nb_channels; > > #define BACK_MASK (AV_CH_BACK_LEFT |AV_CH_BACK_CENTER |AV_CH_BACK_RIGHT| \ > AV_CH_TOP_BACK_LEFT|AV_CH_TOP_BACK_CENTER|AV_CH_TOP_BACK_RIGHT| \ > @@ -439,8 +439,8 @@ static int config_audio_output(AVFilterLink *outlink) > > for (i = 0; i < nb_channels; i++) { > /* channel weighting */ > - const uint64_t chl = av_channel_layout_extract_channel(outlink->channel_layout, i); > - if (chl & (AV_CH_LOW_FREQUENCY|AV_CH_LOW_FREQUENCY_2)) { > + const int chl = av_channel_layout_channel_from_index(&outlink->ch_layout, i); enum AVChannel? > + if (chl == AV_CHAN_LOW_FREQUENCY || chl == AV_CHAN_LOW_FREQUENCY_2) { > ebur128->ch_weighting[i] = 0; > } else if (chl & BACK_MASK) { This looks broken. > diff --git a/libavfilter/formats.h b/libavfilter/formats.h > index a884d15213..e55180f45c 100644 > --- a/libavfilter/formats.h > +++ b/libavfilter/formats.h > @@ -83,7 +83,7 @@ struct AVFilterFormats { > * (e.g. AV_CH_LAYOUT_STEREO and FF_COUNT2LAYOUT(2). > */ > struct AVFilterChannelLayouts { > - uint64_t *channel_layouts; ///< list of channel layouts > + AVChannelLayout *channel_layouts; ///< list of channel layouts > int nb_channel_layouts; ///< number of channel layouts > char all_layouts; ///< accept any known channel layout > char all_counts; ///< accept any channel layout or count > @@ -99,14 +99,16 @@ struct AVFilterChannelLayouts { > * The result is only valid inside AVFilterChannelLayouts and immediately > * related functions. > */ > -#define FF_COUNT2LAYOUT(c) (0x8000000000000000ULL | (c)) > +#define FF_COUNT2LAYOUT(c) ((AVChannelLayout) { .order = AV_CHANNEL_ORDER_UNSPEC, .nb_channels = c }) > > /** > * Decode a channel count encoded as a channel layout. > * Return 0 if the channel layout was a real one. > */ > -#define FF_LAYOUT2COUNT(l) (((l) & 0x8000000000000000ULL) ? \ > - (int)((l) & 0x7FFFFFFF) : 0) > +#define FF_LAYOUT2COUNT(l) (((l)->order == AV_CHANNEL_ORDER_UNSPEC) ? \ > + (l)->nb_channels : 0) > + > +#define KNOWN(l) (!FF_LAYOUT2COUNT(l)) /* for readability */ > > /** > * Construct an empty AVFilterChannelLayouts/AVFilterFormats struct -- > @@ -126,7 +128,7 @@ av_warn_unused_result > AVFilterChannelLayouts *ff_all_channel_counts(void); > > av_warn_unused_result > -AVFilterChannelLayouts *ff_make_format64_list(const int64_t *fmts); > +AVFilterChannelLayouts *ff_make_format64_list(const AVChannelLayout *fmts); The function name no longer makes sense -- 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".