Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Tomas Härdin" <git@haerdin.se>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: [FFmpeg-devel] [PATCH 3/3] lavf/subtitles: Unfix ticket #5032
Date: Fri, 29 Mar 2024 00:06:36 +0100
Message-ID: <6e0b44bc6313c994833c2c7a6622e9d8c8fc324f.camel@haerdin.se> (raw)
In-Reply-To: <fcb0c046a815cef418627a75c0e70373533b9b25.camel@haerdin.se>

[-- Attachment #1: Type: text/plain, Size: 776 bytes --]

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

[-- Attachment #2: 0003-lavf-subtitles-Unfix-ticket-5032.patch --]
[-- Type: text/x-patch, Size: 1953 bytes --]

From 784672af12da1d5fefeefad83cffa4b31f8b50fd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@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


[-- Attachment #3: 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".

  parent reply	other threads:[~2024-03-28 23:06 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-28 22:55 [FFmpeg-devel] [PATCH 1/3] lavf/subtitles: Do not eat \n\n Tomas Härdin
2024-03-28 22:56 ` [FFmpeg-devel] [PATCH 2/3] lavf/srtdec: Permit streaming input Tomas Härdin
2024-03-28 22:57   ` Tomas Härdin
2024-03-29 23:35     ` Michael Niedermayer
2024-03-30  0:03       ` Tomas Härdin
2024-03-30  8:31       ` Tomas Härdin
2024-03-30 11:36         ` Paul B Mahol
2024-03-30 11:44         ` Nicolas George
2024-03-30 14:44           ` Tomas Härdin
2024-03-30 14:49             ` Nicolas George
2024-03-30 15:23               ` Tomas Härdin
2024-03-30 15:34                 ` Nicolas George
2024-03-30 16:02                 ` Andreas Rheinhardt
2024-03-30 16:28                   ` Tomas Härdin
2024-04-01 13:15                   ` arch1t3cht
2024-04-01 14:34                   ` Tomas Härdin
2024-03-28 22:57 ` [FFmpeg-devel] [PATCH 1/3] lavf/subtitles: Do not eat \n\n Tomas Härdin
2024-03-28 23:06 ` Tomas Härdin [this message]
2024-03-29 12:29   ` [FFmpeg-devel] [PATCH 3/3] lavf/subtitles: Unfix ticket #5032 Tomas Härdin
2024-03-30  0:08 ` [FFmpeg-devel] [PATCH 1/2] lavf/subtitles: Add ff_text_peek_r16(), only accept \r, \n, \r\n and \r\r\n line endings Tomas Härdin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6e0b44bc6313c994833c2c7a6622e9d8c8fc324f.camel@haerdin.se \
    --to=git@haerdin.se \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git