[FFmpeg-devel] [PATCH v4 3/3] avformat/tee: Handling slave failure in tee muxer
Marton Balint
cus at passwd.hu
Wed Apr 6 23:37:17 CEST 2016
On Mon, 4 Apr 2016, sebechlebskyjan at gmail.com wrote:
> From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
>
> 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 <sebechlebskyjan at gmail.com>
> ---
> I've just added topic to commit message title as Marton Balint suggested.
>
> doc/muxers.texi | 14 ++++++++
> libavformat/tee.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 100 insertions(+), 10 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index 2aafbad..2d63083 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -1372,6 +1372,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}
> +
> + at 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
> @@ -1386,6 +1392,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):
> + at 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/"
> + at end example
> +
> + at 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 d823805..50aaf9f 100644
> --- a/libavformat/tee.c
> +++ b/libavformat/tee.c
> @@ -29,10 +29,20 @@
>
> #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;
> + unsigned char is_alive;
> +
> /** map from input to output streams indexes,
> * disabled output streams are set to -1 */
> int *stream_map;
> @@ -41,6 +51,7 @@ typedef struct {
> typedef struct TeeContext {
> const AVClass *class;
> unsigned nb_slaves;
> + unsigned nb_alive;
> TeeSlave slaves[MAX_SLAVES];
> } TeeContext;
>
> @@ -135,6 +146,18 @@ end:
> return ret;
> }
>
> +static inline int parse_slave_failure_policy_option(const char *opt)
> +{
> + if (!opt) {
> + return DEFAULT_SLAVE_FAILURE_POLICY;
> + } else if (!av_strcasecmp("abort", opt)) {
> + return ON_SLAVE_FAILURE_ABORT;
> + } else if (!av_strcasecmp("ignore", opt)) {
> + return ON_SLAVE_FAILURE_IGNORE;
> + }
> + return 0;
Probably better, if you return AVERROR(EINVAL) in case of an invalid
option, and check for a negative return value to detect an error,
that is the way most functions work in ffmpeg.
> +}
> +
> static void close_slave(TeeSlave *tee_slave)
> {
> AVFormatContext *avf;
> @@ -165,7 +188,8 @@ static void close_slaves(AVFormatContext *avf)
> unsigned i;
>
> for (i = 0; i < tee->nb_slaves; i++) {
> - close_slave(&tee->slaves[i]);
> + if (tee->slaves[i].is_alive)
> + close_slave(&tee->slaves[i]);
> }
> }
>
> @@ -175,7 +199,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;
> @@ -195,6 +219,17 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
>
> STEAL_OPTION("f", format);
> STEAL_OPTION("select", select);
> + STEAL_OPTION("onfail", on_fail);
> +
> + tee_slave->on_fail = parse_slave_failure_policy_option(on_fail);
> + if (!tee_slave->on_fail) {
> + av_log(avf, AV_LOG_ERROR,
> + "Invalid onfail option value, valid options are 'abort' and 'ignore'\n");
> + ret = AVERROR(EINVAL);
> + /* Set failure behaviour to abort, so invalid option error will not be ignored */
> + tee_slave->on_fail = ON_SLAVE_FAILURE_ABORT;
> + goto end;
> + }
>
> ret = avformat_alloc_output_context2(&avf2, NULL, format, filename);
> if (ret < 0)
> @@ -339,8 +374,11 @@ static int open_slave(AVFormatContext *avf, char *slave, TeeSlave *tee_slave)
> }
>
> end:
> + if (ret < 0)
> + close_slave(tee_slave);
I may miss something, but is this necessary here? In case of an error, you
will call tee_process_slave_failure explictly requiring not to close the
slave... You should consider always closing the slave there, it would make
the code easier to follow IMHO.
> av_free(format);
> av_free(select);
> + av_free(on_fail);
> av_dict_free(&options);
> av_freep(&tmp_select);
> return ret;
> @@ -370,6 +408,31 @@ 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, unsigned char needs_closing)
> +{
> + TeeContext *tee = avf->priv_data;
> + TeeSlave *tee_slave = &tee->slaves[slave_idx];
> +
> + tee_slave->is_alive = 0;
> + tee->nb_alive--;
> +
> + if (needs_closing)
> + close_slave(tee_slave);
If you always close the slave here, the needs_closing parameter will
become unnecessary.
> +
> + 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);
breathing space missing in text.
> + 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;
> @@ -393,19 +456,25 @@ static int tee_write_header(AVFormatContext *avf)
> filename++;
> }
>
> + tee->nb_slaves = tee->nb_alive = nb_slaves;
> +
Here you already changed the order of the tee->nb_slaves assignment I
mentioned in my comments of the second patch in the series, on the other
hand you do not check for possible null tee_slave->avf in close_slave(s).
> 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, 0);
awkward space after i variable.
> + if (ret < 0)
> + goto fail;
> + } else {
> + log_slave(&tee->slaves[i], avf, AV_LOG_VERBOSE);
> + tee->slaves[i].is_alive = 1;
> + }
> av_freep(&slaves[i]);
> }
>
> - tee->nb_slaves = nb_slaves;
> -
> 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].is_alive)
> + 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);
> @@ -427,6 +496,8 @@ static int tee_write_trailer(AVFormatContext *avf)
> unsigned i;
>
> for (i = 0; i < tee->nb_slaves; i++) {
> + if (!tee->slaves[i].is_alive)
> + continue;
> avf2 = tee->slaves[i].avf;
> if ((ret = av_write_trailer(avf2)) < 0)
> if (!ret_all)
> @@ -449,6 +520,9 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt)
> AVRational tb, tb2;
>
> for (i = 0; i < tee->nb_slaves; i++) {
> + if (!tee->slaves[i].is_alive)
> + continue;
> +
> avf2 = tee->slaves[i].avf;
> s = pkt->stream_index;
> s2 = tee->slaves[i].stream_map[s];
> @@ -470,9 +544,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, 1);
> + if (!ret_all && ret < 0)
> ret_all = ret;
> + }
> }
> return ret_all;
> }
> --
> 1.9.1
Regards,
Marton
More information about the ffmpeg-devel
mailing list