[FFmpeg-devel] [PATCH] ffplay: fix and extend option -codec option

[Marton Balint]

On Sat, 8 Dec 2012, Stefano Sabatini wrote:

> Allow to specify the stream for which the codec name is set. Also make
> parsing and error reporting more robust.
> ---
> doc/ffplay.texi |    6 ++++--
> ffplay.c        |   50 ++++++++++++++++++++++++++++++++++++++++----------
> 2 files changed, 44 insertions(+), 12 deletions(-)
>
> diff --git a/doc/ffplay.texi b/doc/ffplay.texi
> index e2bded7..7923492 100644
> --- a/doc/ffplay.texi
> +++ b/doc/ffplay.texi
> @@ -134,8 +134,10 @@ Exit when video is done playing.
> Exit if any key is pressed.
> @item -exitonmousedown
> Exit if any mouse button is pressed.
> - at item -codec:@var{stream_type}
> -Force a specific decoder implementation
> +
> + at item -codec:@var{stream_specifier} @var{codec_name}
> +Force a specific decoder implementation for the stream specified by
> + at var{stream_specifier}.
> @end table
>
> @section While playing
> diff --git a/ffplay.c b/ffplay.c
> index d6b17c2..f4f924d 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -92,6 +92,13 @@ const int program_birth_year = 2003;
>
> static int64_t sws_flags = SWS_BICUBIC;
>
> +typedef struct {
> +    const char *codec_name, *stream_specifier;
> +} StreamCodecSpecifier;
> +
> +StreamCodecSpecifier *stream_codec_specifiers = NULL;
> +int                nb_stream_codec_specifiers = 0;
> +
> typedef struct MyAVPacketList {
>     AVPacket pkt;
>     struct MyAVPacketList *next;
> @@ -299,9 +306,6 @@ static int loop = 1;
> static int framedrop = -1;
> static int infinite_buffer = -1;
> static enum ShowMode show_mode = SHOW_MODE_NONE;
> -static const char *audio_codec_name;
> -static const char *subtitle_codec_name;
> -static const char *video_codec_name;
> static int rdftspeed = 20;
> #if CONFIG_AVFILTER
> static char *vfilters = NULL;
> @@ -2302,6 +2306,7 @@ static int stream_component_open(VideoState *is, int stream_index)
>     AVCodec *codec;
>     AVDictionary *opts;
>     AVDictionaryEntry *t = NULL;
> +    int i, ret;
>
>     if (stream_index < 0 || stream_index >= ic->nb_streams)
>         return -1;
> @@ -2310,9 +2315,24 @@ static int stream_component_open(VideoState *is, int stream_index)
>     codec = avcodec_find_decoder(avctx->codec_id);
>
>     switch(avctx->codec_type){
> -        case AVMEDIA_TYPE_AUDIO   : is->last_audio_stream    = stream_index; if(audio_codec_name   ) codec= avcodec_find_decoder_by_name(   audio_codec_name); break;
> -        case AVMEDIA_TYPE_SUBTITLE: is->last_subtitle_stream = stream_index; if(subtitle_codec_name) codec= avcodec_find_decoder_by_name(subtitle_codec_name); break;
> -        case AVMEDIA_TYPE_VIDEO   : is->last_video_stream    = stream_index; if(video_codec_name   ) codec= avcodec_find_decoder_by_name(   video_codec_name); break;
> +        case AVMEDIA_TYPE_AUDIO   : is->last_audio_stream    = stream_index;
> +        case AVMEDIA_TYPE_SUBTITLE: is->last_subtitle_stream = stream_index;
> +        case AVMEDIA_TYPE_VIDEO   : is->last_video_stream    = stream_index;
> +    }
> +
> +    for (i = 0; i < nb_stream_codec_specifiers; i++) {
> +        ret = avformat_match_stream_specifier(ic, ic->streams[stream_index],
> +                                              stream_codec_specifiers[i].stream_specifier);
> +        if (ret > 0) {
> +            codec = avcodec_find_decoder_by_name(stream_codec_specifiers[i].codec_name);
> +            avctx->codec_id = codec->id;

Codec may be null, you should either check for !codec here, or put the 
assignment to the main code path after the !codec check later in the code.

> +            break;
> +        }
> +        if (ret < 0) {
> +            fprintf(stderr, "Invalid stream specifier '%s' was specified\n",
> +                    stream_codec_specifiers[i].stream_specifier);
> +            return AVERROR(EINVAL);
> +        }
>     }
>     if (!codec)
>         return -1;
> @@ -3149,11 +3169,21 @@ static void opt_input_file(void *optctx, const char *filename)
>
> static int opt_codec(void *o, const char *opt, const char *arg)
> {
> -    switch(opt[strlen(opt)-1]){
> -    case 'a' :    audio_codec_name = arg; break;
> -    case 's' : subtitle_codec_name = arg; break;
> -    case 'v' :    video_codec_name = arg; break;
> +    const char *specifier = strchr(opt, ':');
> +    if (!specifier) {
> +        fprintf(stderr, "No stream specifier was specified in option name '%s'\n", opt);
> +        return AVERROR(EINVAL);
>     }
> +    specifier++;
> +
> +    stream_codec_specifiers =
> +        grow_array(stream_codec_specifiers, sizeof(StreamCodecSpecifier),
> +                   &nb_stream_codec_specifiers, nb_stream_codec_specifiers+1);
> +    if (!stream_codec_specifiers)
> +        return AVERROR(ENOMEM);
> +
> +    stream_codec_specifiers[nb_stream_codec_specifiers-1].stream_specifier = specifier;
> +    stream_codec_specifiers[nb_stream_codec_specifiers-1].codec_name       = arg;
>     return 0;
> }
>

Otherwise LGTM, thanks.

Regards,
Marton