diff mbox series

[FFmpeg-devel,v2] avformat/matroska: Write WebVTT subtitles according to MKV specs

Message ID 20230314032557.945590-1-gwymor@tilde.club
State New
Headers show
Series [FFmpeg-devel,v2] avformat/matroska: Write WebVTT subtitles according to MKV specs | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Gwyneth Morgan March 14, 2023, 3:25 a.m. UTC
When writing WebMs, FFmpeg muxes WebVTT subtitles with the D_WEBVTT/*
codec tags from the WebM specs [1]. However, it does the same when
muxing MKV files, and the Matroska specifications instead use
S_TEXT/WEBVTT tags for WebVTT subtitles [2], which FFmpeg currently
doesn't understand. Support reading MKVs using either tag, write regular
MKVs with S_TEXT/WEBVTT, and write WebMs with the D_WEBVTT/* tags we
already use.

[1]: https://www.webmproject.org/docs/container/
[2]: https://matroska.org/technical/codec_specs.html#s_textwebvtt

Signed-off-by: Gwyneth Morgan <gwymor@tilde.club>
Fixes: https://trac.ffmpeg.org/ticket/5641
---
v2: Rebase as later changes in ffmpeg.git conflicted with this patch.

 libavformat/matroska.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

Comments

Andreas Rheinhardt March 14, 2023, 3:37 a.m. UTC | #1
Gwyneth Morgan:
> When writing WebMs, FFmpeg muxes WebVTT subtitles with the D_WEBVTT/*
> codec tags from the WebM specs [1]. However, it does the same when
> muxing MKV files, and the Matroska specifications instead use
> S_TEXT/WEBVTT tags for WebVTT subtitles [2], which FFmpeg currently
> doesn't understand. Support reading MKVs using either tag, write regular
> MKVs with S_TEXT/WEBVTT, and write WebMs with the D_WEBVTT/* tags we
> already use.
> 
> [1]: https://www.webmproject.org/docs/container/
> [2]: https://matroska.org/technical/codec_specs.html#s_textwebvtt
> 
> Signed-off-by: Gwyneth Morgan <gwymor@tilde.club>
> Fixes: https://trac.ffmpeg.org/ticket/5641
> ---
> v2: Rebase as later changes in ffmpeg.git conflicted with this patch.
> 
>  libavformat/matroska.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/matroska.c b/libavformat/matroska.c
> index 79b2d09..d9e5ff9 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},
> @@ -78,6 +74,11 @@ const CodecTags ff_mkv_codec_tags[]={
>      {"S_HDMV/TEXTST"    , AV_CODEC_ID_HDMV_TEXT_SUBTITLE},
>      {"S_ARIBSUB"        , AV_CODEC_ID_ARIB_CAPTION},
>  
> +    {"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},
> +
>      {"V_AV1"            , AV_CODEC_ID_AV1},
>      {"V_AVS2"           , AV_CODEC_ID_AVS2},
>      {"V_AVS3"           , AV_CODEC_ID_AVS3},

The reason we write it the way we do is that webvtt is muxed differently
in Matroska than WebM. This needs to be fixed, too, before S_TEXT/WEBVTT
can be used for Matroska.

- Andreas
Gwyneth Morgan March 14, 2023, 4:28 p.m. UTC | #2
On 2023-03-14 04:37:17+0100, Andreas Rheinhardt wrote:
> The reason we write it the way we do is that webvtt is muxed differently
> in Matroska than WebM. This needs to be fixed, too, before S_TEXT/WEBVTT
> can be used for Matroska.

Ah, I see. I wasn't aware of the differences in muxing. Thanks for the
info.
diff mbox series

Patch

diff --git a/libavformat/matroska.c b/libavformat/matroska.c
index 79b2d09..d9e5ff9 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},
@@ -78,6 +74,11 @@  const CodecTags ff_mkv_codec_tags[]={
     {"S_HDMV/TEXTST"    , AV_CODEC_ID_HDMV_TEXT_SUBTITLE},
     {"S_ARIBSUB"        , AV_CODEC_ID_ARIB_CAPTION},
 
+    {"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},
+
     {"V_AV1"            , AV_CODEC_ID_AV1},
     {"V_AVS2"           , AV_CODEC_ID_AVS2},
     {"V_AVS3"           , AV_CODEC_ID_AVS3},