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

[Mark Thompson]

On 14/09/17 23:35, Marton Balint wrote:
> 
> 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.

Ok, that's fair.  I've removed the annotation from the the send functions - these two and also av_bsf_send_packet().

Thanks,

- Mark