diff mbox series

[FFmpeg-devel,08/11] fftools/opt_common: switch to avcodec_get_supported_config()

Message ID 20240405185721.111072-8-ffmpeg@haasn.xyz
State New
Headers show
Series [FFmpeg-devel,01/11] avcodec: add avcodec_get_supported_config() | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Niklas Haas April 5, 2024, 6:57 p.m. UTC
From: Niklas Haas <git@haasn.dev>

While rewriting this macro, I decided to make it a bit more flexible so
it can work for all of the fields (including future fields) in a more
generic way.
---
 fftools/opt_common.c | 86 ++++++++++++++++++++++----------------------
 1 file changed, 43 insertions(+), 43 deletions(-)

Comments

James Almer April 5, 2024, 7:36 p.m. UTC | #1
On 4/5/2024 3:57 PM, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> While rewriting this macro, I decided to make it a bit more flexible so
> it can work for all of the fields (including future fields) in a more
> generic way.
> ---
>   fftools/opt_common.c | 86 ++++++++++++++++++++++----------------------
>   1 file changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/fftools/opt_common.c b/fftools/opt_common.c
> index 947a226d8d1..1bf66580192 100644
> --- a/fftools/opt_common.c
> +++ b/fftools/opt_common.c
> @@ -262,22 +262,32 @@ int show_buildconf(void *optctx, const char *opt, const char *arg)
>       return 0;
>   }
>   
> -#define PRINT_CODEC_SUPPORTED(codec, field, type, list_name, term, get_name) \
> -    if (codec->field) {                                                      \
> -        const type *p = codec->field;                                        \
> -                                                                             \
> -        printf("    Supported " list_name ":");                              \
> -        while (*p != term) {                                                 \
> -            get_name(*p);                                                    \
> -            printf(" %s", name);                                             \
> -            p++;                                                             \
> -        }                                                                    \
> -        printf("\n");                                                        \
> -    }                                                                        \
> +#define PRINT_CODEC_SUPPORTED(codec, config, type, name, elem, cond, fmt, ...)  \
> +    do {                                                                        \
> +        const type *elem = NULL;                                                \
> +        avcodec_get_supported_config(NULL, codec, config, 0,                    \
> +                                     (const void **) &elem);                    \
> +        if (elem) {                                                             \
> +            printf("    Supported " name ":");                                  \
> +            while (cond) {                                                      \
> +                printf(" " fmt, __VA_ARGS__);                                   \
> +                elem++;                                                         \
> +            }                                                                   \
> +            printf("\n");                                                       \
> +        }                                                                       \
> +    } while (0)
> +
> +static char *get_channel_layout_desc(const AVChannelLayout *layout,
> +                                     char desc[], int desc_size)
> +{
> +    av_channel_layout_describe(layout, desc, desc_size);
> +    return desc;
> +}
>   
>   static void print_codec(const AVCodec *c)
>   {
>       int encoder = av_codec_is_encoder(c);
> +    char desc[128];
>   
>       printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
>              c->long_name ? c->long_name : "");
> @@ -343,35 +353,19 @@ static void print_codec(const AVCodec *c)
>           printf("\n");
>       }
>   
> -    if (c->supported_framerates) {
> -        const AVRational *fps = c->supported_framerates;
> -
> -        printf("    Supported framerates:");
> -        while (fps->num) {
> -            printf(" %d/%d", fps->num, fps->den);
> -            fps++;
> -        }
> -        printf("\n");
> -    }
> -    PRINT_CODEC_SUPPORTED(c, pix_fmts, enum AVPixelFormat, "pixel formats",
> -                          AV_PIX_FMT_NONE, GET_PIX_FMT_NAME);
> -    PRINT_CODEC_SUPPORTED(c, supported_samplerates, int, "sample rates", 0,
> -                          GET_SAMPLE_RATE_NAME);
> -    PRINT_CODEC_SUPPORTED(c, sample_fmts, enum AVSampleFormat, "sample formats",
> -                          AV_SAMPLE_FMT_NONE, GET_SAMPLE_FMT_NAME);
> -
> -    if (c->ch_layouts) {
> -        const AVChannelLayout *p = c->ch_layouts;
> -
> -        printf("    Supported channel layouts:");
> -        while (p->nb_channels) {
> -            char name[128];
> -            av_channel_layout_describe(p, name, sizeof(name));
> -            printf(" %s", name);
> -            p++;
> -        }
> -        printf("\n");
> -    }
> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_FRAME_RATE, AVRational, "framerates",
> +                          fps, fps->num, "%d/%d", fps->num, fps->den);
> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_PIX_FORMAT, enum AVPixelFormat,
> +                          "pixel formats", fmt, *fmt != AV_PIX_FMT_NONE,
> +                          "%s", av_get_pix_fmt_name(*fmt));
> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_RATE, int, "sample rates",
> +                          rate, *rate != 0, "%d", *rate);
> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
> +                          "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
> +                          "%s", av_get_sample_fmt_name(*fmt));
> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
> +                          "channel layouts", layout, layout->nb_channels,
> +                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));

