[FFmpeg-devel] [PATCH 2/2] refactored semidec

Clément Bœsch u at pkh.me
Fri Apr 10 11:16:16 CEST 2015


> Subject: Re: [FFmpeg-devel] [PATCH 2/2] refactored semidec

"avcodec/samidec: make use of html subtitles parser"

On Thu, Apr 09, 2015 at 11:54:23PM -0700, Yayoi wrote:
> ---
>  libavcodec/Makefile        |  2 +-
>  libavcodec/htmlsubtitles.c |  6 ++++++
>  libavcodec/samidec.c       | 52 ++++++++++++++++------------------------------
>  3 files changed, 25 insertions(+), 35 deletions(-)
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 8384458..8e780ad 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -422,7 +422,7 @@ OBJS-$(CONFIG_RV20_DECODER)            += rv10.o
>  OBJS-$(CONFIG_RV20_ENCODER)            += rv20enc.o
>  OBJS-$(CONFIG_RV30_DECODER)            += rv30.o rv34.o rv30dsp.o rv34dsp.o
>  OBJS-$(CONFIG_RV40_DECODER)            += rv40.o rv34.o rv34dsp.o rv40dsp.o
> -OBJS-$(CONFIG_SAMI_DECODER)            += samidec.o ass.o
> +OBJS-$(CONFIG_SAMI_DECODER)            += samidec.o ass.o htmlsubtitles.o
>  OBJS-$(CONFIG_S302M_DECODER)           += s302m.o
>  OBJS-$(CONFIG_S302M_ENCODER)           += s302menc.o
>  OBJS-$(CONFIG_SANM_DECODER)            += sanm.o
> diff --git a/libavcodec/htmlsubtitles.c b/libavcodec/htmlsubtitles.c
> index 7eeec98..54a9707 100644
> --- a/libavcodec/htmlsubtitles.c
> +++ b/libavcodec/htmlsubtitles.c
> @@ -93,6 +93,11 @@ void ff_htmlmarkup_to_ass(AVCodecContext *avctx, AVBPrint *dst, const char *in)
>                  av_bprint_chars(dst, *in, 1);
>              break;
>          case '<':
> +            if (!av_strncasecmp(in, "<BR", 3)){
> +                    av_bprintf(dst, "\\N");
> +                    in += 4;
> +            }
> +

Wrong style.

Also, how does this behave with "<BR>", "<BR    >", "<BR    /   >", etc?

Are you sure you can increment by 4 here? Like, what if the subtitle ends
with "<BR", won't this overread?

>              tag_close = in[1] == '/';
>              len = 0;
>              if (sscanf(in+tag_close+1, "%127[^>]>%n", buffer, &len) >= 1 && len > 0) {
> @@ -175,5 +180,6 @@ void ff_htmlmarkup_to_ass(AVCodecContext *avctx, AVBPrint *dst, const char *in)
>      while (dst->len >= 2 && !strncmp(&dst->str[dst->len - 2], "\\N", 2))
>          dst->len -= 2;
>      dst->str[dst->len] = 0;
> +

Unrelated change

[...]

I don't have time to test until 2 weeks, but doesn't this change any FATE
test?

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150410/4efbd5c1/attachment.asc>


More information about the ffmpeg-devel mailing list