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

[Clément Bœsch]

On Tue, Sep 10, 2013 at 01:01:28AM +0200, Alexander Strasser wrote:
> 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

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

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

>  }
>  
>  #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.

Rest looks OK, thanks.

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130910/e8c809e9/attachment.asc>