diff mbox series

[FFmpeg-devel,v2] avformat/webvttdec, enc: correctly process files containing STYLE, REGION blocks

Message ID CA+_k4RJ1CNriPbhxxzLf1O6V1VuP2OXch3E8LqOsMNSRUyEnTw@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel,v2] avformat/webvttdec, enc: correctly process files containing STYLE, REGION blocks
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate fail Make fate failed
andriy/PPC64_make warning Make failed

Commit Message

Dave Evans Oct. 13, 2020, 9:25 a.m. UTC
This patch fixes the total failure to parse cues when style and region
definition blocks are contained in the input file, and ensures those
blocks are written to the output when copying.

The sample attached needs to be added to samples at the path shown in
the patch in order to validate that the original issue is fixed.

Same as v1 except the test has been changed as requested in the original review.

Cheers,
Dave
Subject: [PATCH] avformat/webvttdec,enc: correctly decode files containing
 STYLE, REGION blocks

Add the ability to extract cues from files containing one or more WebVTT
region definition block or WebVTT style block. Previously the decoder would
misinterpret these and fail to parse any cues at all. Also add the ability to
write these, or alternative header information, back out in the encoder using
the codec extradata.
---
 libavformat/webvttdec.c                   | 16 ++++++-
 libavformat/webvttenc.c                   | 12 ++++++
 tests/fate/subtitles.mak                  |  3 ++
 tests/ref/fate/sub-webvtt-styleandregions | 51 +++++++++++++++++++++++
 4 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 tests/ref/fate/sub-webvtt-styleandregions
diff mbox series

Patch

diff --git a/libavformat/webvttdec.c b/libavformat/webvttdec.c
index 8d2fdfed37..c6cc367383 100644
--- a/libavformat/webvttdec.c
+++ b/libavformat/webvttdec.c
@@ -60,7 +60,7 @@  static int64_t read_ts(const char *s)
 static int webvtt_read_header(AVFormatContext *s)
 {
     WebVTTContext *webvtt = s->priv_data;
-    AVBPrint cue;
+    AVBPrint cue, header;
     int res = 0;
     AVStream *st = avformat_new_stream(s, NULL);
 
@@ -72,6 +72,7 @@  static int webvtt_read_header(AVFormatContext *s)
     st->disposition |= webvtt->kind;
 
     av_bprint_init(&cue,    0, AV_BPRINT_SIZE_UNLIMITED);
+    av_bprint_init(&header, 0, AV_BPRINT_SIZE_UNLIMITED);
 
     for (;;) {
         int i;
@@ -89,12 +90,18 @@  static int webvtt_read_header(AVFormatContext *s)
         p = identifier = cue.str;
         pos = avio_tell(s->pb);
 
-        /* ignore header chunk */
+        /* ignore the magic word and any comments */
         if (!strncmp(p, "\xEF\xBB\xBFWEBVTT", 9) ||
             !strncmp(p, "WEBVTT", 6) ||
             !strncmp(p, "NOTE", 4))
             continue;
 
+        /* store the style and region blocks from the header */
+        if (!strncmp(p, "STYLE", 5) || !strncmp(p, "REGION", 6)) {
+            av_bprintf(&header, "%s%s", header.len ? "\n\n" : "", p);
+            continue;
+        }
+
         /* optional cue identifier (can be a number like in SRT or some kind of
          * chaptering id) */
         for (i = 0; p[i] && p[i] != '\n' && p[i] != '\r'; i++) {
@@ -161,12 +168,17 @@  static int webvtt_read_header(AVFormatContext *s)
         SET_SIDE_DATA(settings,   AV_PKT_DATA_WEBVTT_SETTINGS);
     }
 
+    res = ff_bprint_to_codecpar_extradata(st->codecpar, &header);
+    if (res < 0)
+        goto end;
+
     ff_subtitles_queue_finalize(s, &webvtt->q);
 
 end:
     if (res < 0)
         ff_subtitles_queue_clean(&webvtt->q);
     av_bprint_finalize(&cue,    NULL);
+    av_bprint_finalize(&header, NULL);
     return res;
 }
 
diff --git a/libavformat/webvttenc.c b/libavformat/webvttenc.c
index cbd989dcb6..fcbd3ee10a 100644
--- a/libavformat/webvttenc.c
+++ b/libavformat/webvttenc.c
@@ -58,6 +58,18 @@  static int webvtt_write_header(AVFormatContext *ctx)
 
     avio_printf(pb, "WEBVTT\n");
 
+    if (par->extradata_size > 0) {
+        size_t header_size = par->extradata_size;
+
+        if (par->extradata[0] != '\n')
+            avio_printf(pb, "\n");
+
+        avio_write(pb, par->extradata, header_size);
+
+        if (par->extradata[header_size - 1] != '\n')
+            avio_printf(pb, "\n");
+    }
+
     return 0;
 }
 
