[FFmpeg-devel] [PATCH v2 04/11] avformat/muxers: Add non-blocking mode support for av_write_trailer

[Jan Sebechlebsky]

On 08/26/2016 12:59 AM, Jan Sebechlebsky wrote:
> On 08/22/2016 04:49 PM, Michael Niedermayer wrote:
>
>> On Mon, Aug 22, 2016 at 04:27:16PM +0200, Jan Sebechlebsky wrote:
>>>
>>> On 08/22/2016 09:51 AM, Michael Niedermayer wrote:
>>>> On Thu, Aug 11, 2016 at 02:38:29PM +0200, sebechlebskyjan at gmail.com 
>>>> wrote:
>>>>> From: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
>>>>>
>>>>> This makes av_write_trailer not to free the resources if 
>>>>> write_trailer
>>>>> call returns AVERROR(EAGAIN) allowing repeated calls of 
>>>>> write_trailer of
>>>>> non-blocking muxer.
>>>>>
>>>>> Signed-off-by: Jan Sebechlebsky <sebechlebskyjan at gmail.com>
>>>>> ---
>>>>>   Changes since the last version of the patch:
>>>>>   - Added assert to the part of the code dealing with flushing
>>>>>     interleaved packets which should not be entered if
>>>>>     muxer in non-blocking mode is used.
>>>>>     (also there is assert for the same condition added into
>>>>>      av_interleaved_write_packet in one of the following
>>>>>      patches).
>>>>>
>>>>>   libavformat/avformat.h |  6 +++++-
>>>>>   libavformat/mux.c      | 10 ++++++++--
>>>>>   2 files changed, 13 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>>>> index d8a6cf3..2cc3156 100644
>>>>> --- a/libavformat/avformat.h
>>>>> +++ b/libavformat/avformat.h
>>>>> @@ -2510,8 +2510,12 @@ int 
>>>>> av_write_uncoded_frame_query(AVFormatContext *s, int stream_index);
>>>>>    *
>>>>>    * May only be called after a successful call to 
>>>>> avformat_write_header.
>>>>>    *
>>>>> + * If AVFMT_FLAG_NONBLOCK is set, this call may return 
>>>>> AVERROR(EAGAIN)
>>>>> + * meaning the operation is pending and the call should be repeated.
>>>>> + *
>>>>>    * @param s media file handle
>>>>> - * @return 0 if OK, AVERROR_xxx on error
>>>>> + * @return 0 if OK, AVERROR(EAGAIN) in case call should be repeated,
>>>>> + *         other AVERROR on error
>>>>>    */
>>>>>   int av_write_trailer(AVFormatContext *s);
>>>>> diff --git a/libavformat/mux.c b/libavformat/mux.c
>>>>> index e9973ed..3ae924c 100644
>>>>> --- a/libavformat/mux.c
>>>>> +++ b/libavformat/mux.c
>>>>> @@ -1204,11 +1204,14 @@ int av_write_trailer(AVFormatContext *s)
>>>>>       for (;; ) {
>>>>>           AVPacket pkt;
>>>>>           ret = interleave_packet(s, &pkt, NULL, 1);
>>>>> -        if (ret < 0)
>>>>> -            goto fail;
>>>>>           if (!ret)
>>>>>               break;
>>>>> +        av_assert0(!(s->flags & AVFMT_FLAG_NONBLOCK));
>>>> this would abort on any error not just EAGAIN
>>> I think it will abort in case interleave_packets does not return 0
>>> from the first call in loop, which means that interleaving was used
>>> (because there are some packets to be flushed) and that situation
>>> cannot happen with AVFMT_FLAG_NONBLOCK set when interleaving is
>>> forbidded. The next patch also adds assert to
>>> av_interleaved_write_packet. But I think the assert here is on the
>>> right place, or have I misunderstood the problem you're pointing
>>> out?
>> I thought interleave_packet can return AVERROR(ENOMEM)
>> maybe this is not possible, still it seems non-robust to assert if
>> it errors out
>>
> I think it cannot return AVERROR(ENOMEM) in case interleaving was not 
> used. But maybe I can remove assert from this function since there 
> will be assert in av_interleaved_write_frame?
>
> Regards,
> Jan
Should I remove the assert from the patch (since it will be in 
av_interleaved_write_frame)?

Regards,
Jan