[FFmpeg-devel] [PATCH] avcodec/samidec: fix misposition of subtitles caused by rounding

[Jeong-Hoon Seo]

Thanks for your point of view.

I'll fix properly and submit later.

2014-11-27 5:53 GMT+09:00 Clément Bœsch <u at pkh.me>:

> On Tue, Nov 25, 2014 at 06:07:30PM +0900, Jeong-Hoon Seo wrote:
> > SAMI and ASS have different precision of duration in FFMPEG
> > (SAMI: millisecond, ASS: centisecond)
> > Sometimes, start-time of current subtitle is overrun with end-time of
> previous one by rounding process
> > This led to misposition of subtitles
> >   <exmple>
> >   - input SAMI
> >   <sync start=1155>
> >     <p class=ENCC> start
> >   <sync start=2850>
> >     <p class=ENCC> end
> >   <sync start=3850>
> >     <p class=ENCC> !
> >   - convert ASS
> >   Dialogue: 0,0:00:01:16,0:00:02:86,Default,start
> >   Dialogue: 0,0:00:02:85,0:00:03:85,Default,end
> >   ...
> >
> >   each values in ASS means "Format": "Layer","Start","End","Style","Text"
> >   "End" value = <sync start> + rounding(difference of <sync start>)
> >   0:00:02:86 = 0:00:01:16 + rounding(2850-1151)
> >
> > in the converted ASS,
> > start-time of 2nd "Text" is overrun by end-time of 1st "Text",
> > and some frames will overlaid with multi-line text
> > (during 0:00:02:85~0:00:02:86, 2nd one is on top of 1st one)
> > After 1st text end-time(expired), 2nd one keep previous
> position(misposition)
> > ---
> >  libavcodec/samidec.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/samidec.c b/libavcodec/samidec.c
> > index 7705f93..92a9863 100644
> > --- a/libavcodec/samidec.c
> > +++ b/libavcodec/samidec.c
> > @@ -119,9 +119,9 @@ static int sami_decode_frame(AVCodecContext *avctx,
> >      SAMIContext *sami = avctx->priv_data;
> >
> >      if (ptr && avpkt->size > 0 && !sami_paragraph_to_ass(avctx, ptr)) {
> > -        int ts_start     = av_rescale_q(avpkt->pts, avctx->time_base,
> (AVRational){1,100});
> > +        int ts_start     = av_rescale_q_rnd(avpkt->pts,
> avctx->time_base, (AVRational){1,100}, AV_ROUND_ZERO);
> >          int ts_duration  = avpkt->duration != -1 ?
> > -                           av_rescale_q(avpkt->duration,
> avctx->time_base, (AVRational){1,100}) : -1;
> > +                           av_rescale_q_rnd(avpkt->duration,
> avctx->time_base, (AVRational){1,100}, AV_ROUND_ZERO) : -1;
> >          int ret = ff_ass_add_rect_bprint(sub, &sami->full, ts_start,
> ts_duration);
> >          if (ret < 0)
> >              return ret;
>
> Well I'm fine with this but:
>  - does the FATE test need updates?
>  - this change is required in other text subtitles decoders as well
>  - the subtitles decoders shouldn't have this code in the first place, so
>    be aware this isn't the proper fix (but is an acceptable change in
>    itself)
>
> Note: I'm working on dropping that code altogether, but it takes some
> time
>
> [...]
>
> --
> Clément B.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>