[FFmpeg-devel] [PATCH 1/2] srtdec: Add timing-less "subrip" decoder.

[Clément Bœsch]

On Sat, Aug 11, 2012 at 08:41:25PM -0700, Philip Langdale wrote:
> After various discussions, we concluded that, amongst other things,
> it made sense to have a separate subrip decoder that did not use
> in-band timing information, and rather relied on the ffmpeg level
> timing.
> 
> As this is 90% the same as the existing srt decoder, it's implemented
> in the same file.
> 
> Signed-off-by: Philip Langdale <philipl at overt.org>
> ---
>  libavcodec/Makefile    |    1 +
>  libavcodec/allcodecs.c |    1 +
>  libavcodec/avcodec.h   |    1 +
>  libavcodec/srtdec.c    |   31 +++++++++++++++++++++++++++----
>  libavcodec/version.h   |    2 +-
>  5 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 911a781..e826885 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -423,6 +423,7 @@ OBJS-$(CONFIG_SONIC_LS_ENCODER)        += sonic.o
>  OBJS-$(CONFIG_SP5X_DECODER)            += sp5xdec.o mjpegdec.o mjpeg.o
>  OBJS-$(CONFIG_SRT_DECODER)             += srtdec.o ass.o
>  OBJS-$(CONFIG_SRT_ENCODER)             += srtenc.o ass_split.o
> +OBJS-$(CONFIG_SUBRIP_DECODER)          += srtdec.o ass.o
>  OBJS-$(CONFIG_SUBVIEWER_DECODER)       += subviewerdec.o ass.o
>  OBJS-$(CONFIG_SUNRAST_DECODER)         += sunrast.o
>  OBJS-$(CONFIG_SUNRAST_ENCODER)         += sunrastenc.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 6b42c30..f7ae449 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -415,6 +415,7 @@ void avcodec_register_all(void)
>      REGISTER_DECODER (REALTEXT, realtext);
>      REGISTER_DECODER (SAMI, sami);
>      REGISTER_ENCDEC  (SRT, srt);
> +    REGISTER_DECODER (SUBRIP, subrip);
>      REGISTER_DECODER (SUBVIEWER, subviewer);
>      REGISTER_ENCDEC  (XSUB, xsub);
>  
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 351e372..c91f4ef 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -441,6 +441,7 @@ enum AVCodecID {
>      AV_CODEC_ID_SAMI       = MKBETAG('S','A','M','I'),
>      AV_CODEC_ID_REALTEXT   = MKBETAG('R','T','X','T'),
>      AV_CODEC_ID_SUBVIEWER  = MKBETAG('S','u','b','V'),
> +    AV_CODEC_ID_SUBRIP,

Please choose MKBETAG('S','R','i','p') or something.

>  
>      /* other specific kind of codecs (generally used for attachments) */
>      AV_CODEC_ID_FIRST_UNKNOWN = 0x18000,           ///< A dummy ID pointing at the start of various fake codecs.
> diff --git a/libavcodec/srtdec.c b/libavcodec/srtdec.c
> index 13e4f9c..6a7812e 100644
> --- a/libavcodec/srtdec.c
> +++ b/libavcodec/srtdec.c
> @@ -217,9 +217,19 @@ static int srt_decode_frame(AVCodecContext *avctx,
>          return avpkt->size;
>  
>      while (ptr < end && *ptr) {
> -        ptr = read_ts(ptr, &ts_start, &ts_end, &x1, &y1, &x2, &y2);
> -        if (!ptr)
> -            break;
> +        if (avctx->codec->id == CODEC_ID_SRT) {
> +            ptr = read_ts(ptr, &ts_start, &ts_end, &x1, &y1, &x2, &y2);
> +            if (!ptr)
> +                break;
> +        } else {
> +            // Do final divide-by-10 outside rescale to force rounding down.
> +            ts_start = av_rescale_q(avpkt->pts,
> +                                    avctx->time_base,
> +                                    (AVRational){1,1000}) / 10;
> +            ts_end   = av_rescale_q(avpkt->pts + avpkt->duration,
> +                                    avctx->time_base,
> +                                    (AVRational){1,1000}) / 10;

Why not rescale to {1,100} and avoid /10?

[...]

Rest LGTM if tested, thank you.

-- 
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/20120812/78796183/attachment.asc>