Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Maryla Ustarroz via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
To: FFmpeg development discussions and patches
	<ffmpeg-devel@ffmpeg.org>,
	Maryla Ustarroz-Calonge <maryla@google.com>
Cc: Maryla Ustarroz <maryla@google.com>
Subject: Re: [FFmpeg-devel] [PATCH] ffprobe: add -codec:<media_spec> option
Date: Tue, 29 Jul 2025 15:15:55 +0200
Message-ID: <CA+yX6GH34L0_HmM=mf-sR3MSrAienCBYLA6ywqfQ=m2ACLpXTQ@mail.gmail.com> (raw)
In-Reply-To: <aIfYhvLaBgenx9SP@mariano>

On Mon, Jul 28, 2025 at 10:07 PM Stefano Sabatini <stefasab@gmail.com> wrote:
>
> Hi, sorry for the slow reply.
>
> On date Tuesday 2025-06-10 17:02:05 +0200, ffmpeg-devel Mailing List wrote:
> > From: Maryla Ustarroz-Calonge <maryla@google.com>
> > Subject: [PATCH] ffprobe: add -codec:<media_spec> option
> > Date: Tue, 10 Jun 2025 17:02:05 +0200
> > To: ffmpeg-devel@ffmpeg.org
> > X-Mailer: git-send-email 2.50.0.rc1.591.g9c95f17f64-goog
> >
> > opt_codec() is mostly copied over from ffplay.c
> >
> > Signed-off-by: Maryla Ustarroz-Calonge <maryla@google.com>
> > ---
> >  Changelog         |   2 +-
> >  fftools/ffprobe.c | 114 +++++++++++++++++++++++++++++++++++++++++-----
> >  2 files changed, 103 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/fftools/ffprobe.c b/fftools/ffprobe.c
> > index 1346ed33c5..0124ce114c 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);;
>
> nit: drop duplicated ;

Thank you for the review.
This is already fixed in v2 of the patch.
https://ffmpeg.org/pipermail/ffmpeg-devel/2025-June/345891.html
V2 also adds a shorter -c:<media_specifier> version of the flag, as
suggested by Marvin.

