diff mbox series

[FFmpeg-devel,2/7] avcodec/avcodec: Document current behaviour of subtitle_header

Message ID HE1PR0301MB2154ECDC5B74A6DDA46E2AED8F499@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 19, 2021, 2:06 a.m. UTC
Mention that avcodec_free_context() always frees it even when
encoding. And mention that freeing is of course performed in
avcodec_close() when decoding and not necessarily in avcodec_open2().

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

Comments

Andreas Rheinhardt April 24, 2021, 12:30 p.m. UTC | #1
Andreas Rheinhardt:
> Mention that avcodec_free_context() always frees it even when
> encoding. And mention that freeing is of course performed in
> avcodec_close() when decoding and not necessarily in avcodec_open2().
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/avcodec.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 8a71c04230..9b68aecd31 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2011,7 +2011,9 @@ typedef struct AVCodecContext {
>       * [Script Info] and [V4+ Styles] section, plus the [Events] line and
>       * the Format line following. It shouldn't include any Dialogue line.
>       * - encoding: Set/allocated/freed by user (before avcodec_open2())
> -     * - decoding: Set/allocated/freed by libavcodec (by avcodec_open2())
> +     * - decoding: Set/allocated/freed by libavcodec (by avcodec_open2()
> +     *             resp. avcodec_close())
> +     * Furthermore, avcodec_free_context() always frees it even when encoding.
>       */
>      uint8_t *subtitle_header;
>      int subtitle_header_size;
> 
Will apply patches 2-6 later today unless there are objections.

- Andreas
James Almer April 24, 2021, 3:09 p.m. UTC | #2
On 4/18/2021 11:06 PM, Andreas Rheinhardt wrote:
> Mention that avcodec_free_context() always frees it even when
> encoding. And mention that freeing is of course performed in
> avcodec_close() when decoding and not necessarily in avcodec_open2().
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavcodec/avcodec.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 8a71c04230..9b68aecd31 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2011,7 +2011,9 @@ typedef struct AVCodecContext {
>        * [Script Info] and [V4+ Styles] section, plus the [Events] line and
>        * the Format line following. It shouldn't include any Dialogue line.
>        * - encoding: Set/allocated/freed by user (before avcodec_open2())
> -     * - decoding: Set/allocated/freed by libavcodec (by avcodec_open2())
> +     * - decoding: Set/allocated/freed by libavcodec (by avcodec_open2()
> +     *             resp. avcodec_close())

What does resp. mean?

> +     * Furthermore, avcodec_free_context() always frees it even when encoding.

What this doxy is missing is the fact the buffer must be allocated with 
av_malloc(), considering we free it with av_free().

LGTM either way.

>        */
>       uint8_t *subtitle_header;
>       int subtitle_header_size;
>
Andreas Rheinhardt April 24, 2021, 3:18 p.m. UTC | #3
James Almer:
> On 4/18/2021 11:06 PM, Andreas Rheinhardt wrote:
>> Mention that avcodec_free_context() always frees it even when
>> encoding. And mention that freeing is of course performed in
>> avcodec_close() when decoding and not necessarily in avcodec_open2().
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>   libavcodec/avcodec.h | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 8a71c04230..9b68aecd31 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -2011,7 +2011,9 @@ typedef struct AVCodecContext {
>>        * [Script Info] and [V4+ Styles] section, plus the [Events]
>> line and
>>        * the Format line following. It shouldn't include any Dialogue
>> line.
>>        * - encoding: Set/allocated/freed by user (before avcodec_open2())
>> -     * - decoding: Set/allocated/freed by libavcodec (by
>> avcodec_open2())
>> +     * - decoding: Set/allocated/freed by libavcodec (by avcodec_open2()
>> +     *             resp. avcodec_close())
> 
> What does resp. mean?
> 

respectively

>> +     * Furthermore, avcodec_free_context() always frees it even when
>> encoding.
> 
> What this doxy is missing is the fact the buffer must be allocated with
> av_malloc(), considering we free it with av_free().
> 

It is actually currently allowed to use custom allocators when encoding
(which this patch further restricts to the case of encoding and not
using avcodec_free_context()).

> LGTM either way.
> 
>>        */
>>       uint8_t *subtitle_header;
>>       int subtitle_header_size;
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 8a71c04230..9b68aecd31 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2011,7 +2011,9 @@  typedef struct AVCodecContext {
      * [Script Info] and [V4+ Styles] section, plus the [Events] line and
      * the Format line following. It shouldn't include any Dialogue line.
      * - encoding: Set/allocated/freed by user (before avcodec_open2())
-     * - decoding: Set/allocated/freed by libavcodec (by avcodec_open2())
+     * - decoding: Set/allocated/freed by libavcodec (by avcodec_open2()
+     *             resp. avcodec_close())
+     * Furthermore, avcodec_free_context() always frees it even when encoding.
      */
     uint8_t *subtitle_header;
     int subtitle_header_size;