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

[Marton Balint]

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.

Regards,
Marton