On 04/15/2016 02:28 AM, Marton Balint wrote:

On Thu, 14 Apr 2016, Marton Balint wrote:


On Tue, 12 Apr 2016, sebechlebsky...@gmail.com wrote:

From: Jan Sebechlebsky <sebechlebsky...@gmail.com>

Calling close_slave in case error is to be returned from open_slave
will free allocated resources.

Since failure can happen before bsfs array is initialized,
close_slave must check that bsfs is not NULL before accessing
tee_slave->bsfs[i] element.

Slave muxer expects write_trailer to be called if it's
write_header suceeded (so resources allocated in write_header
are freed). Therefore if failure happens after successfull
write_header call, we must ensure that write_trailer of
that particular slave is called.

Hmm, I guess you are right, I see no other way freeing resources allocated in write_header then calling write_trailer. It does make the code a bit more complex, but I don't really see a way to make it more simple.

So this looks good to me. Nicolas, any ideas improving this?


Actually I have given this some additional thought, and by using a new per-slave variable to keep the information if the header was written or not, I think your patch can be simplifed, also close_slave can be changed so it will write the trailer if necessary, in fact, the write_trailer function can use it as well.

I have modified your patch (see attached), could you please test/review it and check if I missed anything? I hope you don't mind, this kind of collaborative work is not that common in ffmpeg, but in this case it seemed easier moving those few lines around than describing what I wanted.

Thanks,
Marton

Hello Marton,
I'm ok with it, you're right it is more elegant this way. I've tested it and it seems allright. I've recreated the last patch on top of these changes and I'm sending it in attachment (and I am also cc-ing this mail to Nicolas, so he can review the patches).

Have a nice day!

Jan

>From 81901087b1cda032fc9688f436974578359bfb36 Mon Sep 17 00:00:00 2001
From: Jan Sebechlebsky <sebechlebsky...@gmail.com>
Date: Fri, 15 Apr 2016 17:33:59 +0300
Subject: [PATCH v7 3/3] avformat/tee: Handling slave failure in tee muxer

Adds per slave option 'onfail' to the tee muxer allowing an output to
fail,so other slave outputs can continue.

Signed-off-by: Jan Sebechlebsky <sebechlebsky...@gmail.com>
---
 doc/muxers.texi   | 14 ++++++++
 libavformat/tee.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 100 insertions(+), 12 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 042efce..c62d4b5 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1453,6 +1453,12 @@ 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. You may use multiple stream specifiers
 separated by commas (@code{,}) e.g.: @code{a:0,v}
+
+@item onfail
+Specify behaviour on output failure. This can be set to either @code{abort} (which is
+default) or @code{ignore}. @code{abort} will cause whole process to fail in case of failure
+on this slave output. @code{ignore} will ignore failure on this output, so other outputs
+will continue without being affected.
 @end table
 
 @subsection Examples
@@ -1467,6 +1473,14 @@ ffmpeg -i ... -c:v libx264 -c:a mp2 -f tee -map 0:v -map 0:a
 @end example
 
 @item
+As above, but continue streaming even if output to local file fails
+(for example local drive fills up):
+@example
+ffmpeg -i ... -c:v libx264 -c:a mp2 -f tee -map 0:v -map 0:a
+  "[onfail=ignore]archive-20121107.mkv|[f=mpegts]udp://10.0.1.255:1234/"
+@end example
+
+@item
 Use @command{ffmpeg} to encode the input, and send the output
 to three different destinations. The @code{dump_extra} bitstream
 filter is used to add extradata information to all the output video
diff --git a/libavformat/tee.c b/libavformat/tee.c
index 753f7ea..2aa3071 100644
--- a/libavformat/tee.c
+++ b/libavformat/tee.c
@@ -29,10 +29,19 @@
 
 #define MAX_SLAVES 16
 
+typedef enum {
+    ON_SLAVE_FAILURE_ABORT  = 1,
+    ON_SLAVE_FAILURE_IGNORE = 2
+} SlaveFailurePolicy;
+
+#define DEFAULT_SLAVE_FAILURE_POLICY ON_SLAVE_FAILURE_ABORT
+
 typedef struct {
     AVFormatContext *avf;
     AVBitStreamFilterContext **bsfs; ///< bitstream filters per stream
 
+    SlaveFailurePolicy on_fail;
+
     /** map from input to output streams indexes,
      * disabled output streams are set to -1 */
     int *stream_map;
@@ -42,6 +51,7 @@ typedef struct {
 typedef struct TeeContext {
     const AVClass *class;
     unsigned nb_slaves;
+    unsigned nb_alive;
     TeeSlave slaves[MAX_SLAVES];
 } TeeContext;
 
@@ -136,6 +146,23 @@ end:
     return ret;
 }
 
+static inline int parse_slave_failure_policy_option(const char *opt, TeeSlave *tee_slave)
+{
+    if (!opt) {
+        tee_slave->on_fail = DEFAULT_SLAVE_FAILURE_POLICY;
+        return 0;
+    } else if (!av_strcasecmp("abort", opt)) {
+        tee_slave->on_fail = ON_SLAVE_FAILURE_ABORT;
+        return 0;
+    } else if (!av_strcasecmp("ignore", opt)) {
+        tee_slave->on_fail = ON_SLAVE_FAILURE_IGNORE;
+        return 0;
+    }
+    /* Set failure behaviour to abort, so invalid option error will not be ignored */
+    tee_slave->on_fail = ON_SLAVE_FAILURE_ABORT;
+    return AVERROR(EINVAL);
+}
+
 static int close_slave(TeeSlave *tee_slave)
 {
     AVFormatContext *avf;
@@ -184,7 +211,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
     AVDictionary *options = NULL;
     AVDictionaryEntry *entry;
     char *filename;
-    char *format = NULL, *select = NULL;
+    char *format = NULL, *select = NULL, *on_fail = NULL;
     AVFormatContext *avf2 = NULL;
     AVStream *st, *st2;
     int stream_count;
@@ -204,6 +231,14 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
 
     STEAL_OPTION("f", format);
     STEAL_OPTION("select", select);
+    STEAL_OPTION("onfail", on_fail);
+
+    ret = parse_slave_failure_policy_option(on_fail, tee_slave);
+    if (ret < 0) {
+        av_log(avf, AV_LOG_ERROR,
+               "Invalid onfail option value, valid options are 'abort' and 'ignore'\n");
+        goto end;
+    }
 
     ret = avformat_alloc_output_context2(&avf2, NULL, format, filename);
     if (ret < 0)
@@ -351,6 +386,7 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
 end:
     av_free(format);
     av_free(select);
+    av_free(on_fail);
     av_dict_free(&options);
     av_freep(&tmp_select);
     return ret;
@@ -380,6 +416,28 @@ static void log_slave(TeeSlave *slave, void *log_ctx, int log_level)
     }
 }
 
