Message ID | 20230202180642.3479-1-ffmpeg@gyani.pro |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel] avformat/tee: signal EOF if no more output is to be published. | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | success | Make fate finished |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | success | Make fate finished |
On Thu, 2 Feb 2023, Gyan Doshi wrote: > Prior to 2d924b3a630, ffmpeg.c would exit if any packet write failed. tee's > write_packet seemingly relied on that to enforce its abort failure policy. >> From 2d924b3a630, ffmpeg only closes that OutputStream and keeps on > sending packets of other streams. Hmm, are you sure? I glanced at the code and it seems to me that any failure of av_interleaved_write_frame() will cause the muxing thread to exit. So I don't quite see how other streams can receive packets. Thanks, Marton This breaks the abort behaviour > with the tee muxer when there are multiple streams, leaving the ffmpeg > process running beyond the intended point of abortion. > > Fixed by signaling EOF in tee's write_packet if an abort is required. > --- > libavformat/tee.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavformat/tee.c b/libavformat/tee.c > index dd408dd096..8362cdc972 100644 > --- a/libavformat/tee.c > +++ b/libavformat/tee.c > @@ -54,6 +54,7 @@ typedef struct TeeContext { > const AVClass *class; > unsigned nb_slaves; > unsigned nb_alive; > + int abort; > TeeSlave *slaves; > int use_fifo; > AVDictionary *fifo_options; > @@ -438,6 +439,7 @@ static int tee_process_slave_failure(AVFormatContext *avf, unsigned slave_idx, i > 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); > + tee->abort = 1; > return err_n; > } else { > av_log(avf, AV_LOG_ERROR, "Slave muxer #%u failed: %s, continuing with %u/%u slaves.\n", > @@ -599,7 +601,7 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt) > ret_all = ret; > } > } > - return ret_all; > + return (tee->abort || !tee->nb_alive) ? AVERROR_EOF : ret_all; > } > > const AVOutputFormat ff_tee_muxer = { > -- > 2.39.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 2023-02-03 04:17 am, Marton Balint wrote: > > > On Thu, 2 Feb 2023, Gyan Doshi wrote: > >> Prior to 2d924b3a630, ffmpeg.c would exit if any packet write failed. >> tee's >> write_packet seemingly relied on that to enforce its abort failure >> policy. >>> From 2d924b3a630, ffmpeg only closes that OutputStream and keeps on >> sending packets of other streams. > > Hmm, are you sure? I glanced at the code and it seems to me that any > failure of av_interleaved_write_frame() will cause the muxing thread > to exit. So I don't quite see how other streams can receive packets. Steps to reproduce: 1) ffmpeg -readrate 1 -stream_loop -1 -i INPUT -map 0:v -map 0:a -c copy -f tee "[f=flv:onfail=abort]rtmp://url/playpath|[f=null:onfail=ignore]stub" 2) Kill the rtmp endpoint at the server side. The tee muxer logs that it's aborting, however, ffmpeg keeps running. Contrast with 5.1 or earlier, which exit. av_iwf returns EOF which just results in a call to tq_receive_finish() for that ost without breaking muxer_thread's loop. Reards, Gyan
On Fri, 3 Feb 2023, Gyan Doshi wrote: > > > On 2023-02-03 04:17 am, Marton Balint wrote: >> >> >> On Thu, 2 Feb 2023, Gyan Doshi wrote: >> >>> Prior to 2d924b3a630, ffmpeg.c would exit if any packet write failed. >>> tee's >>> write_packet seemingly relied on that to enforce its abort failure >>> policy. >>>> From 2d924b3a630, ffmpeg only closes that OutputStream and keeps on >>> sending packets of other streams. >> >> Hmm, are you sure? I glanced at the code and it seems to me that any >> failure of av_interleaved_write_frame() will cause the muxing thread to >> exit. So I don't quite see how other streams can receive packets. > > Steps to reproduce: > > 1) ffmpeg -readrate 1 -stream_loop -1 -i INPUT -map 0:v -map 0:a -c copy -f > tee "[f=flv:onfail=abort]rtmp://url/playpath|[f=null:onfail=ignore]stub" > > 2) Kill the rtmp endpoint at the server side. The tee muxer logs that it's > aborting, however, ffmpeg keeps running. Contrast with 5.1 or earlier, which > exit. > > av_iwf returns EOF which just results in a call to tq_receive_finish() for > that ost without breaking muxer_thread's loop. Any av_interleaved_write_frame() negative return value is an error, ffmpeg should abort. It seems that there is a clash of error codes in sync_queue_process which returns AVERROR_EOF to signal sq_send EOF return, but that clashes with the AVERROR_EOF of av_interleaved_write_frame(), so the error condition is lost. Regards, Marton
On 2023-02-03 02:02 pm, Marton Balint wrote: > > > Any av_interleaved_write_frame() negative return value is an error, > ffmpeg should abort. It seems that there is a clash of error codes in > sync_queue_process which returns AVERROR_EOF to signal sq_send EOF > return, but that clashes with the AVERROR_EOF of > av_interleaved_write_frame(), so the error condition is lost. New patch sent. Note that the behaviour since 2d924b3a630 is to terminate only that muxer's thread, which I haven't changed. I think this is preferable to runtime abortion. Regards, Gyan
diff --git a/libavformat/tee.c b/libavformat/tee.c index dd408dd096..8362cdc972 100644 --- a/libavformat/tee.c +++ b/libavformat/tee.c @@ -54,6 +54,7 @@ typedef struct TeeContext { const AVClass *class; unsigned nb_slaves; unsigned nb_alive; + int abort; TeeSlave *slaves; int use_fifo; AVDictionary *fifo_options; @@ -438,6 +439,7 @@ static int tee_process_slave_failure(AVFormatContext *avf, unsigned slave_idx, i 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); + tee->abort = 1; return err_n; } else { av_log(avf, AV_LOG_ERROR, "Slave muxer #%u failed: %s, continuing with %u/%u slaves.\n", @@ -599,7 +601,7 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket *pkt) ret_all = ret; } } - return ret_all; + return (tee->abort || !tee->nb_alive) ? AVERROR_EOF : ret_all; } const AVOutputFormat ff_tee_muxer = {