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 24F0448018 for ; Sun, 7 Apr 2024 00:10:59 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 17D6D68CD00; Sun, 7 Apr 2024 03:10:56 +0300 (EEST) Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 1B63B68CD00 for ; Sun, 7 Apr 2024 03:10:49 +0300 (EEST) Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-6e8f765146fso2821475b3a.0 for ; Sat, 06 Apr 2024 17:10:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712448647; x=1713053447; darn=ffmpeg.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id:from :to:cc:subject:date:message-id:reply-to; bh=FEpHdcmIMCr2pnef7Bny7qBzhX0X+ngYPYqFJtAlfkA=; b=Uyl5yK7WLiesfgYJz2X2kTaZuQ2q0U4pAcRMb1RjkoxiBodXL8S2MQFvNIixUyEjXo CvJoIOdiMShlby+bJRG8faTCLOIVJtMOEumSMRNTTYOLo2NLCWPpCpw+nYYWOW5djOs+ +XPLJFZEwCKBd6OPtmBZxTTGUQpjtzFTW/oQcm4xZHFI3vxpIy8GeFRL5zEY07uS02e0 w1W+Bbf+DVbb0ap5A0PpAPFUxrkwyu3dojdI0W+7Ukga76fbhmCdrCwkHPBDPNO0PYp8 NjKgDTeBjGc0o8pYOipnztnoeaIiI2Mgp6fDvezJ5xtNX2E6TpNm2tOCuK0FbP2Otwa7 cFeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712448647; x=1713053447; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=FEpHdcmIMCr2pnef7Bny7qBzhX0X+ngYPYqFJtAlfkA=; b=FTuRJ9eoJRaUObrdoz3tuwzcVvkGVQSPBwgy1lsSbmIzGrr4pJFRnbWtQpng9P6AZ8 DIyJQfA2sup+e74ITVsJRKn2+LKIJyCU86vxkCm/e8kbDrivkCFz62wPH8fkEoa4YAtY ZehJ5+n6FwDy7aOs7Q00ZsxqNoRhyk+KvLLpNCmH3qfcD28XS9VZhDKR/mSacLDeNv7n V/kE6jda/KuUpIPwdcPK9rRYAFSXcwzsUizFSzagyX+Jtl8U20xQkJYZi9nFkxOU4vgF Pf0VwKorD/rtG8GOM+CvpIYItPPAKepnD9MFdIsEXZ3PpbDvZrkLq1przt4F9+KDmXJ+ 6O5w== X-Gm-Message-State: AOJu0Yx5U7hIkD6fgsHp2WhqpHEe3xcjQO/v8CZS/HNKRk2rR0RIuOtj hTVJfvgqAahBGlW09dRsz2HrBQyeV7TF5xGWIsik6s/cfdIBphlPOUprRnEy X-Google-Smtp-Source: AGHT+IHZYOrWIxm7r53KyzmYrm5jVKmAAzfpeJ7oURETF/HKcVBiccXoYLjMawAQYFfisI9TCR/k2w== X-Received: by 2002:a05:6a20:7f8b:b0:1a7:5f05:9a34 with SMTP id d11-20020a056a207f8b00b001a75f059a34mr158225pzj.8.1712448646797; Sat, 06 Apr 2024 17:10:46 -0700 (PDT) Received: from [192.168.0.15] ([190.194.167.233]) by smtp.gmail.com with ESMTPSA id m10-20020a170902db0a00b001e2a0d33fbbsm3952512plx.219.2024.04.06.17.10.45 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 06 Apr 2024 17:10:46 -0700 (PDT) Message-ID: Date: Sat, 6 Apr 2024 21:10:45 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: ffmpeg-devel@ffmpeg.org References: <20240405185721.111072-1-ffmpeg@haasn.xyz> <20240405185721.111072-8-ffmpeg@haasn.xyz> <3de40d76-d1eb-481b-bee0-7b69d398e289@gmail.com> <20240406133857.GB33922@haasn.xyz> Content-Language: en-US From: James Almer In-Reply-To: <20240406133857.GB33922@haasn.xyz> Subject: Re: [FFmpeg-devel] [PATCH 08/11] fftools/opt_common: switch to avcodec_get_supported_config() 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 4/6/2024 8:38 AM, Niklas Haas wrote: > On Fri, 05 Apr 2024 16:36:50 -0300 James Almer wrote: >> On 4/5/2024 3:57 PM, Niklas Haas wrote: >>> From: Niklas Haas >>> >>> 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". _______________________________________________ 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".