diff mbox series

[FFmpeg-devel,8/8] avcodec/avcodec: Document current behaviour for extradata

Message ID HE1PR0301MB2154EDEA8C3F1C2A0FD45DBE8F449@HE1PR0301MB2154.eurprd03.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel,1/7] avcodec/avcodec: Actually honour the documentation of subtitle_header
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 24, 2021, 12:29 p.m. UTC
Despite the documentation saying that it is not freed by libavcodec
for a decoder, avcodec_free_context() does so and has been doing so
since this function has been added more than seven years ago.

Honouring the current documentation in avcodec_free_context() would
add memleaks to all users of it that don't free their extradata
manually; given how long this behaviour has been around we can safely
assume that these are many (i.e. the fftools are among them, as is
libavformat as well as parts of libavcodec itself).

Therefore adapt the documentation to match actual behaviour.
This fixes ticket #5027.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/avcodec.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

James Almer April 24, 2021, 12:52 p.m. UTC | #1
On 4/24/2021 9:29 AM, Andreas Rheinhardt wrote:
> Despite the documentation saying that it is not freed by libavcodec
> for a decoder, avcodec_free_context() does so and has been doing so
> since this function has been added more than seven years ago.
> 
> Honouring the current documentation in avcodec_free_context() would
> add memleaks to all users of it that don't free their extradata
> manually; given how long this behaviour has been around we can safely
> assume that these are many (i.e. the fftools are among them, as is
> libavformat as well as parts of libavcodec itself).
> 
> Therefore adapt the documentation to match actual behaviour.
> This fixes ticket #5027.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavcodec/avcodec.h | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index b9b487be41..4596d12647 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -633,6 +633,8 @@ typedef struct AVCodecContext {
>        * Must be allocated with the av_malloc() family of functions.
>        * - encoding: Set/allocated/freed by libavcodec.
>        * - decoding: Set/allocated/freed by user.
> +     * Additionally, avcodec_free_context() frees it regardless of whether
> +     * the context is used for encoding or not.

I'd prefer to instead change the decoding line to

* - decoding: Set/allocated by user, freed by libavcodec.
diff mbox series

Patch

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index b9b487be41..4596d12647 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -633,6 +633,8 @@  typedef struct AVCodecContext {
      * Must be allocated with the av_malloc() family of functions.
      * - encoding: Set/allocated/freed by libavcodec.
      * - decoding: Set/allocated/freed by user.
+     * Additionally, avcodec_free_context() frees it regardless of whether
+     * the context is used for encoding or not.
      */
     uint8_t *extradata;
     int extradata_size;