Re: [FFmpeg-devel] [PATCH 2/3] lavf/srtdec: Permit streaming input

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 3055 bytes --]

On Thu, Mar 28, 2024 at 11:57:57PM +0100, Tomas Härdin wrote:
> Here as well

>  libavformat/srtdec.c             |  211 ++++++++++++++++++++-------------------
>  tests/ref/fate/sub-srt-rrn-remux |    4 
>  2 files changed, 116 insertions(+), 99 deletions(-)
> 699d8b957286e190de6d5ca5cd17e67bb59dab7c  0002-lavf-srtdec-Permit-streaming-input.patch
> From 6d0684ca6fe02d80fc07a622fb85445a6917c29f Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
> Date: Thu, 28 Mar 2024 22:15:18 +0100
> Subject: [PATCH 2/3] lavf/srtdec: Permit streaming input
> 
> This is largely a rewrite.
> 
> Read packets in srt_read_packet() rather than reading the entire file in srt_read_header().
> Rely on AVFMT_GENERIC_INDEX for seeking.
> Allow zero-length packets (same as WebVTT).
> The implementation before this is broken in at least the following ways:
> 
> Decimals like .999999 are silently accepted and converted to 999.999 seconds.
> This is because no verification is done on the milliseconds part.
> This patch enforces that the minutes and seconds parts are 00-59, and the milliseconds 000-999.
> It's not perfect since FFmpeg doesn't have regex functionality or indeed any kind of parsing framework,
> but it's better than before.
> 
> Segmenting cues by lines that consist of just a single integer is incredibly wrong,
> since the subtitle text itself may well contain lines that are just a lone integer.
> This means files written with CR line endings that have text with lone integers are
> parsed in a completely broken manner. Neither can we segment by lines containing -->
> since this is permissible in SubRip (as far as I can tell). WebVTT explicitly forbids it however.
> ---
>  libavformat/srtdec.c             | 211 ++++++++++++++++---------------
>  tests/ref/fate/sub-srt-rrn-remux |   4 +
>  2 files changed, 116 insertions(+), 99 deletions(-)

breaks fate here:

--- ./tests/ref/fate/sub-srt-madness-timeshift	2024-03-29 20:43:34.617419731 +0100
+++ tests/data/fate/sub-srt-madness-timeshift	2024-03-30 00:30:08.776949369 +0100
@@ -3,34 +3,7 @@
 okay, let's make things easy

 2
-00:00:05,160 --> 00:00:05,263
-31 i'm a number but the only payload so please keep me :)
-
-3
 00:00:06,473 --> 00:00:07,584
 hello
 5
-don't forget me.
-
-4
-00:00:08,695 --> 00:00:09,806
-no.
-let's add some fun
-
-5
-00:00:10,917 --> 00:00:12,028
-let's do it in reverse bc wtf not
-45 yes this is a number but i'm actually part of the sub
-
-6
-00:00:12,028 --> 00:00:13,139
-1
-0
-next is negative, not a chapnum ;)
--1
-
-7
-00:00:13,241 --> 00:00:13,263
-credits
-2015

Test sub-srt-madness-timeshift failed. Look at tests/data/fate/sub-srt-madness-timeshift.err for details.
tests/Makefile:309: recipe for target 'fate-sub-srt-madness-timeshift' failed
make: *** [fate-sub-srt-madness-timeshift] Error 1


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

You can kill me, but you cannot change the truth.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".