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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170418/be95f508/attachment.sig>