[FFmpeg-devel] [PATCH v18 02/19] avutil/frame: Prepare AVFrame for subtitle handling

[Anton Khirnov]

Quoting Soft Works (2021-11-29 20:47:56)
> @@ -481,6 +489,39 @@ typedef struct AVFrame {
>       */
>      uint64_t channel_layout;
>  
> +    /**
> +     * Display start time, relative to packet pts, in ms.
> +     */
> +    uint32_t subtitle_start_time;
> +
> +    /**
> +     * Display end time, relative to packet pts, in ms.
> +     */
> +    uint32_t subtitle_end_time;
> +
> +    /**
> +     * Number of items in the @ref subtitle_areas array.
> +     */
> +    unsigned num_subtitle_areas;
> +
> +    /**
> +     * Array of subtitle areas, may be empty.
> +     */
> +    AVSubtitleArea **subtitle_areas;
> +
> +    /**
> +     * Usually the same as packet pts, in AV_TIME_BASE.
> +     *
> +     * @deprecated This is kept for compatibility reasons and corresponds to
> +     * AVSubtitle->pts. Might be removed in the future.
> +     */
> +    int64_t subtitle_pts;
> +
> +    /**
> +     * Header containing style information for text subtitles.
> +     */
> +    AVBufferRef *subtitle_header;

Why does this not live in buf[]? That would eliminate the need for the
buf[0] hack and be consistent with audio and video.
You can add a symbolic name for it - e.g.
#define AV_FRAME_BUF_SUB_HEADER 0
if you dislike magic constants.

> diff --git a/libavutil/subfmt.h b/libavutil/subfmt.h
> index 5fc41d0ef2..bb87f61834 100644
> --- a/libavutil/subfmt.h
> +++ b/libavutil/subfmt.h
> @@ -21,6 +21,10 @@
>  #ifndef AVUTIL_SUBFMT_H
>  #define AVUTIL_SUBFMT_H
>  
> +#include <stdint.h>
> +
> +#include "buffer.h"
> +
>  enum AVSubtitleType {
>  
>      /**
> @@ -55,4 +59,48 @@ enum AVSubtitleType {
>      AV_SUBTITLE_FMT_NB,
>  };
>  
> +typedef struct AVSubtitleArea {
> +#define AV_NUM_BUFFER_POINTERS 1

Why 1? And why is it an array? Seems to me you could stuff buf into
AVFrame.buf/extended_buf and avoid inventing a new weird thing specially
for subtitles.

-- 
Anton Khirnov