[FFmpeg-devel] [PATCH] lavc: Mark functions where ignoring returned error code is always wrong

[Marton Balint]

On Thu, 14 Sep 2017, Mark Thompson wrote:

> On 14/09/17 22:54, Marton Balint wrote:
>> 
>> On Thu, 14 Sep 2017, Mark Thompson wrote:
>> 
>>> ---
>>> There are more around, but this marks all of the obvious ones in avcodec.h.
>>>
>> 
>> [...]
>> 
>>> +av_warn_unused_result
>>> int avcodec_send_packet(AVCodecContext *avctx, const AVPacket *avpkt);
>> 
>>>  */
>>> +av_warn_unused_result
>>> int avcodec_send_frame(AVCodecContext *avctx, const AVFrame *frame);
>> 
>> I am not sure about these, these can only fail with ENOMEM if the user 
>> always gets all available frames/packets, but even if there is no 
>> memory, no state will be changed, the ownership of the packet/frame 
>> will remain with the caller, no null pointer will be returned, so if 
>> ignoring these is what the user wants, then so be it, no "undefined" 
>> behaviour will occur and we are not breaking the API contract by 
>> continuing like nothing happened.
>
> I was primarily thinking of the EAGAIN case, where the input data will 
> just have been silently discarded if the user ignores the return value. 
> I admit that isn't quite the same as the failed-allocation ones which 
> result in undefined behaviour, but I don't think ignoring the return 
> value of any of the send/receive functions is sane on the part of the 
> user.
>
> What do you think?  (I could also split the patch into ENOMEM and other 
> cases if that helps.)

If the send/receive API is used in a way that receive is called in a loop 
until EAGAIN is returned after each send, that ensures that no EAGAIN can 
occur in send.

ffplay does exactly that and the only reason it does not ignore the 
return value of send_packet is to be able to warn the user about API 
violations, which - in theory - should not happen.

So I'd rather not tag these functions as warn_unused_result, but if others 
disagree, I don't mind too much if the patch goes in as is.

Regards,
Marton