[FFmpeg-devel] More backports for upcoming FFmpeg 2.0.2 release

Alexander Strasser eclipse7 at gmx.net
Tue Sep 10 01:01:28 CEST 2013


Hi all,

  I took the time and backported Clément's fixes
regarding text subtitles line reading/skipping.

  The backported patches are attached combined in an
mbox. The third one is the real diff to what is in
master currently. It is marked and should be squashed
into the previous commit before pushing to the release/2.0
branch.

  If the 4th patch should be backported I leave for
others to decide. If you are very strict probably not.

  Didn't yet look if these could/should be ported down
to release branch 1.2 yet.

  I checked that the function returns a length of one/two
byte less on zero-terminated data that has no EOL at the
end. I did not do extensive testing though.

Have fun,
  Alexander
-------------- next part --------------
From 3f31acff4b6ff247be0cd2ebedde86422c9d0a06 Mon Sep 17 00:00:00 2001
Message-Id: <3f31acff4b6ff247be0cd2ebedde86422c9d0a06.1378766323.git.eclipse7 at gmx.net>
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <u at pkh.me>
Date: Sun, 8 Sep 2013 16:17:46 +0200
Subject: [PATCH 1/4] avformat/srtdec: skip initial random line breaks.
To: ffdev

I found a bunch of (recent) SRT files in the wild with 3 to 10 line
breaks at the beginning.

Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
---
 libavformat/srtdec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/srtdec.c b/libavformat/srtdec.c
index dbf1866..ac783d9 100644
--- a/libavformat/srtdec.c
+++ b/libavformat/srtdec.c
@@ -37,6 +37,8 @@ static int srt_probe(AVProbeData *p)
     if (AV_RB24(ptr) == 0xEFBBBF)
         ptr += 3;  /* skip UTF-8 BOM */
 
+    while (*ptr == '\r' || *ptr == '\n')
+        ptr++;
     for (i=0; i<2; i++) {
         if ((num == i || num + 1 == i)
             && sscanf(ptr, "%*d:%*2d:%*2d%*1[,.]%*3d --> %*d:%*2d:%*2d%*1[,.]%3d", &v) == 1)
-- 

From 34e59cf0a2429a552b3c2d958b50eef450ee17fe Mon Sep 17 00:00:00 2001
Message-Id: <34e59cf0a2429a552b3c2d958b50eef450ee17fe.1378766323.git.eclipse7 at gmx.net>
In-Reply-To: <3f31acff4b6ff247be0cd2ebedde86422c9d0a06.1378766323.git.eclipse7 at gmx.net>
References: <3f31acff4b6ff247be0cd2ebedde86422c9d0a06.1378766323.git.eclipse7 at gmx.net>
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <u at pkh.me>
Date: Sun, 8 Sep 2013 18:02:45 +0200
Subject: [PATCH 2/4] avformat/subtitles: add a next line jumper and use it.
To: ffdev

This fixes a bunch of possible overread in avformat with the idiom p +=
strcspn(p, "\n") + 1 (strcspn() can focus on the trailing '\0' if no
'\n' is found, so the +1 leads to an overread).

Note on lavf/matroskaenc: no extra subtitles.o Makefile dependency is
added because only the header is required for ff_subtitles_next_line().

Note on lavf/mpsubdec: code gets slightly complex to avoid an infinite
loop in the probing since there is no more forced increment.

Conflicts:
	libavformat/mpl2dec.c

Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
---
 libavformat/jacosubdec.c  |  2 +-
 libavformat/matroskaenc.c |  3 ++-
 libavformat/microdvddec.c |  2 +-
 libavformat/mpl2dec.c     |  2 +-
 libavformat/mpsubdec.c    |  7 ++++++-
 libavformat/srtdec.c      |  8 +++-----
 libavformat/subtitles.h   | 10 ++++++++++
 7 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/libavformat/jacosubdec.c b/libavformat/jacosubdec.c
index da1afad..e2cbaad 100644
--- a/libavformat/jacosubdec.c
+++ b/libavformat/jacosubdec.c
@@ -63,7 +63,7 @@ static int jacosub_probe(AVProbeData *p)
                 return AVPROBE_SCORE_EXTENSION + 1;
             return 0;
         }
-        ptr += strcspn(ptr, "\n") + 1;
+        ptr += ff_subtitles_next_line(ptr);
     }
     return 0;
 }
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index fbb18a6..8954486 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -27,6 +27,7 @@
 #include "isom.h"
 #include "matroska.h"
 #include "riff.h"
