diff mbox series

[FFmpeg-devel] avformat/webvttdec: Fix WebVTT decoder truncating files at first STYLE block

Message ID 20210112164937.7219-1-roderich.schupp@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/webvttdec: Fix WebVTT decoder truncating files at first STYLE block | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Roderich Schupp Jan. 12, 2021, 4:49 p.m. UTC
Bug-ID: 9064

The webvtt decoder truncates the file at the first such block.
Since these blocks typically occur at the top of the webvtt file, this results
in an empty file (except for the WEBVTT header line).

Reason is that at STYLE block neither parses as a valid cue block nor
is it skipped like the WEBVTT (i.e. header) or NOTE blocks, hence
decoding stops.

Solution is to add STYLE to list of skipped blocks. And while we're at it, add REGION, too.
---
 libavformat/webvttdec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Dave Evans Jan. 12, 2021, 5:40 p.m. UTC | #1
Hijacking with related, similar patch from a few months ago:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=2538


On Tue, Jan 12, 2021 at 4:56 PM Roderich Schupp <roderich.schupp@gmail.com>
wrote:

> Bug-ID: 9064
>
> The webvtt decoder truncates the file at the first such block.
> Since these blocks typically occur at the top of the webvtt file, this
> results
> in an empty file (except for the WEBVTT header line).
>
> Reason is that at STYLE block neither parses as a valid cue block nor
> is it skipped like the WEBVTT (i.e. header) or NOTE blocks, hence
> decoding stops.
>
> Solution is to add STYLE to list of skipped blocks. And while we're at it,
> add REGION, too.
> ---
>  libavformat/webvttdec.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/webvttdec.c b/libavformat/webvttdec.c
> index 8d2fdfe..5a982dd 100644
> --- a/libavformat/webvttdec.c
> +++ b/libavformat/webvttdec.c
> @@ -89,10 +89,12 @@ static int webvtt_read_header(AVFormatContext *s)
>          p = identifier = cue.str;
>          pos = avio_tell(s->pb);
>
> -        /* ignore header chunk */
> +        /* ignore header, NOTE, STYLE and REGION chunks */
>          if (!strncmp(p, "\xEF\xBB\xBFWEBVTT", 9) ||
>              !strncmp(p, "WEBVTT", 6) ||
> -            !strncmp(p, "NOTE", 4))
> +            !strncmp(p, "NOTE", 4) ||
> +            !strncmp(p, "STYLE", 5) ||
> +            !strncmp(p, "REGION", 6))
>              continue;
>
>          /* optional cue identifier (can be a number like in SRT or some
> kind of
> --
> 2.30.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Moritz Barsnick Jan. 15, 2021, 4:16 p.m. UTC | #2
On Tue, Jan 12, 2021 at 17:40:06 +0000, Dave Evans wrote:
> Hijacking with related, similar patch from a few months ago:
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=2538

... which even includes a fate test, which is what I was about to
request from the newly submitted patch.

Cheers,
Moritz
diff mbox series

Patch

diff --git a/libavformat/webvttdec.c b/libavformat/webvttdec.c
index 8d2fdfe..5a982dd 100644
--- a/libavformat/webvttdec.c
+++ b/libavformat/webvttdec.c
@@ -89,10 +89,12 @@  static int webvtt_read_header(AVFormatContext *s)
         p = identifier = cue.str;
         pos = avio_tell(s->pb);
 
-        /* ignore header chunk */
+        /* ignore header, NOTE, STYLE and REGION chunks */
         if (!strncmp(p, "\xEF\xBB\xBFWEBVTT", 9) ||
             !strncmp(p, "WEBVTT", 6) ||
-            !strncmp(p, "NOTE", 4))
+            !strncmp(p, "NOTE", 4) ||
+            !strncmp(p, "STYLE", 5) ||
+            !strncmp(p, "REGION", 6))
             continue;
 
         /* optional cue identifier (can be a number like in SRT or some kind of