[FFmpeg-devel] [PATCH 4/5] lavf/matroskaenc: mux WebVTT using standard (non-WebM) formatting

[Andreas Rheinhardt]

rcombs:
> ---
>  libavformat/matroskaenc.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 63d6b50e6a..94e6cc542a 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2117,22 +2117,25 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, const AVPac
>      MatroskaMuxContext *mkv = s->priv_data;
>      mkv_track *track = &mkv->tracks[pkt->stream_index];
>      ebml_master blockgroup;
> -    int id_size, settings_size, size;
> -    const char *id, *settings;
> +    int id_size = 0, settings_size = 0, comment_size = 0, size = pkt->size;
> +    const char *id, *settings, *comment;
>      int64_t ts = track->write_dts ? pkt->dts : pkt->pts;
>      const int flags = 0;
>  
> -    id_size = 0;
>      id = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_IDENTIFIER,
>                                   &id_size);
>      id = id ? id : "";
>  
> -    settings_size = 0;
>      settings = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_SETTINGS,
>                                         &settings_size);
>      settings = settings ? settings : "";
>  
> -    size = id_size + 1 + settings_size + 1 + pkt->size;
> +    comment = av_packet_get_side_data(pkt, AV_PKT_DATA_WEBVTT_COMMENT,
> +                                      &comment_size);
> +    comment = comment ? comment : "";
> +
> +    if (mkv->mode == MODE_WEBM)
> +        size += id_size + 1 + settings_size + 1;
>  
>      /* The following string is identical to the one in mkv_write_block so that
>       * only one copy needs to exist in binaries. */
> @@ -2151,7 +2154,22 @@ static int mkv_write_vtt_blocks(AVFormatContext *s, AVIOContext *pb, const AVPac
>      put_ebml_num(pb, track->track_num, track->track_num_size);
>      avio_wb16(pb, ts - mkv->cluster_pts);
>      avio_w8(pb, flags);
> -    avio_printf(pb, "%.*s\n%.*s\n%.*s", id_size, id, settings_size, settings, pkt->size, pkt->data);
> +    if (mkv->mode == MODE_WEBM)
> +        avio_printf(pb, "%.*s\n%.*s\n", id_size, id, settings_size, settings);
> +    avio_write(pb, pkt->data, pkt->size);
> +
> +    if (mkv->mode != MODE_WEBM && (id_size || settings_size || comment_size)) {
> +        ebml_master block_additions = start_ebml_master(pb, MATROSKA_ID_BLOCKADDITIONS, 0);
> +        ebml_master block_more      = start_ebml_master(pb, MATROSKA_ID_BLOCKMORE,      0);
> +        /* Until dbc50f8a our demuxer used a wrong default value
> +         * of BlockAddID, so we write it unconditionally. */
> +        put_ebml_uint  (pb, MATROSKA_ID_BLOCKADDID, 1);
> +        put_ebml_id(pb, MATROSKA_ID_BLOCKADDITIONAL);
> +        put_ebml_length(pb, id_size + 1 + settings_size + 1 + comment_size, 0);
> +        avio_printf(pb, "%.*s\n%.*s\n%.*s", id_size, id, settings_size, settings, comment_size, comment);
> +        end_ebml_master(pb, block_more);
> +        end_ebml_master(pb, block_additions);
> +    }
>  
>      put_ebml_uint(pb, MATROSKA_ID_BLOCKDURATION, pkt->duration);
>      end_ebml_master(pb, blockgroup);

Did you test this? Did it work? It shouldn't. The initial guess for the
size of blockgroup is based solely on pkt->size (and TrackNumber); it
does not take the size of the side data into account and therefore may
underestimate the amount of bytes needed for the blockgroup length
field. And this leads to an assert.

- Andreas