[FFmpeg-devel] [PATCH] avcodec/bsf: mention that av_bsf_send_packet() returning EAGAIN is not an error

[James Almer]

On 4/11/2020 5:33 AM, Anton Khirnov wrote:
> Quoting James Almer (2020-04-10 17:42:23)
>> On 4/10/2020 11:07 AM, Derek Buitenhuis wrote:
>>> On 10/04/2020 00:09, James Almer wrote:
>>>> EAGAIN is returned when input is provided but can't be consumed. The filtering
>>>> process is unaffected in this case, and the function will be able to consume
>>>> new input after retrieving filtered packets with av_bsf_receive_packet().
>>>>
>>>> Remove the line about empty packets never failing added in
>>>> 41b05b849f215b03eeb9e3608571ba47de64182a while at it. Even if it's currently
>>>> the case, it unnecessarily constrains the API and could be changed in the future
>>>> in case it needs to be extended.
>>>> The user should always check for errors and never expect a call to never fail.
>>>>
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>>  libavcodec/avcodec.h | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> Sounds good to me.
>>>
>>> - Derek
>>
>> Will push, thanks.
>>
>> That aside, I'm planning on restructuring these bsf functions
>> internally, making it behave more in line with the
>> send_packet/receive_frame API for decoding, and there are two options
>> for this: One, i keep the behavior described in the doxy after this
>> patch regarding return values from av_bsf_send_packet() (every AVERROR
>> code except EAGAIN being an error). Or two, make it return EOF instead
>> of 0 when extra unnecessary flush packets are passed to it, thus making
>> both bsf and decode APIs behave exactly the same.
>>
>> The latter would be ideal, but i don't know if it could be considered an
>> API change or not.
> 
> I don't really see how it makes things better. I can imagine valid uses
> for sending multiple flush packets - e.g. when the caller would
> otherwise have to keep an extra variable saying "flush packet has been
> sent". And we don't really gain anything from forbidding extra flush
> packets.

Not necessarily better, just in line with the decode API. It would be
nice to have all our decoupled input/output APIs behaving the same,
instead of each featuring one small difference here and there.

And ok, will ensure it keeps returning 0 on extra flush packets, then.

> 
>> It would have no effect on existing usage for example in decode.c and
>> the CLI. The latter in fact considers EOF from av_bsf_send_packet() a
>> possibility, so it may have been an oversight back when the bsf API
>> was first written.
> 
> I think the check for AVERROR_EOF there is intended for the value from
> receive_packet.
>