[FFmpeg-devel] avformat/tee: signal EOF if no more output is to be published.

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. 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(-)

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