Re: [FFmpeg-devel] [PATCH v2] ffprobe: add -codec:<media_spec> option

[Maryla Ustarroz via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>]

On Thu, Jul 31, 2025 at 12:01 AM Stefano Sabatini <stefasab@gmail.com> wrote:
>
> On date Friday 2025-06-27 10:23:56 +0200, ffmpeg-devel Mailing List wrote:
> > From: Maryla Ustarroz-Calonge <maryla@google.com>
> > Subject: [PATCH v2] ffprobe: add -codec:<media_spec> option
> > Date: Fri, 27 Jun 2025 10:23:56 +0200
> > To: ffmpeg-devel@ffmpeg.org
> > X-Mailer: git-send-email 2.50.0.727.gbf7dc18ff4-goog
> >
> > And -c:<media_spec> variant.
> > Update ffprobe.texi.
> > opt_codec() is mostly copied over from ffplay.c
> >
> > Signed-off-by: Maryla Ustarroz-Calonge <maryla@google.com>
> > ---
> >  Changelog         |   2 +-
> >  doc/ffprobe.texi  |   8 ++++
> >  fftools/ffprobe.c | 117 +++++++++++++++++++++++++++++++++++++++++-----
> >  3 files changed, 114 insertions(+), 13 deletions(-)
> >
> > diff --git a/Changelog b/Changelog
> > index 4217449438..ae73611222 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -18,7 +18,7 @@ version <next>:
> >  - APV encoding support through a libopenapv wrapper
> >  - VVC decoder supports all content of SCC (Screen Content Coding):
> >    IBC (Inter Block Copy), Palette Mode and ACT (Adaptive Color Transform
> > -
> > +- ffprobe -codec option
> >
> >  version 7.1:
> >  - Raw Captions with Time (RCWT) closed caption demuxer
> > diff --git a/doc/ffprobe.texi b/doc/ffprobe.texi
> > index 8834df8d35..4ce0c8b583 100644
> > --- a/doc/ffprobe.texi
> > +++ b/doc/ffprobe.texi
> > @@ -368,6 +368,14 @@ Read @var{input_url}.
> >  Write output to @var{output_url}. If not specified, the output is sent
> >  to stdout.
> >
> > +@item -c:@var{media_specifier} @var{codec_name}
> > +Force a specific decoder implementation for the stream identified by
> > +@var{media_specifier}, which can assume the values @code{a} (audio),
> > +@code{v} (video), @code{s} (subtitle), and @code{d} (data).
> > +
> > +@item -codec:@var{media_specifier} @var{codec_name}
> > +Alias for -c:@var{media_specifier}.
>
> Nit: I'd put the long version before the short one, but keep as is if
> this is consistent with the other docs.

I just sent a v3 addressing your comments.
In the texi file, I merged the two entries for -c and -codec with -c first,
as is done in ffmpeg.texi
Note that in the flag descriptions in ffprobe.c, -codec is marked as being
an alias of -c, which seems backwards to me, but I did it this way to be
consistent with ffmpeg.c


> > +
> >  @end table
> >  @c man end
> >
> > diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> > index 1346ed33c5..14ba4d3fb0 100644
> > --- a/fftools/ffprobe.c
> > +++ b/fftools/ffprobe.c
> > @@ -36,6 +36,7 @@
> >  #include "libavutil/ambient_viewing_environment.h"
> >  #include "libavutil/avassert.h"
> >  #include "libavutil/avstring.h"
> > +#include "libavutil/avutil.h"
> >  #include "libavutil/bprint.h"
> >  #include "libavutil/channel_layout.h"
> >  #include "libavutil/display.h"
> > @@ -130,6 +131,11 @@ static int use_byte_value_binary_prefix = 0;
> >  static int use_value_sexagesimal_format = 0;
> >  static int show_private_data            = 1;
> >
> > +static const char *audio_codec_name = NULL;
> > +static const char *data_codec_name = NULL;
> > +static const char *subtitle_codec_name = NULL;
> > +static const char *video_codec_name = NULL;
> > +
> >  #define SHOW_OPTIONAL_FIELDS_AUTO       -1
> >  #define SHOW_OPTIONAL_FIELDS_NEVER       0
> >  #define SHOW_OPTIONAL_FIELDS_ALWAYS      1
> > @@ -2284,6 +2290,64 @@ static void show_error(AVTextFormatContext *tfc, int err)
> >      avtext_print_section_footer(tfc);
> >  }
> >
> > +static int get_decoder_by_name(const char *codec_name, const AVCodec **codec)
> > +{
> > +    if (codec_name == NULL)
> > +        return 0;
> > +
> > +    *codec = avcodec_find_decoder_by_name(codec_name);
> > +    if (*codec == NULL) {
> > +        av_log(NULL, AV_LOG_ERROR,
> > +                "No codec could be found with name '%s'\n", codec_name);
> > +        return AVERROR(EINVAL);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int set_decoders(AVFormatContext *fmt_ctx)
> > +{
> > +    int ret;
>
> > +    ret = get_decoder_by_name(audio_codec_name, &fmt_ctx->audio_codec);
> > +    if (ret < 0) return ret;
>
> nit++: could be turned to a macro to capture the pattern:
>
> #define GET_DECODER(type_) \
>     ret = get_decoder_by_name(type_##_codec_name, &fmt_ctx->type_##_codec);\
>     if (ret < 0) return ret;
>
> GET_DECODER(audio);
> GET_DECODER(data);
> ...
>
> > +    ret = get_decoder_by_name(data_codec_name, &fmt_ctx->data_codec);
> > +    if (ret < 0) return ret;
> > +    ret = get_decoder_by_name(subtitle_codec_name, &fmt_ctx->subtitle_codec);
> > +    if (ret < 0) return ret;
> > +    ret = get_decoder_by_name(video_codec_name, &fmt_ctx->video_codec);
> > +    if (ret < 0) return ret;
> > +    return 0;
> > +}
> > +
>
> Looks good apart from the minor nits, you can update the patch with
> the extended commit description otherwise I'd push this in a few days
> if there are no more comments.
>
> Thanks.
_______________________________________________
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".