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

[Walter]

I understand. Would it be better to roll back the patch to just adding S_TEXT/WEBVTT to list of recognized codec ids for now?
-------- Original message --------From: Andreas Rheinhardt <andreas.rheinhardt at gmail.com> Date: 2021-02-07  3:08 AM  (GMT-05:00) To: ffmpeg-devel at ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH] Add matroska codec id S_TEXT/WEBVTT for WebVTT streams. 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 youmisunderstand the problem: There are two ways to mux WebVTT, one forWebM (using these "D_WEBVTT_*" values and one for Matroska. The WebM wayof doing WebVTT is the older one of the two and it is horrible, but itis the one that is supported. There was a patchset last year [1], [2] tofix this and implement the Matroska way, yet the author never sent afollow-up.Your patch would make the muxer write the packets in the WebM way, yetthe header would claim it to be the Matroska way. This is worse than itis now.- Andreas[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-June/264695.html[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-June/264693.html_______________________________________________ffmpeg-devel mailing listffmpeg-devel at ffmpeg.orghttps://ffmpeg.org/mailman/listinfo/ffmpeg-develTo unsubscribe, visit link above, or emailffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".