> On Dec 3, 2015, at 03:30, Eelco Lempsink <e...@tupil.com> wrote: > > When converting SRT to SRT (to normalize) or WebVTT the end timestamps were > modified compared to the original. > > Fixes trac 4783. > > NOTE: The FATE test 'sub-srt' fails after this patch, because the end times of > the Dialogue lines change from X:XX:XX.50 to X:XX:XX.49. I can argue that this > is semantically correct, because in the original SRT the begin and end times > are such that there is no (unintended) overlap between cues, but since the > semantics of SRT are poorly defined, I’m not sure it’s correct. This is incorrect. ASS doesn't produce overlap when 2 consecutive lines end and start on the same timestamp, since it uses `<` instead of `<=` when comparing end times.
> --- > libavcodec/srtdec.c | 15 ++++++---- > tests/ref/fate/sub-webvttenc | 66 ++++++++++++++++++++++---------------------- > 2 files changed, 43 insertions(+), 38 deletions(-) > > diff --git a/libavcodec/srtdec.c b/libavcodec/srtdec.c > index 542dd35..54568ca 100644 > --- a/libavcodec/srtdec.c > +++ b/libavcodec/srtdec.c > @@ -57,7 +57,7 @@ static int srt_decode_frame(AVCodecContext *avctx, > { > AVSubtitle *sub = data; > AVBPrint buffer; > - int ts_start, ts_end, x1 = -1, y1 = -1, x2 = -1, y2 = -1; > + int ts_start, ts_duration, x1 = -1, y1 = -1, x2 = -1, y2 = -1; > int size, ret; > const uint8_t *p = av_packet_get_side_data(avpkt, > AV_PKT_DATA_SUBTITLE_POSITION, &size); > > @@ -77,12 +77,17 @@ static int srt_decode_frame(AVCodecContext *avctx, > ts_start = av_rescale_q(avpkt->pts, > avctx->time_base, > (AVRational){1,100}); > - ts_end = av_rescale_q(avpkt->pts + avpkt->duration, > - avctx->time_base, > - (AVRational){1,100}); > + > + // Floor the duration (for ASS output) > + ts_duration = avpkt->duration / 10; > + > + // Set an exact end display time to prevent the rounding for ASS messing > it up > + sub->end_display_time = av_rescale_q(avpkt->duration, > + avctx->pkt_timebase, > + (AVRational){1,1000}); Is this patch still effective if you just add this^ statement, and leave out the ts_end/ts_duration changes (to preserve the ASS rounding behavior)? > > srt_to_ass(avctx, &buffer, avpkt->data, x1, y1, x2, y2); > - ret = ff_ass_add_rect_bprint(sub, &buffer, ts_start, ts_end-ts_start); > + ret = ff_ass_add_rect_bprint(sub, &buffer, ts_start, ts_duration); > av_bprint_finalize(&buffer, NULL); > if (ret < 0) > return ret; > diff --git a/tests/ref/fate/sub-webvttenc b/tests/ref/fate/sub-webvttenc > index dbeadb0..ba567c3 100644 > --- a/tests/ref/fate/sub-webvttenc > +++ b/tests/ref/fate/sub-webvttenc > @@ -14,12 +14,12 @@ If you see this with the normal font, the player don't > (fully) support font face > 00:04.500 --> 00:04.500 > Hidden > > -00:04.501 --> 00:07.501 > +00:04.501 --> 00:07.500 > This text should be small > This text should be normal > This text should be big > > -00:07.501 --> 00:11.501 > +00:07.501 --> 00:11.500 > This should be an E with an accent: È > 日本語 > <b><i><u>This text should be bold, italics and underline</u></i></b> > @@ -27,7 +27,7 @@ This text should be small and green > This text should be small and red > This text should be big and brown > > -00:11.501 --> 00:14.501 > +00:11.501 --> 00:14.500 > <b>This line should be bold</b> > <i>This line should be italics</i> > <u>This line should be underline</u> > @@ -35,7 +35,7 @@ This line should be strikethrough > <u>Both lines > should be underline</u> > > -00:14.501 --> 00:17.501 > +00:14.501 --> 00:17.500 >> > It would be a good thing to > hide invalid html tags that are closed and show the text in them > @@ -43,110 +43,110 @@ hide invalid html tags that are closed and show the > text in them > Show not opened tags</invalid_tag_not_opened> > < > > -00:17.501 --> 00:20.501 > +00:17.501 --> 00:20.500 > and also > hide invalid html tags with parameters that are closed and show the text in > them > <invalid_tag_uc par=5>but show un-closed invalid html tags > <u>This text should be showed underlined without problems also: > 2<3,5>1,4<6</u> > This shouldn't be underlined > > -00:20.501 --> 00:21.501 > +00:20.501 --> 00:21.500 > This text should be in the normal position... > > -00:21.501 --> 00:22.501 > +00:21.501 --> 00:22.500 > This text should NOT be in the normal position > > -00:22.501 --> 00:24.501 > +00:22.501 --> 00:24.500 > Implementation is the same of the ASS tag > This text should be at the > top and horizontally centered > > -00:22.501 --> 00:24.501 > +00:22.501 --> 00:24.500 > This text should be at the > middle and horizontally centered > > -00:22.501 --> 00:24.501 > +00:22.501 --> 00:24.500 > This text should be at the > bottom and horizontally centered > > -00:24.501 --> 00:26.501 > +00:24.501 --> 00:26.500 > This text should be at the > top and horizontally at the left > > -00:24.501 --> 00:26.501 > +00:24.501 --> 00:26.500 > This text should be at the > middle and horizontally at the left > (The second position must be ignored) > > -00:24.501 --> 00:26.501 > +00:24.501 --> 00:26.500 > This text should be at the > bottom and horizontally at the left > > -00:26.501 --> 00:28.501 > +00:26.501 --> 00:28.500 > This text should be at the > top and horizontally at the right > > -00:26.501 --> 00:28.501 > +00:26.501 --> 00:28.500 > This text should be at the > middle and horizontally at the right > > -00:26.501 --> 00:28.501 > +00:26.501 --> 00:28.500 > This text should be at the > bottom and horizontally at the right > > -00:28.501 --> 00:31.501 > +00:28.501 --> 00:31.500 > This could be the most difficult thing to implement > > -00:31.501 --> 00:50.501 > +00:31.501 --> 00:50.500 > First text > > 00:33.500 --> 00:35.500 > Second, it shouldn't overlap first > > -00:35.501 --> 00:37.501 > +00:35.501 --> 00:37.500 > Third, it should replace second > > -00:36.501 --> 00:50.501 > +00:36.501 --> 00:50.500 > Fourth, it shouldn't overlap first and third > > -00:40.501 --> 00:45.501 > +00:40.501 --> 00:45.500 > Fifth, it should replace third > > -00:45.501 --> 00:50.501 > +00:45.501 --> 00:50.500 > Sixth, it shouldn't be > showed overlapped > > -00:50.501 --> 00:52.501 > +00:50.501 --> 00:52.500 > TEXT 1 (bottom) > > -00:50.501 --> 00:52.501 > +00:50.501 --> 00:52.500 > text 2 > > -00:52.501 --> 00:54.501 > +00:52.501 --> 00:54.500 > Hide these tags: > also hide these tags: > but show this: {normal text} > > -00:54.501 --> 01:00.501 > +00:54.501 --> 01:00.500 > > \ N is a forced line break > \ h is a hard space > Normal spaces at the start and at the end of the line are trimmed while hard > spaces are not trimmed. > The\hline\hwill\hnever\hbreak\hautomatically\hright\hbefore\hor\hafter\ha\hhard\hspace.\h:-D > > -00:54.501 --> 00:56.501 > +00:54.501 --> 00:56.500 > > \h\h\h\h\hA (05 hard spaces followed by a letter) > A (Normal spaces followed by a letter) > A (No hard spaces followed by a letter) > > -00:56.501 --> 00:58.501 > +00:56.501 --> 00:58.500 > \h\h\h\h\hA (05 hard spaces followed by a letter) > A (Normal spaces followed by a letter) > A (No hard spaces followed by a letter) > Show this: \TEST and this: \-) > > -00:58.501 --> 01:00.501 > +00:58.501 --> 01:00.500 > > A letter followed by 05 hard spaces: A\h\h\h\h\h > A letter followed by normal spaces: A > @@ -156,22 +156,22 @@ A letter followed by no hard spaces: A > > ^--Forced line break > > -01:00.501 --> 01:02.501 > +01:00.501 --> 01:02.500 > Both line should be strikethrough, > yes. > Correctly closed tags > should be hidden. > > -01:02.501 --> 01:04.501 > +01:02.501 --> 01:04.500 > It shouldn't be strikethrough, > not opened tag showed as text.</s> > Not opened tag showed as text.</xxxxx> > > -01:04.501 --> 01:06.501 > +01:04.501 --> 01:06.500 > Three lines should be strikethrough, > yes. > <yyyy>Not closed tags showed as text > > -01:06.501 --> 01:08.501 > +01:06.501 --> 01:08.500 > Both line should be strikethrough but > the wrong closing tag should be showed</b> > -- > 2.4.9 (Apple Git-60) > > _______________________________________________ > 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