[FFmpeg-devel] More backports for upcoming FFmpeg 2.0.2 release
Alexander Strasser
eclipse7 at gmx.net
Tue Sep 10 23:04:15 CEST 2013
Hi Clément,
On 2013-09-10 07:47 +0200, Clément Bœsch wrote:
> On Tue, Sep 10, 2013 at 01:01:28AM +0200, Alexander Strasser wrote:
> > 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
>
> > 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)
> > --
> >
>
> Note that this one is not really a bug fix but more like allowing more
> badly made .srt.
I would still vote to take it in.
> > 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(-)
> >
> [...]
> > 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;
>
> You can just add the "ptr += n" chunk; after strcspn() you can only focus
> on \0 and \n.
This is comes IMHO much more naturally. It is easier to understand
and fits well with the following patch. To be more clear I do not intent
to remake this if it has no bug, hope that's is OK for you.
> > }
> >
> > #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;
> > --
> >
>
> This is adding a feature, I'm not sure it's relevant to backport.
> Actually, the code path for \r as return line not being really complete
> yet nor tested, I'd better not.
Fine with me. Better to leave this one out.
> Rest looks OK, thanks.
Thank you.
Alexander
-------------- 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/93e26b35/attachment.asc>
More information about the ffmpeg-devel
mailing list