[FFmpeg-devel] [PATCH] avformat/tee: Support arbitrary number of slaves

[Nicolas George]

Le tridi 23 prairial, an CCXXIV, sebechlebskyjan at gmail.com a écrit :
> From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
> 
> Signed-off-by: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
> ---
>  libavformat/tee.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)


Out of curiosity, in what situation is this a problem?


> diff --git a/libavformat/tee.c b/libavformat/tee.c
> index 806beaa..427e999 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -27,8 +27,6 @@
>  #include "avformat.h"
>  #include "avio_internal.h"
>  
> -#define MAX_SLAVES 16
> -
>  typedef enum {
>      ON_SLAVE_FAILURE_ABORT  = 1,
>      ON_SLAVE_FAILURE_IGNORE = 2
> @@ -52,7 +50,7 @@ typedef struct TeeContext {
>      const AVClass *class;
>      unsigned nb_slaves;
>      unsigned nb_alive;
> -    TeeSlave slaves[MAX_SLAVES];
> +    TeeSlave *slaves;
>  } TeeContext;
>  
>  static const char *const slave_delim     = "|";
> @@ -203,6 +201,8 @@ static void close_slaves(AVFormatContext *avf)
>      for (i = 0; i < tee->nb_slaves; i++) {
>          close_slave(&tee->slaves[i]);
>      }

> +

Minor nit: I do not think we need an extra line here.

> +    av_freep(&tee->slaves);
>  }
>  
>  static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> @@ -443,17 +443,17 @@ static int tee_write_header(AVFormatContext *avf)
>      TeeContext *tee = avf->priv_data;
>      unsigned nb_slaves = 0, i;
>      const char *filename = avf->filename;
> -    char *slaves[MAX_SLAVES];
> +    char **slaves = NULL;
>      int ret;
>  
>      while (*filename) {
> -        if (nb_slaves == MAX_SLAVES) {
> -            av_log(avf, AV_LOG_ERROR, "Maximum %d slave muxers reached.\n",
> -                   MAX_SLAVES);
> -            ret = AVERROR_PATCHWELCOME;
> +        char *slave = av_get_token(&filename, slave_delim);
> +        if (!slave) {
> +            ret = AVERROR(ENOMEM);
>              goto fail;
>          }
> -        if (!(slaves[nb_slaves++] = av_get_token(&filename, slave_delim))) {
> +        dynarray_add(&slaves, &nb_slaves, slave);
> +        if (!slaves) {
>              ret = AVERROR(ENOMEM);
>              goto fail;
>          }
> @@ -461,6 +461,10 @@ static int tee_write_header(AVFormatContext *avf)
>              filename++;
>      }
>  

> +    if (!(tee->slaves = calloc(nb_slaves, sizeof(*tee->slaves)))) {

Calling calloc() directly is not allowed, use av_mallocz_array() instead.

> +        ret = AVERROR(ENOMEM);
> +        goto fail;
> +    }
>      tee->nb_slaves = tee->nb_alive = nb_slaves;
>  
>      for (i = 0; i < nb_slaves; i++) {
> @@ -483,12 +487,13 @@ static int tee_write_header(AVFormatContext *avf)
>              av_log(avf, AV_LOG_WARNING, "Input stream #%d is not mapped "
>                     "to any slave.\n", i);
>      }
> +    av_free(slaves);
>      return 0;

> -

Stray.

>  fail:
>      for (i = 0; i < nb_slaves; i++)
>          av_freep(&slaves[i]);
>      close_slaves(avf);
> +    av_free(slaves);
>      return ret;
>  }
>  
> @@ -505,6 +510,8 @@ static int tee_write_trailer(AVFormatContext *avf)
>                  ret_all = ret;
>          }
>      }

> +

Ditto about the empty line.

> +    av_freep(&tee->slaves);
>      return ret_all;
>  }

Apart from the calloc() point, LGTM.

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/20160612/45ce000c/attachment.sig>