[FFmpeg-devel] [PATCH] Add matroska codec id S_TEXT/WEBVTT for WebVTT streams.

[Andreas Rheinhardt]

Walter Wong:
> Added the codec id S_TEXT/WEBVTT in order to bring ffmpeg generated files
> closer to the matroska spec. The list of matroska codec ids was also
> rearranged to push the old codec id (D_WEBVTT/SUBTITLES) to the bottom of
> the list so that S_TEXT/WEBVTT is the default codec id for new mkv files
> with webvtt subtitles.
> 
> Fixes ticket #5641: http://trac.ffmpeg.org/ticket/5641
> ---
>  libavformat/matroska.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/matroska.c b/libavformat/matroska.c
> index 7c56aba403..1128c96451 100644
> --- a/libavformat/matroska.c
> +++ b/libavformat/matroska.c
> @@ -60,16 +60,12 @@ const CodecTags ff_mkv_codec_tags[]={
>      {"A_VORBIS"         , AV_CODEC_ID_VORBIS},
>      {"A_WAVPACK4"       , AV_CODEC_ID_WAVPACK},
>  
> -    {"D_WEBVTT/SUBTITLES"   , AV_CODEC_ID_WEBVTT},
> -    {"D_WEBVTT/CAPTIONS"    , AV_CODEC_ID_WEBVTT},
> -    {"D_WEBVTT/DESCRIPTIONS", AV_CODEC_ID_WEBVTT},
> -    {"D_WEBVTT/METADATA"    , AV_CODEC_ID_WEBVTT},
> -
>      {"S_TEXT/UTF8"      , AV_CODEC_ID_SUBRIP},
>      {"S_TEXT/UTF8"      , AV_CODEC_ID_TEXT},
>      {"S_TEXT/ASCII"     , AV_CODEC_ID_TEXT},
>      {"S_TEXT/ASS"       , AV_CODEC_ID_ASS},
>      {"S_TEXT/SSA"       , AV_CODEC_ID_ASS},
> +    {"S_TEXT/WEBVTT"    , AV_CODEC_ID_WEBVTT},
>      {"S_ASS"            , AV_CODEC_ID_ASS},
>      {"S_SSA"            , AV_CODEC_ID_ASS},
>      {"S_VOBSUB"         , AV_CODEC_ID_DVD_SUBTITLE},
> @@ -100,6 +96,14 @@ const CodecTags ff_mkv_codec_tags[]={
>      {"V_VP8"            , AV_CODEC_ID_VP8},
>      {"V_VP9"            , AV_CODEC_ID_VP9},
>  
> +    /* These codec strings for webvtt seem to be depreciated from the Matroska
> +       spec, but it might be useful to keep them for compatibility for older
> +       files. These are at the bottom so they get matched after S_TEXT/WEBVTT. */
> +    {"D_WEBVTT/SUBTITLES"   , AV_CODEC_ID_WEBVTT},
> +    {"D_WEBVTT/CAPTIONS"    , AV_CODEC_ID_WEBVTT},
> +    {"D_WEBVTT/DESCRIPTIONS", AV_CODEC_ID_WEBVTT},
> +    {"D_WEBVTT/METADATA"    , AV_CODEC_ID_WEBVTT},
> +
>      {""                 , AV_CODEC_ID_NONE}
>  };
>  
> 
You are not fixing #5641 at all; in fact your comment suggests that you
misunderstand the problem: There are two ways to mux WebVTT, one for
WebM (using these "D_WEBVTT_*" values and one for Matroska. The WebM way
of doing WebVTT is the older one of the two and it is horrible, but it
is the one that is supported. There was a patchset last year [1], [2] to
fix this and implement the Matroska way, yet the author never sent a
follow-up.

Your patch would make the muxer write the packets in the WebM way, yet
the header would claim it to be the Matroska way. This is worse than it
is now.

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-June/264695.html
[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-June/264693.html