[FFmpeg-devel] [PATCH 2/4] avformat/ttmlenc: enable writing out additional header values

Tue Mar 30 12:18:47 EEST 2021

Jan Ekström:
> From: Jan Ekström <jan.ekstrom at 24i.com>
> 
> This way the encoder may pass on the following values to the muxer:
> 1) Additional root "tt" element attributes, such as the subtitle
>    canvas reference size.
> 2) Anything before the body element of the document, such as regions
>    in the head element, which can configure styles.
> 
> Signed-off-by: Jan Ekström <jan.ekstrom at 24i.com>
> ---
>  libavcodec/ttmlenc.h  |  5 +++
>  libavformat/ttmlenc.c | 78 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 78 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/ttmlenc.h b/libavcodec/ttmlenc.h
> index c1dd5ec990..c3bb11478d 100644
> --- a/libavcodec/ttmlenc.h
> +++ b/libavcodec/ttmlenc.h
> @@ -25,4 +25,9 @@
>  #define TTMLENC_EXTRADATA_SIGNATURE "lavc-ttmlenc"
>  #define TTMLENC_EXTRADATA_SIGNATURE_SIZE (sizeof(TTMLENC_EXTRADATA_SIGNATURE) - 1)
>  
> +static const char ttml_default_namespacing[] =
> +"  xmlns=\"http://www.w3.org/ns/ttml\"\n"
> +"  xmlns:ttm=\"http://www.w3.org/ns/ttml#metadata\"\n"
> +"  xmlns:tts=\"http://www.w3.org/ns/ttml#styling\"\n";
> +
>  #endif /* AVCODEC_TTMLENC_H */
> diff --git a/libavformat/ttmlenc.c b/libavformat/ttmlenc.c
> index 940f8bbd4e..5d9ad6b756 100644
> --- a/libavformat/ttmlenc.c
> +++ b/libavformat/ttmlenc.c
> @@ -37,18 +37,23 @@ enum TTMLPacketType {
>      PACKET_TYPE_DOCUMENT,
>  };
>  
> +struct TTMLHeaderParameters {
> +    char *tt_element_params;
> +    char *pre_body_elements;
> +};
> +
>  typedef struct TTMLMuxContext {
>      enum TTMLPacketType input_type;
>      unsigned int document_written;
> +    struct TTMLHeaderParameters header_params;
>  } TTMLMuxContext;
>  
>  static const char ttml_header_text[] =
>  "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
>  "<tt\n"
> -"  xmlns=\"http://www.w3.org/ns/ttml\"\n"
> -"  xmlns:ttm=\"http://www.w3.org/ns/ttml#metadata\"\n"
> -"  xmlns:tts=\"http://www.w3.org/ns/ttml#styling\"\n"
> +"%s"
>  "  xml:lang=\"%s\">\n"
> +"%s"
>  "  <body>\n"
>  "    <div>\n";
>  
> @@ -72,6 +77,47 @@ static void ttml_write_time(AVIOContext *pb, const char tag[],
>                  tag, hour, min, sec, millisec);
>  }
>  
> +static int ttml_set_header_values_from_extradata(
> +    AVCodecParameters *par, struct TTMLHeaderParameters *header_params)
> +{
> +    size_t additional_data_size =
> +        par->extradata_size - TTMLENC_EXTRADATA_SIGNATURE_SIZE;
> +

Missing check that extradata_size is >= TTMLENC_EXTRADATA_SIGNATURE_SIZE.

> +    if (!additional_data_size) {
> +        header_params->tt_element_params =
> +            av_strndup(ttml_default_namespacing,
> +                       sizeof(ttml_default_namespacing) - 1);
> +        header_params->pre_body_elements = av_strndup("", 1);

Why are you using av_strndup and not the ordinary av_strdup here?
Anyway, these allocations and the ones below are unnecessary as you only
need all of this when writing the header.

> +
> +        if (!header_params->tt_element_params ||
> +            !header_params->pre_body_elements)
> +            return AVERROR(ENOMEM);
> +
> +        return 0;
> +    }
> +
> +    {
> +        char *value =
> +            (char *)par->extradata + TTMLENC_EXTRADATA_SIGNATURE_SIZE;
> +        size_t value_size = av_strnlen(value, additional_data_size);
> +
> +        if (!(header_params->tt_element_params = av_strndup(value, value_size)))
> +            return AVERROR(ENOMEM);
> +
> +        additional_data_size -= value_size + 1;
> +        value += value_size + 1;
> +        if (additional_data_size <= 0)
> +            return AVERROR_INVALIDDATA;
> +
> +        value_size = av_strnlen(value, additional_data_size);
> +
> +        if (!(header_params->pre_body_elements = av_strndup(value, value_size)))
> +            return AVERROR(ENOMEM);
> +
> +        return 0;
> +    }
> +}
> +
>  static int ttml_write_header(AVFormatContext *ctx)
>  {
>      TTMLMuxContext *ttml_ctx = ctx->priv_data;
> @@ -103,8 +149,21 @@ static int ttml_write_header(AVFormatContext *ctx)
>  
>          avpriv_set_pts_info(st, 64, 1, 1000);
>  
> -        if (ttml_ctx->input_type == PACKET_TYPE_PARAGRAPH)
> -            avio_printf(pb, ttml_header_text, printed_lang);
> +        if (ttml_ctx->input_type == PACKET_TYPE_PARAGRAPH) {
> +            int ret = ttml_set_header_values_from_extradata(
> +                st->codecpar, &ttml_ctx->header_params);
> +            if (ret < 0) {
> +                av_log(ctx, AV_LOG_ERROR,
> +                       "Failed to parse TTML header values from extradata: "
> +                       "%s!\n", av_err2str(ret));
> +                return ret;
> +            }
> +
> +            avio_printf(pb, ttml_header_text,
> +                        ttml_ctx->header_params.tt_element_params,
> +                        printed_lang,
> +                        ttml_ctx->header_params.pre_body_elements);
> +        }
>      }
>  
>      return 0;
> @@ -159,6 +218,14 @@ static int ttml_write_trailer(AVFormatContext *ctx)
>      return 0;
>  }
>  
> +static void ttml_free(AVFormatContext *ctx)
> +{
> +    TTMLMuxContext *ttml_ctx = ctx->priv_data;
> +
> +    av_freep(&(ttml_ctx->header_params.tt_element_params));
> +    av_freep(&(ttml_ctx->header_params.pre_body_elements));

Unnecessary parentheses (on top of these allocations being unnecessary).

> +}
> +
>  AVOutputFormat ff_ttml_muxer = {
>      .name              = "ttml",
>      .long_name         = NULL_IF_CONFIG_SMALL("TTML subtitle"),
> @@ -171,4 +238,5 @@ AVOutputFormat ff_ttml_muxer = {
>      .write_header      = ttml_write_header,
>      .write_packet      = ttml_write_packet,
>      .write_trailer     = ttml_write_trailer,
> +    .deinit            = ttml_free,
>  };
> 