Re: [FFmpeg-devel] [PATCH] avcodec/samidec: fix misposition of subtitles caused by rounding
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...@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@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/samidec: fix misposition of subtitles caused by rounding
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. pgp4LcviT5zYa.pgp Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avcodec/samidec: fix misposition of subtitles caused by rounding
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; -- 1.9.4.msysgit.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel