[FFmpeg-devel] [PATCH] avcodec/srtdec: Keep exact end times
Rodger Combs
rodger.combs at gmail.com
Tue Dec 29 03:41:48 CET 2015
> On Dec 3, 2015, at 03:30, Eelco Lempsink <eml at 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 at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list