+#include "subtitles.h"
 #include "wv.h"
 
 #include "libavutil/avstring.h"
@@ -1328,7 +1329,7 @@ static int srt_get_duration(uint8_t **buf)
             s_hsec += 1000*s_sec;       e_hsec += 1000*e_sec;
             duration = e_hsec - s_hsec;
         }
-        *buf += strcspn(*buf, "\n") + 1;
+        *buf += ff_subtitles_next_line(*buf);
     }
     return duration;
 }
diff --git a/libavformat/microdvddec.c b/libavformat/microdvddec.c
index 4b42846..5d9b13e 100644
--- a/libavformat/microdvddec.c
+++ b/libavformat/microdvddec.c
@@ -47,7 +47,7 @@ static int microdvd_probe(AVProbeData *p)
             sscanf(ptr, "{%*d}{%*d}%c",  &c) != 1 &&
             sscanf(ptr, "{DEFAULT}{}%c", &c) != 1)
             return 0;
-        ptr += strcspn(ptr, "\n") + 1;
+        ptr += ff_subtitles_next_line(ptr);
     }
     return AVPROBE_SCORE_MAX;
 }
diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
index b152cc8..17b302d 100644
--- a/libavformat/mpl2dec.c
+++ b/libavformat/mpl2dec.c
@@ -43,7 +43,7 @@ static int mpl2_probe(AVProbeData *p)
         if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) != 3 &&
             sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) != 2)
             return 0;
-        ptr += strcspn(ptr, "\r\n") + 1;
+        ptr += ff_subtitles_next_line(ptr);
         if (ptr >= ptr_end)
             return 0;
     }
diff --git a/libavformat/mpsubdec.c b/libavformat/mpsubdec.c
index 6a0cefb..c5bdcdb 100644
--- a/libavformat/mpsubdec.c
+++ b/libavformat/mpsubdec.c
@@ -37,11 +37,16 @@ static int mpsub_probe(AVProbeData *p)
     const char *ptr_end = p->buf + p->buf_size;
 
     while (ptr < ptr_end) {
+        int inc;
+
         if (!memcmp(ptr, "FORMAT=TIME", 11))
             return AVPROBE_SCORE_EXTENSION;
         if (!memcmp(ptr, "FORMAT=", 7))
             return AVPROBE_SCORE_EXTENSION / 3;
-        ptr += strcspn(ptr, "\n") + 1;
+        inc = ff_subtitles_next_line(ptr);
+        if (!inc)
+            break;
+        ptr += inc;
     }
     return 0;
 }
diff --git a/libavformat/srtdec.c b/libavformat/srtdec.c
index ac783d9..7f911bd 100644
--- a/libavformat/srtdec.c
+++ b/libavformat/srtdec.c
@@ -44,7 +44,7 @@ static int srt_probe(AVProbeData *p)
             && sscanf(ptr, "%*d:%*2d:%*2d%*1[,.]%*3d --> %*d:%*2d:%*2d%*1[,.]%3d", &v) == 1)
             return AVPROBE_SCORE_MAX;
         num = atoi(ptr);
-        ptr += strcspn(ptr, "\n") + 1;
+        ptr += ff_subtitles_next_line(ptr);
     }
     return 0;
 }
@@ -65,12 +65,10 @@ static int64_t get_pts(const char **buf, int *duration,
             int64_t start = (hh1*3600LL + mm1*60LL + ss1) * 1000LL + ms1;
             int64_t end   = (hh2*3600LL + mm2*60LL + ss2) * 1000LL + ms2;
             *duration = end - start;
-            *buf += strcspn(*buf, "\n");
-            *buf += !!**buf;
+            *buf += ff_subtitles_next_line(*buf);
             return start;
         }
-        *buf += strcspn(*buf, "\n");
-        *buf += !!**buf;
+        *buf += ff_subtitles_next_line(*buf);
     }
     return AV_NOPTS_VALUE;
 }
diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h
index 455b374..96de9fa 100644
--- a/libavformat/subtitles.h
+++ b/libavformat/subtitles.h
@@ -96,4 +96,14 @@ const char *ff_smil_get_attr_ptr(const char *s, const char *attr);
  */
 void ff_subtitles_read_chunk(AVIOContext *pb, AVBPrint *buf);
 
+/**
+ * Get the number of characters to increment to jump to the next line, or to
+ * the end of the string.
+ */
+static av_always_inline int ff_subtitles_next_line(const char *ptr)
+{
+    int n = strcspn(ptr, "\n");
+    return n + !!*ptr;
+}
+
 #endif /* AVFORMAT_SUBTITLES_H */