Since you're rewriting everything, might as well go and port this to 
AVBprint, so we're not limited to 128 bytes for the channel layout.
You can use av_bprintf() to write to the buffer using the different 
specifiers, then printf() the final bprint buffer to stdout.

>   
>       if (c->priv_class) {
>           show_help_children(c->priv_class,
> @@ -566,8 +560,14 @@ static void show_help_bsf(const char *name)
>       }
>   
>       printf("Bit stream filter %s\n", bsf->name);
> -    PRINT_CODEC_SUPPORTED(bsf, codec_ids, enum AVCodecID, "codecs",
> -                          AV_CODEC_ID_NONE, GET_CODEC_NAME);
> +    if (bsf->codec_ids) {
> +        const enum AVCodecID *id = bsf->codec_ids;
> +        printf("    Supported codecs:");
> +        while (*id != AV_CODEC_ID_NONE) {
> +            printf(" %s", avcodec_descriptor_get(*id)->name);
> +            id++;
> +        }
> +    }
>       if (bsf->priv_class)
>           show_help_children(bsf->priv_class, AV_OPT_FLAG_BSF_PARAM);
>   }
Niklas Haas April 6, 2024, 11:38 a.m. UTC | #2
On Fri, 05 Apr 2024 16:36:50 -0300 James Almer <jamrial@gmail.com> wrote:
> On 4/5/2024 3:57 PM, Niklas Haas wrote:
> > From: Niklas Haas <git@haasn.dev>
> > 
> > While rewriting this macro, I decided to make it a bit more flexible so
> > it can work for all of the fields (including future fields) in a more
> > generic way.
> > ---
> >   fftools/opt_common.c | 86 ++++++++++++++++++++++----------------------
> >   1 file changed, 43 insertions(+), 43 deletions(-)
> > 
> > diff --git a/fftools/opt_common.c b/fftools/opt_common.c
> > index 947a226d8d1..1bf66580192 100644
> > --- a/fftools/opt_common.c
> > +++ b/fftools/opt_common.c
> > @@ -262,22 +262,32 @@ int show_buildconf(void *optctx, const char *opt, const char *arg)
> >       return 0;
> >   }
> >   
> > -#define PRINT_CODEC_SUPPORTED(codec, field, type, list_name, term, get_name) \
> > -    if (codec->field) {                                                      \
> > -        const type *p = codec->field;                                        \
> > -                                                                             \
> > -        printf("    Supported " list_name ":");                              \
> > -        while (*p != term) {                                                 \
> > -            get_name(*p);                                                    \
> > -            printf(" %s", name);                                             \
> > -            p++;                                                             \
> > -        }                                                                    \
> > -        printf("\n");                                                        \
> > -    }                                                                        \
> > +#define PRINT_CODEC_SUPPORTED(codec, config, type, name, elem, cond, fmt, ...)  \
> > +    do {                                                                        \
> > +        const type *elem = NULL;                                                \
> > +        avcodec_get_supported_config(NULL, codec, config, 0,                    \
> > +                                     (const void **) &elem);                    \
> > +        if (elem) {                                                             \
> > +            printf("    Supported " name ":");                                  \
> > +            while (cond) {                                                      \
> > +                printf(" " fmt, __VA_ARGS__);                                   \
> > +                elem++;                                                         \
> > +            }                                                                   \
> > +            printf("\n");                                                       \
> > +        }                                                                       \
> > +    } while (0)
> > +
> > +static char *get_channel_layout_desc(const AVChannelLayout *layout,
> > +                                     char desc[], int desc_size)
> > +{
> > +    av_channel_layout_describe(layout, desc, desc_size);
> > +    return desc;
> > +}
> >   
> >   static void print_codec(const AVCodec *c)
> >   {
> >       int encoder = av_codec_is_encoder(c);
> > +    char desc[128];
> >   
> >       printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
> >              c->long_name ? c->long_name : "");
> > @@ -343,35 +353,19 @@ static void print_codec(const AVCodec *c)
> >           printf("\n");
> >       }
> >   
> > -    if (c->supported_framerates) {
> > -        const AVRational *fps = c->supported_framerates;
> > -
> > -        printf("    Supported framerates:");
> > -        while (fps->num) {
> > -            printf(" %d/%d", fps->num, fps->den);
> > -            fps++;
> > -        }
> > -        printf("\n");
> > -    }
> > -    PRINT_CODEC_SUPPORTED(c, pix_fmts, enum AVPixelFormat, "pixel formats",
> > -                          AV_PIX_FMT_NONE, GET_PIX_FMT_NAME);
> > -    PRINT_CODEC_SUPPORTED(c, supported_samplerates, int, "sample rates", 0,
> > -                          GET_SAMPLE_RATE_NAME);
> > -    PRINT_CODEC_SUPPORTED(c, sample_fmts, enum AVSampleFormat, "sample formats",
> > -                          AV_SAMPLE_FMT_NONE, GET_SAMPLE_FMT_NAME);
> > -
> > -    if (c->ch_layouts) {
> > -        const AVChannelLayout *p = c->ch_layouts;
> > -
> > -        printf("    Supported channel layouts:");
> > -        while (p->nb_channels) {
> > -            char name[128];
> > -            av_channel_layout_describe(p, name, sizeof(name));
> > -            printf(" %s", name);
> > -            p++;
> > -        }
> > -        printf("\n");
> > -    }
> > +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_FRAME_RATE, AVRational, "framerates",
> > +                          fps, fps->num, "%d/%d", fps->num, fps->den);
> > +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_PIX_FORMAT, enum AVPixelFormat,
> > +                          "pixel formats", fmt, *fmt != AV_PIX_FMT_NONE,
> > +                          "%s", av_get_pix_fmt_name(*fmt));
> > +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_RATE, int, "sample rates",
> > +                          rate, *rate != 0, "%d", *rate);
> > +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
> > +                          "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
> > +                          "%s", av_get_sample_fmt_name(*fmt));
> > +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
> > +                          "channel layouts", layout, layout->nb_channels,
> > +                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));
> 
> Since you're rewriting everything, might as well go and port this to 
> AVBprint, so we're not limited to 128 bytes for the channel layout.
> You can use av_bprintf() to write to the buffer using the different 
> specifiers, then printf() the final bprint buffer to stdout.

