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

[Jan Sebechlebsky]

On 06/12/2016 04:26 PM, Nicolas George wrote:

Le tridi 23 prairial, an CCXXIV, sebechlebsky...@gmail.com a écrit :

From: Jan Sebechlebsky 

Signed-off-by: Jan Sebechlebsky 
---
  libavformat/tee.c | 27 +--
  1 file changed, 17 insertions(+), 10 deletions(-)


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

I don't think someone will ever need more than 16 slaves,
but why not :)

Thanks for review!

Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[Nicolas George]

Le tridi 23 prairial, an CCXXIV, sebechlebsky...@gmail.com a écrit :
> From: Jan Sebechlebsky 
> 
> Signed-off-by: Jan Sebechlebsky 
> ---
>  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(>slaves[i]);
>  }

> +

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

> +av_freep(>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(, slave_delim);
> +if (!slave) {
> +ret = AVERROR(ENOMEM);
>  goto fail;
>  }
> -if (!(slaves[nb_slaves++] = av_get_token(, slave_delim))) {
> +dynarray_add(, _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([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(>slaves);
>  return ret_all;
>  }

Apart from the calloc() point, LGTM.

Regards,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[sebechlebskyjan]

From: Jan Sebechlebsky 

Signed-off-by: Jan Sebechlebsky 
---
 libavformat/tee.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

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(>slaves[i]);
 }
+
+av_freep(>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(, slave_delim);
+if (!slave) {
+ret = AVERROR(ENOMEM);
 goto fail;
 }
-if (!(slaves[nb_slaves++] = av_get_token(, slave_delim))) {
+dynarray_add(, _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 {
+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;
-
 fail:
 for (i = 0; i < nb_slaves; i++)
 av_freep([i]);
 close_slaves(avf);
+av_free(slaves);
 return ret;
 }
 
@@ -505,6 +510,8 @@ static int tee_write_trailer(AVFormatContext *avf)
 ret_all = ret;
 }
 }
+
+av_freep(>slaves);
 return ret_all;
 }
 
-- 
1.9.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel