From e0c973e5bea94dc70baf20d5a36e123b1ca1f901 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer Date: Tue, 30 Jul 2019 19:11:27 +0200 Subject: [PATCH] avformat/mpsubdec: Remove floating point usage This makes the code bitexact between platforms. Intermediate timestamps between frames are preserved. The timebase is simplified. Rounding differs from doubles in cases where timestamps/durations are "funny" Suggested-by: jb Signed-off-by: Michael Niedermayer --- libavformat/mpsubdec.c | 84 ++++++++++++++++++++++++++------- tests/ref/fate/sub-mpsub-frames | 4 +- 2 files changed, 69 insertions(+), 19 deletions(-) diff --git a/libavformat/mpsubdec.c b/libavformat/mpsubdec.c index a8217a4a61..3d8dcb35c2 100644 --- a/libavformat/mpsubdec.c +++ b/libavformat/mpsubdec.c @@ -27,6 +27,8 @@ #include "internal.h" #include "subtitles.h" +#define TSBASE 10000000 + typedef struct { FFDemuxSubtitlesQueue q; } MPSubContext; @@ -51,21 +53,55 @@ static int mpsub_probe(const AVProbeData *p) return 0; } +static int parse_line(const char *line, int64_t *value, int64_t *value2) +{ + int vi, p1, p2; + + for (vi = 0; vi < 2; vi++) { + long long intval, fracval; + int n = av_sscanf(line, "%lld%n.%lld%n", &intval, &p1, &fracval, &p2); + if (n <= 0 || intval < INT64_MIN / TSBASE || intval > INT64_MAX / TSBASE) + return AVERROR_INVALIDDATA; + + intval *= TSBASE; + + if (n == 2) { + if (fracval < 0) + return AVERROR_INVALIDDATA; + for (;p2 - p1 < 7 + 1; p1--) + fracval *= 10; + for (;p2 - p1 > 7 + 1; p1++) + fracval /= 10; + if (intval > 0) intval += fracval; + else intval -= fracval; + line += p2; + } else + line += p1; + + *value = intval; + + value = value2; + } + + return 0; +} + static int mpsub_read_header(AVFormatContext *s) { MPSubContext *mpsub = s->priv_data; AVStream *st; AVBPrint buf; - AVRational pts_info = (AVRational){ 100, 1 }; // ts based by default + AVRational pts_info = (AVRational){ TSBASE, 1 }; // ts based by default int res = 0; - int multiplier = 100; - double current_pts = 0; + int64_t current_pts = 0; + int i; + int common_factor = 0; av_bprint_init(&buf, 0, AV_BPRINT_SIZE_UNLIMITED); while (!avio_feof(s->pb)) { char line[1024]; - double start, duration; + int64_t start, duration; int fps, len = ff_get_line(s->pb, line, sizeof(line)); if (!len) @@ -75,34 +111,48 @@ static int mpsub_read_header(AVFormatContext *s) if (sscanf(line, "FORMAT=%d", &fps) == 1 && fps > 3 && fps < 100) { /* frame based timing */ - pts_info = (AVRational){ fps, 1 }; - multiplier = 1; - } else if (sscanf(line, "%lf %lf", &start, &duration) == 2) { + pts_info = (AVRational){ TSBASE * fps, 1 }; + } else if (parse_line(line, &start, &duration) >= 0) { AVPacket *sub; const int64_t pos = avio_tell(s->pb); ff_subtitles_read_chunk(s->pb, &buf); if (buf.len) { - double ts = current_pts + start*multiplier; sub = ff_subtitles_queue_insert(&mpsub->q, buf.str, buf.len, 0); if (!sub) { res = AVERROR(ENOMEM); goto end; } - if (!isfinite(ts) || ts < INT64_MIN || ts > INT64_MAX) { - avpriv_request_sample(s, "Invalid ts\n"); - } else - sub->pts = (int64_t)ts; - if (!isfinite(duration) || duration * multiplier > INT_MAX || duration < 0) { - avpriv_request_sample(s, "Invalid duration\n"); - } else - sub->duration = (int)(duration * multiplier); - current_pts += (start + duration) * multiplier; + if ( current_pts < 0 && start < INT64_MIN - current_pts + || current_pts > 0 && start > INT64_MAX - current_pts) { + res = AVERROR_INVALIDDATA; + goto end; + } + sub->pts = current_pts + start; + if (duration < 0 || sub->pts > INT64_MAX - duration) { + res = AVERROR_INVALIDDATA; + goto end; + } + sub->duration = duration; + + common_factor = av_gcd(duration, common_factor); + common_factor = av_gcd(sub->pts, common_factor); + + current_pts = sub->pts + duration; sub->pos = pos; } } } + if (common_factor > 1) { + common_factor = av_gcd(pts_info.num, common_factor); + for (i = 0; i < mpsub->q.nb_subs; i++) { + mpsub->q.subs[i].pts /= common_factor; + mpsub->q.subs[i].duration /= common_factor; + } + pts_info.num /= common_factor; + } + st = avformat_new_stream(s, NULL); if (!st) return AVERROR(ENOMEM); diff --git a/tests/ref/fate/sub-mpsub-frames b/tests/ref/fate/sub-mpsub-frames index 64528ec70e..17695787be 100644 --- a/tests/ref/fate/sub-mpsub-frames +++ b/tests/ref/fate/sub-mpsub-frames @@ -10,5 +10,5 @@ Style: Default,Arial,16,&Hffffff,&Hffffff,&H0,&H0,0,0,0,0,100,100,0,0,1,1,0,2,10 [Events] Format: Layer, Start, End, Style, Name, MarginL, MarginR, MarginV, Effect, Text -Dialogue: 0,0:00:01.00,0:00:02.48,Default,,0,0,0,,Start at 1sec,\Nlast 1.5 seconds -Dialogue: 0,0:00:02.52,0:00:11.52,Default,,0,0,0,,One frame later,\Nduring 9 seconds +Dialogue: 0,0:00:01.00,0:00:02.50,Default,,0,0,0,,Start at 1sec,\Nlast 1.5 seconds +Dialogue: 0,0:00:02.54,0:00:11.54,Default,,0,0,0,,One frame later,\Nduring 9 seconds