+static int tee_process_slave_failure(AVFormatContext *avf, unsigned slave_idx, int err_n)
+{
+    TeeContext *tee = avf->priv_data;
+    TeeSlave *tee_slave = &tee->slaves[slave_idx];
+
+    tee->nb_alive--;
+
+    close_slave(tee_slave);
+
+    if (!tee->nb_alive) {
+        av_log(avf, AV_LOG_ERROR, "All tee outputs failed.\n");
+        return err_n;
+    } else if (tee_slave->on_fail == ON_SLAVE_FAILURE_ABORT) {
+        av_log(avf, AV_LOG_ERROR, "Slave muxer #%u failed, aborting.\n", slave_idx);
+        return err_n;
+    } else {
+        av_log(avf, AV_LOG_ERROR, "Slave muxer #%u failed, continuing with %u/%u slaves.\n",
+               slave_idx, tee->nb_alive, tee->nb_slaves);
+        return 0;
+    }
+}
+
 static int tee_write_header(AVFormatContext *avf)
 {
     TeeContext *tee = avf->priv_data;
@@ -403,19 +461,24 @@ static int tee_write_header(AVFormatContext *avf)
             filename++;
     }
 
-    tee->nb_slaves = nb_slaves;
+    tee->nb_slaves = tee->nb_alive = nb_slaves;
 
     for (i = 0; i < nb_slaves; i++) {
-        if ((ret = open_slave(avf, slaves[i], &tee->slaves[i])) < 0)
-            goto fail;
-        log_slave(&tee->slaves[i], avf, AV_LOG_VERBOSE);
+        if ((ret = open_slave(avf, slaves[i], &tee->slaves[i])) < 0) {
+            ret = tee_process_slave_failure(avf, i, ret);
+            if (ret < 0)
+                goto fail;
+        } else {
+            log_slave(&tee->slaves[i], avf, AV_LOG_VERBOSE);
+        }
         av_freep(&slaves[i]);
     }
 
     for (i = 0; i < avf->nb_streams; i++) {
         int j, mapped = 0;
         for (j = 0; j < tee->nb_slaves; j++)
-            mapped += tee->slaves[j].stream_map[i] >= 0;
+            if (tee->slaves[j].avf)
+                mapped += tee->slaves[j].stream_map[i] >= 0;
         if (!mapped)
             av_log(avf, AV_LOG_WARNING, "Input stream #%d is not mapped "
                    "to any slave.\n", i);
@@ -423,8 +486,9 @@ static int tee_write_header(AVFormatContext *avf)
     return 0;
 
 fail:
-    for (i = 0; i < nb_slaves; i++)
+    for (i = 0; i < nb_slaves; i++) {
         av_freep(&slaves[i]);
+    }
     close_slaves(avf);
     return ret;
 }
@@ -432,14 +496,20 @@ fail:
 static int tee_write_trailer(AVFormatContext *avf)
 {
     TeeContext *tee = avf->priv_data;
+    AVFormatContext *avf2;
     int ret_all = 0, ret;
     unsigned i;
 
     for (i = 0; i < tee->nb_slaves; i++) {
-        if ((ret = close_slave(&tee->slaves[i])) < 0)
-            if (!ret_all)
+        if (!(avf2 = tee->slaves[i].avf))
+            continue;
+        if ((ret = close_slave(&tee->slaves[i])) < 0) {
+            ret = tee_process_slave_failure(avf2, i, ret);
+            if (!ret_all && ret < 0)
                 ret_all = ret;
+        }
     }
+    close_slaves(avf);
     return ret_all;
 }
 
@@ -454,7 +524,9 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt)
     AVRational tb, tb2;
 
     for (i = 0; i < tee->nb_slaves; i++) {
-        avf2 = tee->slaves[i].avf;
+        if (!(avf2 = tee->slaves[i].avf))
+            continue;
+
         s = pkt->stream_index;
         s2 = tee->slaves[i].stream_map[s];
         if (s2 < 0)
@@ -475,9 +547,11 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt)
 
         if ((ret = av_apply_bitstream_filters(avf2->streams[s2]->codec, &pkt2,
                                               tee->slaves[i].bsfs[s2])) < 0 ||
-            (ret = av_interleaved_write_frame(avf2, &pkt2)) < 0)
-            if (!ret_all)
+            (ret = av_interleaved_write_frame(avf2, &pkt2)) < 0) {
+            ret = tee_process_slave_failure(avf, i, ret);
+            if (!ret_all && ret < 0)
                 ret_all = ret;
+        }
     }
     return ret_all;
 }
-- 
1.9.1

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

Reply via email to