[FFmpeg-devel] avcodec: improve avcodec_receive_frame() doxy

Make it clear that the returned reference is owned by the caller.
---
 libavcodec/avcodec.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

On 5/20/2020 9:52 AM, Anton Khirnov wrote:
> Make it clear that the returned reference is owned by the caller.
> ---
>  libavcodec/avcodec.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index c9baf859ac..cadd213301 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3728,10 +3728,10 @@ int avcodec_send_packet(AVCodecContext *avctx, const AVPacket *avpkt);
>   * Return decoded output data from a decoder.
>   *
>   * @param avctx codec context
> - * @param frame This will be set to a reference-counted video or audio
> - *              frame (depending on the decoder type) allocated by the
> - *              decoder. Note that the function will always call
> - *              av_frame_unref(frame) before doing anything else.
> + * @param frame This parameter should point to a "clean" (unallocated) frame,
> + *              into which the decoder will write a reference to the decoded
> + *              frame. The returned reference is owned by the caller and must be
> + *              freed with av_frame_unref().
>   *
>   * @return
>   *      0:                 success, a frame was returned
> 

LGTM

Anton Khirnov:
> Make it clear that the returned reference is owned by the caller.
> ---
>  libavcodec/avcodec.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index c9baf859ac..cadd213301 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3728,10 +3728,10 @@ int avcodec_send_packet(AVCodecContext *avctx, const AVPacket *avpkt);
>   * Return decoded output data from a decoder.
>   *
>   * @param avctx codec context
> - * @param frame This will be set to a reference-counted video or audio
> - *              frame (depending on the decoder type) allocated by the
> - *              decoder. Note that the function will always call
> - *              av_frame_unref(frame) before doing anything else.
> + * @param frame This parameter should point to a "clean" (unallocated) frame,
> + *              into which the decoder will write a reference to the decoded
> + *              frame. The returned reference is owned by the caller and must be
> + *              freed with av_frame_unref().
>   *
>   * @return
>   *      0:                 success, a frame was returned
> 
You are adding a requirement for the frame to be clean initially. This
doesn't seem to be intended, as the commit message doesn't mention it.

- Andreas

On 5/20/2020 3:56 PM, Andreas Rheinhardt wrote:
> Anton Khirnov:
>> Make it clear that the returned reference is owned by the caller.
>> ---
>>  libavcodec/avcodec.h | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index c9baf859ac..cadd213301 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -3728,10 +3728,10 @@ int avcodec_send_packet(AVCodecContext *avctx, const AVPacket *avpkt);
>>   * Return decoded output data from a decoder.
>>   *
>>   * @param avctx codec context
>> - * @param frame This will be set to a reference-counted video or audio
>> - *              frame (depending on the decoder type) allocated by the
>> - *              decoder. Note that the function will always call
>> - *              av_frame_unref(frame) before doing anything else.
>> + * @param frame This parameter should point to a "clean" (unallocated) frame,
>> + *              into which the decoder will write a reference to the decoded
>> + *              frame. The returned reference is owned by the caller and must be
>> + *              freed with av_frame_unref().
>>   *
>>   * @return
>>   *      0:                 success, a frame was returned
>>
> You are adding a requirement for the frame to be clean initially. This
> doesn't seem to be intended, as the commit message doesn't mention it.

Should is not must. It's not really adding a new requirement. But much
like it was before this patch i guess it could mention that the frame
will be cleaned if it wasn't already, regardless of return value (This
is important because plenty of functions don't alter input arguments on
failure and state as much).