diff mbox series

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

Message ID 20200520125206.18834-1-anton@khirnov.net
State New
Headers show
Series [FFmpeg-devel] avcodec: improve avcodec_receive_frame() doxy
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Anton Khirnov May 20, 2020, 12:52 p.m. UTC
Make it clear that the returned reference is owned by the caller.
---
 libavcodec/avcodec.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

James Almer May 20, 2020, 6:17 p.m. UTC | #1
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
Andreas Rheinhardt May 20, 2020, 6:56 p.m. UTC | #2
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
James Almer May 20, 2020, 7:12 p.m. UTC | #3
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).
diff mbox series

Patch

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