Obviously the most controversial patch in this set. I'd like to know
what program exactly that produced the broken file in ticket #5032 so
that we can send the guilty party to parsing jail

More seriously, eating any run of CR CR CR... is incredibly broken and
is part of the reason for the convoluted parsing logic in srtdec before
this patchset. A compromise solution could be to peek two bytes forward
and see if the line ending is CR CR LF. My guess is the file was
produced by running unix2dos on a file with CR LF, thus converting the
LF to CR LF.

webvttdec is better with this, probably because these kinds of line
ending is explicitly banned by the WebVTT spec. That is, CR CR LF is to
be treated as two line endings, no ifs or buts about it.

/Tomas
From 784672af12da1d5fefeefad83cffa4b31f8b50fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <g...@haerdin.se>
Date: Thu, 28 Mar 2024 23:30:06 +0100
Subject: [PATCH 3/3] lavf/subtitles: Unfix ticket #5032

This ticket should have been marked as wontfix so as to not encourage completely broken muxers.
---
 libavformat/subtitles.c  | 10 ++--------
 tests/fate/subtitles.mak |  3 ---
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
index bda549abd0..11fa89f5eb 100644
--- a/libavformat/subtitles.c
+++ b/libavformat/subtitles.c
@@ -459,13 +459,7 @@ ptrdiff_t ff_subtitles_read_line(FFTextReader *tr, char *buf, size_t size)
         buf[cur++] = c;
         buf[cur] = '\0';
     }
-    // don't eat \n\n
-    if (c == '\r') {
-        // sub/ticket5032-rrn.srt has \r\r\n
-        while (ff_text_peek_r8(tr) == '\r')
-            ff_text_r8(tr);
-        if (ff_text_peek_r8(tr) == '\n')
-            ff_text_r8(tr);
-    }
+    if (c == '\r' && ff_text_peek_r8(tr) == '\n')
+        ff_text_r8(tr);
     return cur;
 }
diff --git a/tests/fate/subtitles.mak b/tests/fate/subtitles.mak
index 90412e9ac1..940cd9a9ec 100644
--- a/tests/fate/subtitles.mak
+++ b/tests/fate/subtitles.mak
@@ -73,9 +73,6 @@ fate-sub-srt: CMD = fmtstdout ass -i $(TARGET_SAMPLES)/sub/SubRip_capability_tes
 FATE_SUBTITLES_ASS-$(call DEMDEC, SRT, SUBRIP) += fate-sub-srt-badsyntax
 fate-sub-srt-badsyntax: CMD = fmtstdout ass -i $(TARGET_SAMPLES)/sub/badsyntax.srt
 
-FATE_SUBTITLES-$(call ALLYES, SRT_DEMUXER SUBRIP_DECODER SRT_MUXER) += fate-sub-srt-rrn-remux
-fate-sub-srt-rrn-remux: CMD = fmtstdout srt -i $(TARGET_SAMPLES)/sub/ticket5032-rrn.srt -c:s copy
-
 FATE_SUBTITLES-$(call ALLYES, SRT_DEMUXER SUBRIP_DECODER SRT_MUXER) += fate-sub-srt-madness-timeshift
 fate-sub-srt-madness-timeshift: CMD = fmtstdout srt -itsoffset 3.14 -i $(TARGET_SAMPLES)/sub/madness.srt -c:s copy
 
-- 
2.39.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to