[FFmpeg-devel] [PATCH] avcodec/samidec: fix misposition of subtitles caused by rounding
Clément Bœsch
u at pkh.me
Wed Nov 26 21:53:19 CET 2014
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.
-------------- 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/20141126/701f455b/attachment.asc>
More information about the ffmpeg-devel
mailing list