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

[Bodecs Bela]

2015.10.04. 20:06 keltezéssel, Nicolas George írta:
> Le tridi 13 vendémiaire, an CCXXIV, Bodecs Bela a écrit :
>> git format-patch -n -o /tmp/ --attach  origin
> Thanks. Any particular reason to use --attach? It is not a big issue, just a
> matter of curiosity.
Now, I used a different method to create the patch file.
>>>> @@ -172,15 +175,26 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>>>>       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);
>>>> -                goto end;
>>>> -            }
>>>> +            fullret = 0;
>>>> +            first_subselect = select;
>>>> +            next_subselect = NULL;
>>>> +            while (subselect = av_strtok(first_subselect, slave_select_sep, &next_subselect)) {
>>>> +                first_subselect = NULL;
>> You would be right unless I used
>>
>> first_subselect = NULL;
>>
>> right after the while statement, so the next round av_strtok will go
>> further, because the first parameter will be null, not the same string and
>> next_subselect will point the next start.
>> Yes it is destructive, but we never use this string (select) again in this
>> function.
> I suspect we are talking at cross-purpose here. The "first_subselect = NULL"
> you are talking about is in the while loop for av_strtok(), it is indeed the
> normal way of using av_strtok().
>
> But I was talking about the surrounding for loop that goes over all the
> streams. I left the code quoted above for reference. For i=0, the code is
> valid, but as soon as i>=1, "first_subselect = select" will use a truncated
> string
>
>> I swear, I have really tested it, I use it on my own  in my production
>> environment.
> I do not believe that you neglected to test it (nor that you are lying, of
> course!), I am just really surprised. Just to be sure, I just tested, adding
> a debug log just after "first_subselect = select", with the following
> command-line:
>
> ./ffmpeg_g -lavfi 'testsrc=r=5;testsrc=r=7;testsrc=r=11' \
>    -c:v rawvideo -f tee '[f=framecrc:select=0,1]-'
>
> I get the following output:
>
> select = '0,1'
> select = '0'
>      Last message repeated 1 times
> [tee @ 0x36143e0] Input stream #1 is not mapped to any slave.
> [tee @ 0x36143e0] Input stream #2 is not mapped to any slave.
>
> The second "select =" line shows that it is working with a truncated line,
> and stream #1 should be mapped. Indeed, if I swap the specifiers: "1,0", it
> maps #0 and #1: #0 is matched first while the string is still intact, #1 is
> matched by the truncated string.
>
> Regards,
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
I have altered the code to work correctly in situations with multiple 
streams. I have also tested it with your  command.

best regards,

bb

-------------- next part --------------
diff --git a/doc/muxers.texi b/doc/muxers.texi
index 91d131f..610ff58 100644
--- 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;
             }
+            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);
+
+            if (fullret == 0) { /* no match */
                 tee_slave->stream_map[i] = -1;
                 continue;
             }