[FFmpeg-devel] [RFC] lavfi/subtitles offset option

[Clément Bœsch]

On Fri, Jan 04, 2013 at 12:57:51PM +0100, Stefano Sabatini wrote:
> Hi,
> 
> this patch should allow to specify an offset for fixing
> synchronization subtitles problems. Unfortunately libass doesn't seem
> to like the offset, so it won't recognize events if an offset is added
> to time_ms.

But that's what your patch does right? What doesn't work, I don't get it?

> -- 
> FFmpeg = Fanciful and Fundamentalist Miracolous Patchable Ecumenical Game

> From 50f93048d6ca7a62b8ddc3f1d6688007320ab002 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Fri, 4 Jan 2013 10:22:37 +0100
> Subject: [PATCH] lavfi/ass: add offset option
> 
> ---
>  libavfilter/vf_ass.c |   18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_ass.c b/libavfilter/vf_ass.c
> index ade1b37..ebd2dc3 100644
> --- a/libavfilter/vf_ass.c
> +++ b/libavfilter/vf_ass.c
> @@ -50,6 +50,8 @@ typedef struct {
>      ASS_Renderer *renderer;
>      ASS_Track    *track;
>      char *filename;
> +    char *offset_str;
> +    int64_t offset;            ///< subtitles offset, expressed in milliseconds
>      uint8_t rgba_map[4];
>      int     pix_step[4];       ///< steps per pixel for each plane of the main output
>      int original_w, original_h;
> @@ -63,6 +65,7 @@ static const AVOption options[] = {
>      {"filename",       "set the filename of file to read",                         OFFSET(filename),   AV_OPT_TYPE_STRING,     {.str = NULL},  CHAR_MIN, CHAR_MAX, FLAGS },
>      {"f",              "set the filename of file to read",                         OFFSET(filename),   AV_OPT_TYPE_STRING,     {.str = NULL},  CHAR_MIN, CHAR_MAX, FLAGS },
>      {"original_size",  "set the size of the original video (used to scale fonts)", OFFSET(original_w), AV_OPT_TYPE_IMAGE_SIZE, {.str = NULL},  CHAR_MIN, CHAR_MAX, FLAGS },
> +    {"offset",         "set offset to which to display the subtitles", OFFSET(offset_str), AV_OPT_TYPE_STRING,    {.str = "0"},  CHAR_MIN, CHAR_MAX, FLAGS },

I think using the word "delay" would be more appropriate than "offset".
It's a word often used in subtitles (VobSub, SubViewer, ...)

Also, what is the unit?

>      {NULL},
>  };
>  
> @@ -89,7 +92,7 @@ static void ass_log(int ass_level, const char *fmt, va_list args, void *ctx)
>  static av_cold int init(AVFilterContext *ctx, const char *args, const AVClass *class)
>  {
>      AssContext *ass = ctx->priv;
> -    static const char *shorthand[] = { "filename", NULL };
> +    static const char *shorthand[] = { "filename", "offset", NULL };
>      int ret;
>  
>      ass->class = class;
> @@ -103,6 +106,12 @@ static av_cold int init(AVFilterContext *ctx, const char *args, const AVClass *c
>          return AVERROR(EINVAL);
>      }
>  
> +    if ((ret = av_parse_time(&ass->offset, ass->offset_str, 1)) < 0) {
> +        av_log(ctx, AV_LOG_ERROR, "Invalid offset '%s'\n", ass->offset_str);
> +        return ret;
> +    }
> +    ass->offset /= 1000;        /* micro -> milliseconds */
> +

We need a AV_OPT_TYPE_TIME :)

>      ass->library = ass_library_init();
>      if (!ass->library) {
>          av_log(ctx, AV_LOG_ERROR, "Could not initialize libass.\n");
> @@ -117,6 +126,7 @@ static av_cold int init(AVFilterContext *ctx, const char *args, const AVClass *c
>      }
>  
>      ass_set_fonts(ass->renderer, NULL, NULL, 1, NULL, 1);
> +

looks unrelated

>      return 0;
>  }
>  
> @@ -180,12 +190,12 @@ static int filter_frame(AVFilterLink *inlink, AVFilterBufferRef *picref)
>      AVFilterLink *outlink = ctx->outputs[0];
>      AssContext *ass = ctx->priv;
>      int detect_change = 0;
> -    double time_ms = picref->pts * av_q2d(inlink->time_base) * 1000;
> +    double time_ms = picref->pts * av_q2d(inlink->time_base) * 1000 - ass->offset;
>      ASS_Image *image = ass_render_frame(ass->renderer, ass->track,
>                                          time_ms, &detect_change);
> -
> +    av_log(ctx, AV_LOG_INFO, "t(ms):%f\n", time_ms);

remaining debug?

>      if (detect_change)

> -        av_log(ctx, AV_LOG_DEBUG, "Change happened at time ms:%f\n", time_ms);
> +        av_log(ctx, AV_LOG_INFO, "Change happened at time ms:%f\n", time_ms);

why?

[...]

-- 
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/20130104/1af8eeda/attachment.asc>