[FFmpeg-devel] [PATCH v2] avcodec/vaapi_encode_h264: add support for a/53 closed caption sei

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Aman Gupta
> Sent: Friday, August 2, 2019 07:33
> To: ffmpeg-devel at ffmpeg.org
> Cc: Aman Gupta <aman at tmm1.net>
> Subject: [FFmpeg-devel] [PATCH v2] avcodec/vaapi_encode_h264: add
> support for a/53 closed caption sei
> 
> From: Aman Gupta <aman at tmm1.net>
> 
> Signed-off-by: Aman Gupta <aman at tmm1.net>
> ---
>  libavcodec/vaapi_encode_h264.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vaapi_encode_h264.c
> b/libavcodec/vaapi_encode_h264.c
> index f4965d8b09..9860bacb91 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -39,6 +39,7 @@ enum {
>      SEI_TIMING         = 0x01,
>      SEI_IDENTIFIER     = 0x02,
>      SEI_RECOVERY_POINT = 0x04,
> +    SEI_A53_CC         = 0x08,
>  };
> 
>  // Random (version 4) ISO 11578 UUID.
> @@ -98,6 +99,8 @@ typedef struct VAAPIEncodeH264Context {
>      H264RawSEIRecoveryPoint        sei_recovery_point;
>      H264RawSEIUserDataUnregistered sei_identifier;
>      char                          *sei_identifier_string;
> +    H264RawSEIUserDataRegistered   sei_a53cc;
> +    void                          *sei_a53cc_data;
> 
>      int aud_needed;
>      int sei_needed;
> @@ -251,6 +254,11 @@ static int
> vaapi_encode_h264_write_extra_header(AVCodecContext *avctx,
>              sei->payload[i].payload.recovery_point = priv->sei_recovery_point;
>              ++i;
>          }
> +        if (priv->sei_needed & SEI_A53_CC) {
> +            sei->payload[i].payload_type =
> H264_SEI_TYPE_USER_DATA_REGISTERED;
> +            sei->payload[i].payload.user_data_registered = priv->sei_a53cc;
> +            ++i;
> +        }
> 
>          sei->payload_count = i;
>          av_assert0(sei->payload_count > 0);
> @@ -626,7 +634,8 @@ static int
> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>      VAAPIEncodePicture              *prev = pic->prev;
>      VAAPIEncodeH264Picture         *hprev = prev ? prev->priv_data : NULL;
>      VAEncPictureParameterBufferH264 *vpic = pic->codec_picture_params;
> -    int i;
> +    int i, err;
> +    size_t sei_a53cc_len;
> 
>      if (pic->type == PICTURE_TYPE_IDR) {
>          av_assert0(pic->display_order == pic->encode_order);
> @@ -700,6 +709,18 @@ static int
> vaapi_encode_h264_init_picture_params(AVCodecContext *avctx,
>          priv->sei_needed |= SEI_RECOVERY_POINT;
>      }
> 
> +    av_freep(&priv->sei_a53cc_data);

Is this necessary?

> +    err = ff_alloc_a53_sei(pic->input_image, 0, &priv->sei_a53cc_data,
> &sei_a53cc_len);

IMHO, an option "a53cc" is needed under "SEI", like identifier/timing/recovery_point does.
And check before setting a53_sei will be good.

> +    if (err < 0)
> +        return err;
> +    if (priv->sei_a53cc_data != NULL) {

"If (priv->sei_a53cc_data)" can be enough.

Regards,

- linjie