[FFmpeg-devel] [PATCH] ffmpeg; check return code of avcodec_send_frame when flushing encoders

Marton Balint cus at passwd.hu
Sun Apr 23 00:15:58 EEST 2017


On Wed, 19 Apr 2017, Marton Balint wrote:

>
>
> On Tue, 18 Apr 2017, Michael Niedermayer wrote:
>
>> On Tue, Apr 18, 2017 at 08:46:24PM +0200, Marton Balint wrote:
>>>
>>>
>>> On Tue, 18 Apr 2017, Michael Niedermayer wrote:
>>>
>>>> On Tue, Apr 18, 2017 at 07:09:30AM +0200, Nicolas George wrote:
>>>>> Le nonidi 29 germinal, an CCXXV, Michael Niedermayer a écrit :
>>>>>>> +                while ((ret = avcodec_receive_packet(enc, &pkt)) == 
> AVERROR(EAGAIN)) {
>>>>>>> +                    ret = avcodec_send_frame(enc, NULL);
>>>>>
>>>>> The doc says:
>>>>>
>>>>> # The functions will not return AVERROR(EAGAIN), unless you forgot to
>>>>> # enter draining mode.
>>>>
>>>> The full paragraph in the docs which you qoted from says this:
>>>> * - Call avcodec_receive_frame() (decoding) or avcodec_receive_packet()
>>>> *   (encoding) in a loop until AVERROR_EOF is returned. The functions 
> will
>>>> *   not return AVERROR(EAGAIN), unless you forgot to enter draining mode.
>>>>
>>>> the patch adds a check to avcodec_send_frame()
>>>>
>>>>
>>>>>
>>>>>> can the code be changed to not require this ?
>>>>>
>>>>> I would say the code does not require this as is.
>>>>
>>>> For decoding theres an explicit
>>>> "Sending the first flush packet will return success."
>>>
>>> As far as I see this sentence is only true if there was no decoding
>>> error in the legacy API during a flush. So this should probably be
>>> changed to something like "The first flush packet will not return
>>> AVERROR_EOF, if it returns success then any subsequent flush packets
>>> will return AVERROR_EOF." By the way we also guarantee this at
>>> libavcodec level, so it is harder to write a codec with the new API
>>> which violates this.
>>>
>>>> I cannot find similar for encoding, which is the case the patch changes
>>>> and what i think should be fixed if possible as it would be simpler,
>>>> making the patch unneeded.
>>>> Its quite possible iam missing something that makes it uneeded though
>>>
>>> The same is true for send_frame, based on the code involving the
>>> legacy API.
>>>
>>> We can of course decide to change the code involving the legacy API
>>> and enforce that flushing always succeed, but I'd rather keep it as
>>> is, even if that means a bit more error checking. It would be ugly
>>> API-wise that you sometimes have to check the return value of a
>>> function, sometimes you don't.
>>
>> iam happy with any solution or the patch as is
>>
>
> Unless there are no further comments I will apply this as is and backport 
> it to 3.3 as well.

Pushed.

Regards,
Marton


More information about the ffmpeg-devel mailing list