I implemented something like this initially, but it is IMO premature
optimization as there is no multi-threading going on in this code, so no
risk of colliding printf() calls. printf() already internally buffers
data, so we are not gaining anything by buffering it inside an AVBPrint
a second time.

That said, using an AVBPrint instead of a fixed size buffer is a good
idea for the channel layout specifically, something like this:


diff --git a/fftools/opt_common.c b/fftools/opt_common.c
index 1bf66580192..fae68949f89 100644
--- a/fftools/opt_common.c
+++ b/fftools/opt_common.c
@@ -277,17 +277,17 @@ int show_buildconf(void *optctx, const char *opt, const char *arg)
         }                                                                       \
     } while (0)
 
-static char *get_channel_layout_desc(const AVChannelLayout *layout,
-                                     char desc[], int desc_size)
+static char *get_channel_layout_desc(const AVChannelLayout *layout, AVBPrint *bp)
 {
-    av_channel_layout_describe(layout, desc, desc_size);
-    return desc;
+    av_bprint_clear(bp);
+    av_channel_layout_describe_bprintf(layout, bp);
+    return bp->str;
 }
 
 static void print_codec(const AVCodec *c)
 {
     int encoder = av_codec_is_encoder(c);
-    char desc[128];
+    AVBPrint desc;
 
     printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
            c->long_name ? c->long_name : "");
@@ -363,9 +363,12 @@ static void print_codec(const AVCodec *c)
     PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
                           "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
                           "%s", av_get_sample_fmt_name(*fmt));
+
+    av_bprint_init(&desc, AV_BPRINT_SIZE_AUTOMATIC);
     PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
                           "channel layouts", layout, layout->nb_channels,
