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

2014-11-30 Thread 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...@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

2014-11-26 Thread Clément Bœsch 
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

2014-11-25 Thread Jeong-Hoon Seo 
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