-- 

From 0b0aa64f8dda6cf838cb2f19abf7fcca83920d59 Mon Sep 17 00:00:00 2001
Message-Id: <0b0aa64f8dda6cf838cb2f19abf7fcca83920d59.1378766323.git.eclipse7 at gmx.net>
In-Reply-To: <3f31acff4b6ff247be0cd2ebedde86422c9d0a06.1378766323.git.eclipse7 at gmx.net>
References: <3f31acff4b6ff247be0cd2ebedde86422c9d0a06.1378766323.git.eclipse7 at gmx.net>
From: Alexander Strasser <eclipse7 at gmx.net>
Date: Tue, 10 Sep 2013 00:23:15 +0200
Subject: [PATCH 3/4] FIXUP: ff_subtitles_next_line: Fix length calculation
To: ffdev


Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
---
 libavformat/subtitles.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h
index 96de9fa..8f68e7b 100644
--- a/libavformat/subtitles.h
+++ b/libavformat/subtitles.h
@@ -103,7 +103,10 @@ void ff_subtitles_read_chunk(AVIOContext *pb, AVBPrint *buf);
 static av_always_inline int ff_subtitles_next_line(const char *ptr)
 {
     int n = strcspn(ptr, "\n");
-    return n + !!*ptr;
+    ptr += n;
+    if (*ptr == '\n')
+        n++;
+    return n;
 }
 
 #endif /* AVFORMAT_SUBTITLES_H */
-- 

From a8da583cd803fe94313cf201067a28bec7cd2cb7 Mon Sep 17 00:00:00 2001
Message-Id: <a8da583cd803fe94313cf201067a28bec7cd2cb7.1378766323.git.eclipse7 at gmx.net>
In-Reply-To: <3f31acff4b6ff247be0cd2ebedde86422c9d0a06.1378766323.git.eclipse7 at gmx.net>
References: <3f31acff4b6ff247be0cd2ebedde86422c9d0a06.1378766323.git.eclipse7 at gmx.net>
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <u at pkh.me>
Date: Sun, 8 Sep 2013 18:05:11 +0200
Subject: [PATCH 4/4] avformat/subtitles: support standalone CR (MacOS).
To: ffdev

Recent .srt files with CR only were found in the wild.

Conflicts:
	libavformat/subtitles.h

Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
---
 libavformat/subtitles.c | 5 +++--
 libavformat/subtitles.h | 8 +++++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
index 2af0450..9ef5770 100644
--- a/libavformat/subtitles.c
+++ b/libavformat/subtitles.c
@@ -191,7 +191,7 @@ static inline int is_eol(char c)
 
 void ff_subtitles_read_chunk(AVIOContext *pb, AVBPrint *buf)
 {
-    char eol_buf[5];
+    char eol_buf[5], last_was_cr = 0;
     int n = 0, i = 0, nb_eol = 0;
 
     av_bprint_clear(buf);
@@ -208,12 +208,13 @@ void ff_subtitles_read_chunk(AVIOContext *pb, AVBPrint *buf)
 
         /* line break buffering: we don't want to add the trailing \r\n */
         if (is_eol(c)) {
-            nb_eol += c == '\n';
+            nb_eol += c == '\n' || last_was_cr;
             if (nb_eol == 2)
                 break;
             eol_buf[i++] = c;
             if (i == sizeof(eol_buf) - 1)
                 break;
+            last_was_cr = c == '\r';
             continue;
         }
 
diff --git a/libavformat/subtitles.h b/libavformat/subtitles.h
index 8f68e7b..eced380 100644
--- a/libavformat/subtitles.h
+++ b/libavformat/subtitles.h
@@ -99,11 +99,17 @@ void ff_subtitles_read_chunk(AVIOContext *pb, AVBPrint *buf);
 /**
  * Get the number of characters to increment to jump to the next line, or to
  * the end of the string.
+ * The function handles the following line breaks schemes: LF (any sane
+ * system), CRLF (MS), or standalone CR (old MacOS).
  */
 static av_always_inline int ff_subtitles_next_line(const char *ptr)
 {
-    int n = strcspn(ptr, "\n");
+    int n = strcspn(ptr, "\r\n");
     ptr += n;
+    if (*ptr == '\r') {
+        ptr++;
+        n++;
+    }
     if (*ptr == '\n')
         n++;
     return n;
-- 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130910/519e43e8/attachment.asc>


More information about the ffmpeg-devel mailing list