-                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));
+                          "%s", get_channel_layout_desc(layout, &desc));
+    av_bprint_finalize(&desc, NULL);
 
     if (c->priv_class) {
         show_help_children(c->priv_class,

> 
> >   
> >       if (c->priv_class) {
> >           show_help_children(c->priv_class,
> > @@ -566,8 +560,14 @@ static void show_help_bsf(const char *name)
> >       }
> >   
> >       printf("Bit stream filter %s\n", bsf->name);
> > -    PRINT_CODEC_SUPPORTED(bsf, codec_ids, enum AVCodecID, "codecs",
> > -                          AV_CODEC_ID_NONE, GET_CODEC_NAME);
> > +    if (bsf->codec_ids) {
> > +        const enum AVCodecID *id = bsf->codec_ids;
> > +        printf("    Supported codecs:");
> > +        while (*id != AV_CODEC_ID_NONE) {
> > +            printf(" %s", avcodec_descriptor_get(*id)->name);
> > +            id++;
> > +        }
> > +    }
> >       if (bsf->priv_class)
> >           show_help_children(bsf->priv_class, AV_OPT_FLAG_BSF_PARAM);
> >   }
> _______________________________________________
> 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".
James Almer April 7, 2024, 12:10 a.m. UTC | #3
On 4/6/2024 8:38 AM, Niklas Haas wrote:
> On Fri, 05 Apr 2024 16:36:50 -0300 James Almer <jamrial@gmail.com> wrote:
>> On 4/5/2024 3:57 PM, Niklas Haas wrote:
>>> From: Niklas Haas <git@haasn.dev>
>>>
>>> While rewriting this macro, I decided to make it a bit more flexible so
>>> it can work for all of the fields (including future fields) in a more
>>> generic way.
>>> ---
>>>    fftools/opt_common.c | 86 ++++++++++++++++++++++----------------------
>>>    1 file changed, 43 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/fftools/opt_common.c b/fftools/opt_common.c
>>> index 947a226d8d1..1bf66580192 100644
>>> --- a/fftools/opt_common.c
>>> +++ b/fftools/opt_common.c
>>> @@ -262,22 +262,32 @@ int show_buildconf(void *optctx, const char *opt, const char *arg)
>>>        return 0;
>>>    }
>>>    
>>> -#define PRINT_CODEC_SUPPORTED(codec, field, type, list_name, term, get_name) \
>>> -    if (codec->field) {                                                      \
>>> -        const type *p = codec->field;                                        \
>>> -                                                                             \
>>> -        printf("    Supported " list_name ":");                              \
>>> -        while (*p != term) {                                                 \
>>> -            get_name(*p);                                                    \
>>> -            printf(" %s", name);                                             \
>>> -            p++;                                                             \
>>> -        }                                                                    \
>>> -        printf("\n");                                                        \
>>> -    }                                                                        \
>>> +#define PRINT_CODEC_SUPPORTED(codec, config, type, name, elem, cond, fmt, ...)  \
>>> +    do {                                                                        \
>>> +        const type *elem = NULL;                                                \
>>> +        avcodec_get_supported_config(NULL, codec, config, 0,                    \
>>> +                                     (const void **) &elem);                    \
>>> +        if (elem) {                                                             \
>>> +            printf("    Supported " name ":");                                  \
>>> +            while (cond) {                                                      \
>>> +                printf(" " fmt, __VA_ARGS__);                                   \
>>> +                elem++;                                                         \
>>> +            }                                                                   \
>>> +            printf("\n");                                                       \
>>> +        }                                                                       \
>>> +    } while (0)
>>> +
>>> +static char *get_channel_layout_desc(const AVChannelLayout *layout,
>>> +                                     char desc[], int desc_size)
>>> +{
>>> +    av_channel_layout_describe(layout, desc, desc_size);
>>> +    return desc;
>>> +}
>>>    
>>>    static void print_codec(const AVCodec *c)
>>>    {
>>>        int encoder = av_codec_is_encoder(c);
>>> +    char desc[128];
>>>    
>>>        printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
>>>               c->long_name ? c->long_name : "");
>>> @@ -343,35 +353,19 @@ static void print_codec(const AVCodec *c)
>>>            printf("\n");
>>>        }
>>>    
>>> -    if (c->supported_framerates) {
>>> -        const AVRational *fps = c->supported_framerates;
>>> -
>>> -        printf("    Supported framerates:");
>>> -        while (fps->num) {
>>> -            printf(" %d/%d", fps->num, fps->den);
>>> -            fps++;
>>> -        }
>>> -        printf("\n");
>>> -    }
>>> -    PRINT_CODEC_SUPPORTED(c, pix_fmts, enum AVPixelFormat, "pixel formats",
>>> -                          AV_PIX_FMT_NONE, GET_PIX_FMT_NAME);
>>> -    PRINT_CODEC_SUPPORTED(c, supported_samplerates, int, "sample rates", 0,
>>> -                          GET_SAMPLE_RATE_NAME);
>>> -    PRINT_CODEC_SUPPORTED(c, sample_fmts, enum AVSampleFormat, "sample formats",
>>> -                          AV_SAMPLE_FMT_NONE, GET_SAMPLE_FMT_NAME);
>>> -
>>> -    if (c->ch_layouts) {
>>> -        const AVChannelLayout *p = c->ch_layouts;
>>> -
>>> -        printf("    Supported channel layouts:");
>>> -        while (p->nb_channels) {
>>> -            char name[128];
>>> -            av_channel_layout_describe(p, name, sizeof(name));
>>> -            printf(" %s", name);
>>> -            p++;
>>> -        }
>>> -        printf("\n");
>>> -    }
>>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_FRAME_RATE, AVRational, "framerates",
>>> +                          fps, fps->num, "%d/%d", fps->num, fps->den);
>>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_PIX_FORMAT, enum AVPixelFormat,
>>> +                          "pixel formats", fmt, *fmt != AV_PIX_FMT_NONE,
>>> +                          "%s", av_get_pix_fmt_name(*fmt));
>>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_RATE, int, "sample rates",
>>> +                          rate, *rate != 0, "%d", *rate);
>>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
>>> +                          "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
>>> +                          "%s", av_get_sample_fmt_name(*fmt));
>>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
>>> +                          "channel layouts", layout, layout->nb_channels,
>>> +                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));
>>
>> Since you're rewriting everything, might as well go and port this to
>> AVBprint, so we're not limited to 128 bytes for the channel layout.
>> You can use av_bprintf() to write to the buffer using the different
>> specifiers, then printf() the final bprint buffer to stdout.
> 
> I implemented something like this initially, but it is IMO premature
> optimization as there is no multi-threading going on in this code, so no
> risk of colliding printf() calls. printf() already internally buffers
> data, so we are not gaining anything by buffering it inside an AVBPrint
> a second time.
> 
> That said, using an AVBPrint instead of a fixed size buffer is a good
> idea for the channel layout specifically, something like this:
> 
> 
> diff --git a/fftools/opt_common.c b/fftools/opt_common.c
> index 1bf66580192..fae68949f89 100644
> --- a/fftools/opt_common.c
> +++ b/fftools/opt_common.c
> @@ -277,17 +277,17 @@ int show_buildconf(void *optctx, const char *opt, const char *arg)
>           }                                                                       \
>       } while (0)
>   
> -static char *get_channel_layout_desc(const AVChannelLayout *layout,
> -                                     char desc[], int desc_size)
> +static char *get_channel_layout_desc(const AVChannelLayout *layout, AVBPrint *bp)
>   {
> -    av_channel_layout_describe(layout, desc, desc_size);
> -    return desc;
> +    av_bprint_clear(bp);

I don't think this is needed if it's called immediately after 
av_bprint_init().

> +    av_channel_layout_describe_bprintf(layout, bp);
> +    return bp->str;
>   }
>   
>   static void print_codec(const AVCodec *c)
>   {
>       int encoder = av_codec_is_encoder(c);
> -    char desc[128];
> +    AVBPrint desc;
>   
>       printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
>              c->long_name ? c->long_name : "");
> @@ -363,9 +363,12 @@ static void print_codec(const AVCodec *c)
>       PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
>                             "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
>                             "%s", av_get_sample_fmt_name(*fmt));
> +
> +    av_bprint_init(&desc, AV_BPRINT_SIZE_AUTOMATIC);
>       PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
>                             "channel layouts", layout, layout->nb_channels,
> -                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));
> +                          "%s", get_channel_layout_desc(layout, &desc));
> +    av_bprint_finalize(&desc, NULL);

