Re: [FFmpeg-devel] [PATCH] avcodec/sbcenc: port to AVCodec.get_supported_config()

[Zhao Zhili <quinkblack-at-foxmail.com@ffmpeg.org>]

> On Feb 19, 2025, at 11:08, epirat07@gmail.com wrote:
> 
> 
> 
> On 19 Feb 2025, at 3:47, Zhao Zhili wrote:
> 
>>> On Feb 19, 2025, at 03:28, James Almer <jamrial@gmail.com> wrote:
>>> 
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> libavcodec/sbcenc.c | 50 +++++++++++++++++++++++++++++++++++++--------
>>> 1 file changed, 41 insertions(+), 9 deletions(-)
>>> 
>>> diff --git a/libavcodec/sbcenc.c b/libavcodec/sbcenc.c
>>> index f2c4fbe329..d85ab76cb0 100644
>>> --- a/libavcodec/sbcenc.c
>>> +++ b/libavcodec/sbcenc.c
>>> @@ -194,6 +194,8 @@ static size_t sbc_pack_frame(AVPacket *avpkt, struct sbc_frame *frame,
>>>    return put_bytes_output(&pb);
>>> }
>>> 
>>> +static const int sbc_supported_samplerates[] = { 16000, 32000, 44100, 48000, 0 };
>>> +
>>> static int sbc_encode_init(AVCodecContext *avctx)
>>> {
>>>    SBCEncContext *sbc = avctx->priv_data;
>>> @@ -260,8 +262,8 @@ static int sbc_encode_init(AVCodecContext *avctx)
>>>        avctx->frame_size = 4*((frame->subbands >> 3) + 1) * 4*(frame->blocks >> 2);
>>>    }
>>> 
>>> -    for (int i = 0; avctx->codec->supported_samplerates[i]; i++)
>>> -        if (avctx->sample_rate == avctx->codec->supported_samplerates[i])
>>> +    for (int i = 0; sbc_supported_samplerates[i]; i++)
>>> +        if (avctx->sample_rate == sbc_supported_samplerates[i])
>>>            frame->frequency = i;
>>> 
>>>    frame->channels = avctx->ch_layout.nb_channels;
>>> @@ -326,6 +328,41 @@ static int sbc_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>>>    return 0;
>>> }
>>> 
>>> +static const enum AVSampleFormat sbc_sample_fmts[] = {
>>> +   AV_SAMPLE_FMT_S16,
>>> +   AV_SAMPLE_FMT_NONE
>>> +};
>>> +
>>> +static const AVChannelLayout sbc_ch_layouts[] = {
>>> +    AV_CHANNEL_LAYOUT_MONO,
>>> +    AV_CHANNEL_LAYOUT_STEREO,
>>> +    { 0 }
>>> +};
>>> +
>>> +static int sbc_get_supported_config(const AVCodecContext *avctx,
>>> +                                    const AVCodec *codec,
>>> +                                    enum AVCodecConfig config,
>>> +                                    unsigned flags, const void **out,
>>> +                                    int *out_num)
>>> +{
>>> +    switch (config) {
>>> +    case AV_CODEC_CONFIG_SAMPLE_RATE:
>>> +        *out = sbc_supported_samplerates;
>>> +        *out_num = FF_ARRAY_ELEMS(sbc_supported_samplerates) - 1;
>>> +        return 0;
>>> +    case AV_CODEC_CONFIG_SAMPLE_FORMAT:
>>> +        *out = sbc_sample_fmts;
>>> +        *out_num = FF_ARRAY_ELEMS(sbc_sample_fmts) - 1;
>>> +        return 0;
>>> +    case AV_CODEC_CONFIG_CHANNEL_LAYOUT:
>>> +        *out = sbc_ch_layouts;
>>> +        *out_num = FF_ARRAY_ELEMS(sbc_ch_layouts) - 1;
>>> +        return 0;
>>> +    }
>>> +
>>> +    return ff_default_get_supported_config(avctx, codec, config, flags, out, out_num);
>>> +}
>>> +
>>> #define OFFSET(x) offsetof(SBCEncContext, x)
>>> #define AE AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>>> static const AVOption options[] = {
>>> @@ -344,7 +381,7 @@ static const AVClass sbc_class = {
>>>    .version    = LIBAVUTIL_VERSION_INT,
>>> };
>>> 
>>> -const FFCodec ff_sbc_encoder = {
>>> +FFCodec ff_sbc_encoder = {
>>>    .p.name                = "sbc",
>>>    CODEC_LONG_NAME("SBC (low-complexity subband codec)"),
>>>    .p.type                = AVMEDIA_TYPE_AUDIO,
>>> @@ -354,12 +391,7 @@ const FFCodec ff_sbc_encoder = {
>>>    .priv_data_size        = sizeof(SBCEncContext),
>>>    .init                  = sbc_encode_init,
>>>    FF_CODEC_ENCODE_CB(sbc_encode_frame),
>>> -    .p.ch_layouts          = (const AVChannelLayout[]) { AV_CHANNEL_LAYOUT_MONO,
>>> -                                                         AV_CHANNEL_LAYOUT_STEREO,
>>> -                                                         { 0 } },
>>> -    .p.sample_fmts         = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16,
>>> -                                                             AV_SAMPLE_FMT_NONE },
>>> -    .p.supported_samplerates = (const int[]) { 16000, 32000, 44100, 48000, 0 },
>>> +    .get_supported_config  = sbc_get_supported_config,
>>>    .p.priv_class          = &sbc_class,
>>>    .p.profiles            = NULL_IF_CONFIG_SMALL(ff_sbc_profiles),
>>> };
>> 
>> static fields works for most of the codecs. It’s not worth the complexity and code duplication
>> to implement get_supported_config just to silence the warning.
>> 
>> The issue has two parts:
>> 1. Prepare to remove those fields inside libavcodec.
>> 2. Silence the warning
>> 
>> For the first part, I prefer add those fields to FFCodec, e.g.,
>> 
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2025-February/339330.html
>> 
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2025-February/339335.html
>> 
> 
> Somehow I missed these previous patches… We just had some discussion
> on IRC about it too as I tried to solve this too.
> 
> When we move the fields, maybe we could use a union like I did here:
> 
> https://github.com/FFmpeg/FFmpeg/commit/48cfd98710cbd4430ee0e4560e16ffdd49ebeef2#diff-ab77ecd11adfbb83894f20846e2b9efc2d126693d93fb82a7bef13973bb444f2

Union looks good. The macro FF_CODEC_SET_CONFIG_COMPAT_HELPER doesn’t work as Andreas pointed out.

> 
>> This method doesn’t silence the warning.
>> 
>> For the second part, I have a non portable try
>> 
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2025-February/339334.html
>> 
>> And Andreas pointed out how to make it portable.
> 
> Not sure it makes sense to duplicate all those for now?
> It's what I was experimenting with at
> https://github.com/FFmpeg/FFmpeg/compare/master...ePirat:FFmpeg:epirat-move-deprecated-fields-to-ffcodec
> but ultimately decided it might be better to just silence
> the deprecation warnings for now and wait instead of all these changes
> that partially need to be changed all over again once we can get rid of
> the fields in AVCodec when we do another major bump in a year or so.

New code path should be ready before major bump, and major bump should
just drop deprecated code. Add new code path at the same time of major
bump is tricky, it doesn’t get enough test.

> 
>> 
>> 
>>> -- 
>>> 2.48.1
>>> 
>>> _______________________________________________
>>> 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".
>> 
>> 
>> _______________________________________________
>> 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".
> _______________________________________________
> 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".

_______________________________________________
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".