[FFmpeg-devel] [PATCH]select attribute of tee pseudo demuxer may contain multiple stream specifiers

[Nicolas George]

L'octidi 18 vendémiaire, an CCXXIV, Bodecs Bela a écrit :
> I have altered the code to work correctly in situations with multiple
> streams. I have also tested it with your  command.

Thanks, it looks almost good for apply.

Your other patches were made with git format-patch, so that is ok; do not
forget the "signed-off-by" tag (--signoff option), just to be on the safe
side legally.

> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -1272,7 +1272,8 @@
>  @item select
>  Select the streams that should be mapped to the slave output,
>  specified by a stream specifier. If not specified, this defaults to
> -all the input streams.
> +all the input streams. You may use multiple stream specifiers 
> +separated by commas (@code{,}) e.g.: @code{a:0,v}
>  @end table
>  
>  @subsection Examples
> diff --git a/libavformat/tee.c b/libavformat/tee.c
> index c619eae..324aa43 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -47,6 +47,7 @@
>  static const char *const slave_opt_close = "]";
>  static const char *const slave_opt_delim = ":]"; /* must have the close too */
>  static const char *const slave_bsfs_spec_sep = "/";
> +static const char *const slave_select_sep = ",";
>  
>  static const AVClass tee_muxer_class = {
>      .class_name = "Tee muxer",
> @@ -142,6 +143,8 @@
>      AVFormatContext *avf2 = NULL;
>      AVStream *st, *st2;
>      int stream_count;
> +    int fullret;
> +    char *subselect = NULL, *next_subselect = NULL, *first_subselect = NULL, *tmp_select = NULL;
>  
>      if ((ret = parse_slave_options(avf, slave, &options, &filename)) < 0)
>          return ret;
> @@ -172,15 +175,33 @@
>      for (i = 0; i < avf->nb_streams; i++) {
>          st = avf->streams[i];
>          if (select) {
> -            ret = avformat_match_stream_specifier(avf, avf->streams[i], select);
> -            if (ret < 0) {
> -                av_log(avf, AV_LOG_ERROR,
> -                       "Invalid stream specifier '%s' for output '%s'\n",
> -                       select, slave);

> +            tmp_select = av_strdup(select);  // av_strtok is destructive so we regenerate it in each loop
> +            if (!tmp_select) {
> +                av_log(avf, AV_LOG_ERROR, "Unexpected internal memory allocation error occured during string duplication\n");
>                  goto end;

ret needs to be set to some value before jumping to end. Setting it to
AVERROR(ENOMEM) makes the error message unnecessary; the code usually does
not print a message for OOM.

>              }
> +            fullret = 0;
> +            first_subselect = tmp_select;
> +            next_subselect = NULL;
> +            while (subselect = av_strtok(first_subselect, slave_select_sep, &next_subselect)) {
> +                first_subselect = NULL;
>  
> -            if (ret == 0) { /* no match */
> +                ret = avformat_match_stream_specifier(avf, avf->streams[i], subselect);
> +                if (ret < 0) {
> +                    av_log(avf, AV_LOG_ERROR,
> +                           "Invalid stream specifier '%s' for output '%s'\n",
> +                           subselect, slave);

> +                    av_free(tmp_select);
> +                    goto end;
> +                }
> +                if (ret != 0) {
> +                    fullret = 1; // match
> +                    break;
> +                }
> +            }
> +            av_free(tmp_select);

This code is safe. For extra robustness, it could ensure that tmp_select is
NULL when not used (initing to NULL and using av_freep()), and move the free
in the error case to the end clause. But I do not insist on that change.

> +
> +            if (fullret == 0) { /* no match */
>                  tee_slave->stream_map[i] = -1;
>                  continue;
>              }

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151010/8f808240/attachment.sig>