Yeah, this is good. Thanks.

>   
>       if (c->priv_class) {
>           show_help_children(c->priv_class,
> 
>>
>>>    
>>>        if (c->priv_class) {
>>>            show_help_children(c->priv_class,
>>> @@ -566,8 +560,14 @@ static void show_help_bsf(const char *name)
>>>        }
>>>    
>>>        printf("Bit stream filter %s\n", bsf->name);
>>> -    PRINT_CODEC_SUPPORTED(bsf, codec_ids, enum AVCodecID, "codecs",
>>> -                          AV_CODEC_ID_NONE, GET_CODEC_NAME);
>>> +    if (bsf->codec_ids) {
>>> +        const enum AVCodecID *id = bsf->codec_ids;
>>> +        printf("    Supported codecs:");
>>> +        while (*id != AV_CODEC_ID_NONE) {
>>> +            printf(" %s", avcodec_descriptor_get(*id)->name);
>>> +            id++;
>>> +        }
>>> +    }
>>>        if (bsf->priv_class)
>>>            show_help_children(bsf->priv_class, AV_OPT_FLAG_BSF_PARAM);
>>>    }
>> _______________________________________________
>> 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".
Niklas Haas April 8, 2024, 11:15 a.m. UTC | #4
On Sat, 06 Apr 2024 21:10:45 -0300 James Almer <jamrial@gmail.com> wrote:
> On 4/6/2024 8:38 AM, Niklas Haas wrote:
> > On Fri, 05 Apr 2024 16:36:50 -0300 James Almer <jamrial@gmail.com> wrote:
> >> On 4/5/2024 3:57 PM, Niklas Haas wrote:
> >>> From: Niklas Haas <git@haasn.dev>
> >>>
> >>> While rewriting this macro, I decided to make it a bit more flexible so
> >>> it can work for all of the fields (including future fields) in a more
> >>> generic way.
> >>> ---
> >>>    fftools/opt_common.c | 86 ++++++++++++++++++++++----------------------
> >>>    1 file changed, 43 insertions(+), 43 deletions(-)
> >>>
> >>> diff --git a/fftools/opt_common.c b/fftools/opt_common.c
> >>> index 947a226d8d1..1bf66580192 100644
> >>> --- a/fftools/opt_common.c
> >>> +++ b/fftools/opt_common.c
> >>> @@ -262,22 +262,32 @@ int show_buildconf(void *optctx, const char *opt, const char *arg)
> >>>        return 0;
> >>>    }
> >>>    
> >>> -#define PRINT_CODEC_SUPPORTED(codec, field, type, list_name, term, get_name) \
> >>> -    if (codec->field) {                                                      \
> >>> -        const type *p = codec->field;                                        \
> >>> -                                                                             \
> >>> -        printf("    Supported " list_name ":");                              \
> >>> -        while (*p != term) {                                                 \
> >>> -            get_name(*p);                                                    \
> >>> -            printf(" %s", name);                                             \
> >>> -            p++;                                                             \
> >>> -        }                                                                    \
> >>> -        printf("\n");                                                        \
> >>> -    }                                                                        \
> >>> +#define PRINT_CODEC_SUPPORTED(codec, config, type, name, elem, cond, fmt, ...)  \
> >>> +    do {                                                                        \
> >>> +        const type *elem = NULL;                                                \
> >>> +        avcodec_get_supported_config(NULL, codec, config, 0,                    \
> >>> +                                     (const void **) &elem);                    \
> >>> +        if (elem) {                                                             \
> >>> +            printf("    Supported " name ":");                                  \
> >>> +            while (cond) {                                                      \
> >>> +                printf(" " fmt, __VA_ARGS__);                                   \
> >>> +                elem++;                                                         \
> >>> +            }                                                                   \
> >>> +            printf("\n");                                                       \
> >>> +        }                                                                       \
> >>> +    } while (0)
> >>> +
> >>> +static char *get_channel_layout_desc(const AVChannelLayout *layout,
> >>> +                                     char desc[], int desc_size)
> >>> +{
> >>> +    av_channel_layout_describe(layout, desc, desc_size);
> >>> +    return desc;
> >>> +}
> >>>    
> >>>    static void print_codec(const AVCodec *c)
> >>>    {
> >>>        int encoder = av_codec_is_encoder(c);
> >>> +    char desc[128];
> >>>    
> >>>        printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
> >>>               c->long_name ? c->long_name : "");
> >>> @@ -343,35 +353,19 @@ static void print_codec(const AVCodec *c)
> >>>            printf("\n");
> >>>        }
> >>>    
> >>> -    if (c->supported_framerates) {
> >>> -        const AVRational *fps = c->supported_framerates;
> >>> -
> >>> -        printf("    Supported framerates:");
> >>> -        while (fps->num) {
> >>> -            printf(" %d/%d", fps->num, fps->den);
> >>> -            fps++;
> >>> -        }
> >>> -        printf("\n");
> >>> -    }
> >>> -    PRINT_CODEC_SUPPORTED(c, pix_fmts, enum AVPixelFormat, "pixel formats",
> >>> -                          AV_PIX_FMT_NONE, GET_PIX_FMT_NAME);
> >>> -    PRINT_CODEC_SUPPORTED(c, supported_samplerates, int, "sample rates", 0,
> >>> -                          GET_SAMPLE_RATE_NAME);
> >>> -    PRINT_CODEC_SUPPORTED(c, sample_fmts, enum AVSampleFormat, "sample formats",
> >>> -                          AV_SAMPLE_FMT_NONE, GET_SAMPLE_FMT_NAME);
> >>> -
> >>> -    if (c->ch_layouts) {
> >>> -        const AVChannelLayout *p = c->ch_layouts;
> >>> -
> >>> -        printf("    Supported channel layouts:");
> >>> -        while (p->nb_channels) {
> >>> -            char name[128];
> >>> -            av_channel_layout_describe(p, name, sizeof(name));
> >>> -            printf(" %s", name);
> >>> -            p++;
> >>> -        }
> >>> -        printf("\n");
> >>> -    }
> >>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_FRAME_RATE, AVRational, "framerates",
> >>> +                          fps, fps->num, "%d/%d", fps->num, fps->den);
> >>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_PIX_FORMAT, enum AVPixelFormat,
> >>> +                          "pixel formats", fmt, *fmt != AV_PIX_FMT_NONE,
> >>> +                          "%s", av_get_pix_fmt_name(*fmt));
> >>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_RATE, int, "sample rates",
> >>> +                          rate, *rate != 0, "%d", *rate);
> >>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
> >>> +                          "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
> >>> +                          "%s", av_get_sample_fmt_name(*fmt));
> >>> +    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
> >>> +                          "channel layouts", layout, layout->nb_channels,
> >>> +                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));
> >>
> >> Since you're rewriting everything, might as well go and port this to
> >> AVBprint, so we're not limited to 128 bytes for the channel layout.
> >> You can use av_bprintf() to write to the buffer using the different
> >> specifiers, then printf() the final bprint buffer to stdout.
> > 
> > I implemented something like this initially, but it is IMO premature
> > optimization as there is no multi-threading going on in this code, so no
> > risk of colliding printf() calls. printf() already internally buffers
> > data, so we are not gaining anything by buffering it inside an AVBPrint
> > a second time.
> > 
> > That said, using an AVBPrint instead of a fixed size buffer is a good
> > idea for the channel layout specifically, something like this:
> > 
> > 
> > diff --git a/fftools/opt_common.c b/fftools/opt_common.c
> > index 1bf66580192..fae68949f89 100644
> > --- a/fftools/opt_common.c
> > +++ b/fftools/opt_common.c
> > @@ -277,17 +277,17 @@ int show_buildconf(void *optctx, const char *opt, const char *arg)
> >           }                                                                       \
> >       } while (0)
> >   
> > -static char *get_channel_layout_desc(const AVChannelLayout *layout,
> > -                                     char desc[], int desc_size)
> > +static char *get_channel_layout_desc(const AVChannelLayout *layout, AVBPrint *bp)
> >   {
> > -    av_channel_layout_describe(layout, desc, desc_size);
> > -    return desc;
> > +    av_bprint_clear(bp);
> 
> I don't think this is needed if it's called immediately after 
> av_bprint_init().

get_channel_layout_desc() is called in a loop.
> 
> > +    av_channel_layout_describe_bprintf(layout, bp);
> > +    return bp->str;
> >   }
> >   
> >   static void print_codec(const AVCodec *c)
> >   {
> >       int encoder = av_codec_is_encoder(c);
> > -    char desc[128];
> > +    AVBPrint desc;
> >   
> >       printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
> >              c->long_name ? c->long_name : "");
> > @@ -363,9 +363,12 @@ static void print_codec(const AVCodec *c)
> >       PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
> >                             "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
> >                             "%s", av_get_sample_fmt_name(*fmt));
> > +
> > +    av_bprint_init(&desc, AV_BPRINT_SIZE_AUTOMATIC);
> >       PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
> >                             "channel layouts", layout, layout->nb_channels,
> > -                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));
> > +                          "%s", get_channel_layout_desc(layout, &desc));
> > +    av_bprint_finalize(&desc, NULL);
> 
> Yeah, this is good. Thanks.
> 
> >   
> >       if (c->priv_class) {
> >           show_help_children(c->priv_class,
> > 
> >>
> >>>    
> >>>        if (c->priv_class) {
> >>>            show_help_children(c->priv_class,
> >>> @@ -566,8 +560,14 @@ static void show_help_bsf(const char *name)
> >>>        }
> >>>    
> >>>        printf("Bit stream filter %s\n", bsf->name);
> >>> -    PRINT_CODEC_SUPPORTED(bsf, codec_ids, enum AVCodecID, "codecs",
> >>> -                          AV_CODEC_ID_NONE, GET_CODEC_NAME);
> >>> +    if (bsf->codec_ids) {
> >>> +        const enum AVCodecID *id = bsf->codec_ids;
> >>> +        printf("    Supported codecs:");
> >>> +        while (*id != AV_CODEC_ID_NONE) {
> >>> +            printf(" %s", avcodec_descriptor_get(*id)->name);
> >>> +            id++;
> >>> +        }
> >>> +    }
> >>>        if (bsf->priv_class)
> >>>            show_help_children(bsf->priv_class, AV_OPT_FLAG_BSF_PARAM);
> >>>    }
> >> _______________________________________________
> >> 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".
diff mbox series

Patch

diff --git a/fftools/opt_common.c b/fftools/opt_common.c
index 947a226d8d1..1bf66580192 100644
--- a/fftools/opt_common.c
+++ b/fftools/opt_common.c
@@ -262,22 +262,32 @@  int show_buildconf(void *optctx, const char *opt, const char *arg)
     return 0;
 }
 
-#define PRINT_CODEC_SUPPORTED(codec, field, type, list_name, term, get_name) \
-    if (codec->field) {                                                      \
-        const type *p = codec->field;                                        \
-                                                                             \
-        printf("    Supported " list_name ":");                              \
-        while (*p != term) {                                                 \
-            get_name(*p);                                                    \
-            printf(" %s", name);                                             \
-            p++;                                                             \
-        }                                                                    \
-        printf("\n");                                                        \
-    }                                                                        \
+#define PRINT_CODEC_SUPPORTED(codec, config, type, name, elem, cond, fmt, ...)  \
+    do {                                                                        \
+        const type *elem = NULL;                                                \
+        avcodec_get_supported_config(NULL, codec, config, 0,                    \
+                                     (const void **) &elem);                    \
+        if (elem) {                                                             \
+            printf("    Supported " name ":");                                  \
+            while (cond) {                                                      \
+                printf(" " fmt, __VA_ARGS__);                                   \
+                elem++;                                                         \
+            }                                                                   \
+            printf("\n");                                                       \
+        }                                                                       \
+    } while (0)
+
+static char *get_channel_layout_desc(const AVChannelLayout *layout,
+                                     char desc[], int desc_size)
+{
+    av_channel_layout_describe(layout, desc, desc_size);
+    return desc;
+}
 
 static void print_codec(const AVCodec *c)
 {
     int encoder = av_codec_is_encoder(c);
+    char desc[128];
 
     printf("%s %s [%s]:\n", encoder ? "Encoder" : "Decoder", c->name,
            c->long_name ? c->long_name : "");
@@ -343,35 +353,19 @@  static void print_codec(const AVCodec *c)
         printf("\n");
     }
 
-    if (c->supported_framerates) {
-        const AVRational *fps = c->supported_framerates;
-
-        printf("    Supported framerates:");
-        while (fps->num) {
-            printf(" %d/%d", fps->num, fps->den);
-            fps++;
-        }
-        printf("\n");
-    }
-    PRINT_CODEC_SUPPORTED(c, pix_fmts, enum AVPixelFormat, "pixel formats",
-                          AV_PIX_FMT_NONE, GET_PIX_FMT_NAME);
-    PRINT_CODEC_SUPPORTED(c, supported_samplerates, int, "sample rates", 0,
-                          GET_SAMPLE_RATE_NAME);
-    PRINT_CODEC_SUPPORTED(c, sample_fmts, enum AVSampleFormat, "sample formats",
-                          AV_SAMPLE_FMT_NONE, GET_SAMPLE_FMT_NAME);
-
-    if (c->ch_layouts) {
-        const AVChannelLayout *p = c->ch_layouts;
-
-        printf("    Supported channel layouts:");
-        while (p->nb_channels) {
-            char name[128];
-            av_channel_layout_describe(p, name, sizeof(name));
-            printf(" %s", name);
-            p++;
-        }
-        printf("\n");
-    }
+    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_FRAME_RATE, AVRational, "framerates",
+                          fps, fps->num, "%d/%d", fps->num, fps->den);
+    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_PIX_FORMAT, enum AVPixelFormat,
+                          "pixel formats", fmt, *fmt != AV_PIX_FMT_NONE,
+                          "%s", av_get_pix_fmt_name(*fmt));
+    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_RATE, int, "sample rates",
+                          rate, *rate != 0, "%d", *rate);
+    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_SAMPLE_FORMAT, enum AVSampleFormat,
+                          "sample formats", fmt, *fmt != AV_SAMPLE_FMT_NONE,
+                          "%s", av_get_sample_fmt_name(*fmt));
+    PRINT_CODEC_SUPPORTED(c, AV_CODEC_CONFIG_CHANNEL_LAYOUT, AVChannelLayout,
+                          "channel layouts", layout, layout->nb_channels,
+                          "%s", get_channel_layout_desc(layout, desc, sizeof(desc)));
 
     if (c->priv_class) {
         show_help_children(c->priv_class,
@@ -566,8 +560,14 @@  static void show_help_bsf(const char *name)
     }
 
     printf("Bit stream filter %s\n", bsf->name);
-    PRINT_CODEC_SUPPORTED(bsf, codec_ids, enum AVCodecID, "codecs",
-                          AV_CODEC_ID_NONE, GET_CODEC_NAME);
+    if (bsf->codec_ids) {
+        const enum AVCodecID *id = bsf->codec_ids;
+        printf("    Supported codecs:");
+        while (*id != AV_CODEC_ID_NONE) {
+            printf(" %s", avcodec_descriptor_get(*id)->name);
+            id++;
+        }
+    }
     if (bsf->priv_class)
         show_help_children(bsf->priv_class, AV_OPT_FLAG_BSF_PARAM);
 }