> > +    }
> > +    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;
> > +    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;
> > +}
> > +
> > +static const AVCodec *get_decoder_for_stream(AVFormatContext *fmt_ctx, AVStream *stream)
> > +{
> > +    const AVCodec *codec = NULL;
> > +    switch (stream->codecpar->codec_type) {
> > +        case AVMEDIA_TYPE_VIDEO:    codec = fmt_ctx->video_codec; break;
> > +        case AVMEDIA_TYPE_AUDIO:    codec = fmt_ctx->audio_codec; break;
> > +        case AVMEDIA_TYPE_SUBTITLE: codec = fmt_ctx->subtitle_codec; break;
> > +        case AVMEDIA_TYPE_DATA:     codec = fmt_ctx->data_codec; break;
> > +    }
> > +
> > +    if (codec != NULL)
> > +        return codec;
> > +
> > +    if (stream->codecpar->codec_id == AV_CODEC_ID_PROBE) {
> > +        av_log(NULL, AV_LOG_WARNING,
> > +               "Failed to probe codec for input stream %d\n", stream->index);
> > +        return NULL;
> > +    }
> > +
> > +    codec = avcodec_find_decoder(stream->codecpar->codec_id);
> > +    if (codec == NULL) {
> > +        av_log(NULL, AV_LOG_WARNING,
> > +               "Unsupported codec with id %d for input stream %d\n",
> > +               stream->codecpar->codec_id, stream->index);
> > +        return NULL;
> > +    }
> > +
> > +    return codec;
> > +}
> > +
> >  static int open_input_file(InputFile *ifile, const char *filename,
> >                             const char *print_filename)
> >  {
> > @@ -2296,6 +2360,9 @@ static int open_input_file(InputFile *ifile, const char *filename,
> >      if (!fmt_ctx)
> >          return AVERROR(ENOMEM);
> >
> > +    err = set_decoders(fmt_ctx);
> > +    if (err < 0)
> > +        return err;
> >      if (!av_dict_get(format_opts, "scan_all_pmts", NULL, AV_DICT_MATCH_CASE)) {
> >          av_dict_set(&format_opts, "scan_all_pmts", "1", AV_DICT_DONT_OVERWRITE);
> >          scan_all_pmts_set = 1;
> > @@ -2350,20 +2417,10 @@ static int open_input_file(InputFile *ifile, const char *filename,
> >
> >          ist->st = stream;
> >
> > -        if (stream->codecpar->codec_id == AV_CODEC_ID_PROBE) {
> > -            av_log(NULL, AV_LOG_WARNING,
> > -                   "Failed to probe codec for input stream %d\n",
> > -                    stream->index);
> > +        codec = get_decoder_for_stream(fmt_ctx, stream);
> > +        if (!codec)
> >              continue;
> > -        }
> >
> > -        codec = avcodec_find_decoder(stream->codecpar->codec_id);
> > -        if (!codec) {
> > -            av_log(NULL, AV_LOG_WARNING,
> > -                    "Unsupported codec with id %d for input stream %d\n",
> > -                    stream->codecpar->codec_id, stream->index);
> > -            continue;
> > -        }
> >          {
> >              AVDictionary *opts;
> >
> > @@ -2510,6 +2567,10 @@ end:
> >      av_freep(&selected_streams);
> >      av_freep(&streams_with_closed_captions);
> >      av_freep(&streams_with_film_grain);
> > +    av_freep(&audio_codec_name);
> > +    av_freep(&data_codec_name);
> > +    av_freep(&subtitle_codec_name);
> > +    av_freep(&video_codec_name);
> >
> >      return ret;
> >  }
> > @@ -2964,6 +3025,34 @@ static int opt_sections(void *optctx, const char *opt, const char *arg)
> >      return 0;
> >  }
> >
> > +static int opt_codec(void *optctx, const char *opt, const char *arg)
> > +{
> > +   const char *spec = strchr(opt, ':');
> > +   const char **name;
> > +   if (!spec) {
> > +       av_log(NULL, AV_LOG_ERROR,
> > +              "No media specifier was specified for '%s' in option '%s'. Use -%s:<media_spec>\n",
> > +               arg, opt, opt);
> > +       return AVERROR(EINVAL);
> > +   }
> > +   spec++;
> > +
> > +   switch (spec[0]) {
> > +   case 'a' : name = &audio_codec_name;    break;
> > +   case 'd' : name = &data_codec_name;     break;
> > +   case 's' : name = &subtitle_codec_name; break;
> > +   case 'v' : name = &video_codec_name;    break;
> > +   default:
> > +       av_log(NULL, AV_LOG_ERROR,
> > +              "Invalid media specifier '%s' in option '%s'. Must be one of: v, a, s, d\n", spec, opt);
> > +       return AVERROR(EINVAL);
> > +   }
> > +
> > +   av_freep(name);
> > +   *name = av_strdup(arg);
> > +   return *name ? 0 : AVERROR(ENOMEM);
> > +}
> > +
>
> Not super happy that this is duplicated but I see no sane way to
> factorize it.
>
> >  static int opt_show_versions(void *optctx, const char *opt, const char *arg)
> >  {
> >      mark_section_show_entries(SECTION_ID_PROGRAM_VERSION, 1, NULL);
> > @@ -3039,6 +3128,7 @@ static const OptionDef real_options[] = {
> >      { "print_filename",        OPT_TYPE_FUNC, OPT_FUNC_ARG, {.func_arg = opt_print_filename}, "override the printed input filename", "print_file"},
> >      { "find_stream_info",      OPT_TYPE_BOOL, OPT_INPUT | OPT_EXPERT, { &find_stream_info },
> >          "read and decode the streams to fill missing information with heuristics" },
> > +    { "codec",                 OPT_TYPE_FUNC, OPT_FUNC_ARG, { .func_arg = opt_codec}, "force decoder", "decoder_name" },
> >      { NULL, },
> >  };
>
> Please also add an update for the docs and a short explanation of the
> use case in the commit log.
In v2 I added a line to the changelog and also updated ffprobe.texi
When you say "commit log", do you mean the changelog? Or the patch description?
I can add the following wherever is appropriate:
"Useful when different decoders have different capabilities, or to
test a specific decoder".

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

      reply	other threads:[~2025-07-29 13:16 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-10 15:02 Maryla Ustarroz-Calonge via ffmpeg-devel
2025-06-19 15:02 ` Maryla Ustarroz via ffmpeg-devel
2025-06-25 19:23 ` James Zern via ffmpeg-devel
     [not found] ` <CABWgkXL9s=RAkKQZqoaBGsdJbFZRXVay9NGWb+pB0R8ZAXpopQ@mail.gmail.com>
2025-06-26 10:15   ` Maryla Ustarroz via ffmpeg-devel
2025-06-26 14:19     ` Marvin Scholz
2025-07-28 20:07 ` Stefano Sabatini
2025-07-29 13:15   ` Maryla Ustarroz via ffmpeg-devel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+yX6GH34L0_HmM=mf-sR3MSrAienCBYLA6ywqfQ=m2ACLpXTQ@mail.gmail.com' \
    --to=ffmpeg-devel@ffmpeg.org \
    --cc=maryla@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git