[FFmpeg-devel] [PATCH 2/4] avformat/ttmlenc: enable writing out additional header values
Jan Ekström
jeebjp at gmail.com
Wed Mar 31 09:54:13 EEST 2021
On Tue, Mar 30, 2021 at 12:19 PM Andreas Rheinhardt
<andreas.rheinhardt at outlook.com> wrote:
>
> 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.
This function is only called if `ttml_ctx->input_type ==
PACKET_TYPE_PARAGRAPH`, which is only set if `extradata_size >=
TTMLENC_EXTRADATA_SIGNATURE_SIZE` (and if the , thus I thought that
checking it again would be unnecessary. Of course, if you think a
check is still needed, the following `!additional_data_size` check
`additional_data_size <= 0`.
>
> > + 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?
True, just got used to using the function with the max size defined.
> Anyway, these allocations and the ones below are unnecessary as you only
> need all of this when writing the header.
Yes, I could either utilize a struct, or pass a pointer to
values/struct. Just tried to be careful and not to re-utilize buffers
too easily from the extradata as-is.
>
> > +
> > + 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,
> > };
> >
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list