diff --git a/tests/fate/subtitles.mak b/tests/fate/subtitles.mak
index 6323d0f93d..375f81ef93 100644
--- a/tests/fate/subtitles.mak
+++ b/tests/fate/subtitles.mak
@@ -91,6 +91,9 @@  fate-sub-webvtt: CMD = fmtstdout ass -i $(TARGET_SAMPLES)/sub/WebVTT_capability_
 FATE_SUBTITLES_ASS-$(call DEMDEC, WEBVTT, WEBVTT) += fate-sub-webvtt2
 fate-sub-webvtt2: CMD = fmtstdout ass -i $(TARGET_SAMPLES)/sub/WebVTT_extended_tester.vtt
 
+FATE_SUBTITLES-$(call ALLYES, WEBVTT_DEMUXER, WEBVTT_MUXER) += fate-sub-webvtt-styleandregions
+fate-sub-webvtt-styleandregions: CMD = fmtstdout webvtt -i $(TARGET_SAMPLES)/sub/webvtt_style_and_regions.vtt -c:s copy
+
 FATE_SUBTITLES-$(call ALLYES, SRT_DEMUXER SUBRIP_DECODER WEBVTT_ENCODER WEBVTT_MUXER) += fate-sub-webvttenc
 fate-sub-webvttenc: CMD = fmtstdout webvtt -i $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt
 
diff --git a/tests/ref/fate/sub-webvtt-styleandregions b/tests/ref/fate/sub-webvtt-styleandregions
new file mode 100644
index 0000000000..8c8776adab
--- /dev/null
+++ b/tests/ref/fate/sub-webvtt-styleandregions
@@ -0,0 +1,51 @@ 
+WEBVTT
+
+REGION
+id:son
+width:40%
+lines:3
+regionanchor:20%,80%
+viewportanchor:20%,80%
+scroll:up
+
+REGION
+id:father
+width:40%
+lines:3
+regionanchor:80%,80%
+viewportanchor:80%,80%
+scroll:up
+
+STYLE
+::cue(i) {
+  /* make i tags italic */
+  font-style: italic
+}
+
+STYLE
+::cue(v[voice="Son"]) {
+  color: magenta
+}
+
+STYLE
+::cue(v[voice="Father"]) {
+  color: yellow
+}
+
+00:10.000 --> 00:25.000 region:son align:left
+<v Son>Can I tell you a joke, Dad?
+
+00:12.500 --> 00:27.500 region:father align:right
+<v Father>Sure, I could do with a laugh.
+
+00:15.000 --> 00:30.000 region:son align:left
+<v Son>Where do sheep go to get their hair cut?
+
+00:17.500 --> 00:32.500 region:father align:right
+<v Father>I don't know, son. Where do sheep go to get their hair cut?
+
+00:20.000 --> 00:35.000 region:son align:left
+<v Son>To the baa-baa shop!
+
+00:22.500 --> 00:37.500 region:father align:right
+<v Father><i>[facepalms]</i>