From 93ce90cbf90089e57cba57249ca2a8b7d9c7ed26 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 22 Dec 2011 21:27:35 +0000 Subject: [PATCH] Store a numerator and denominator in agi::vfr::Framerate rather than a fps. Fixes minor rounding issues with 24000/1001 and 30000/1001 frame rates. Originally committed to SVN as r6115. --- aegisub/libaegisub/common/vfr.cpp | 100 ++++++++++++-------- aegisub/libaegisub/include/libaegisub/vfr.h | 43 +++++++-- aegisub/tests/libaegisub_vfr.cpp | 32 ++++++- 3 files changed, 123 insertions(+), 52 deletions(-) diff --git a/aegisub/libaegisub/common/vfr.cpp b/aegisub/libaegisub/common/vfr.cpp index 30f846f61..540772c0e 100644 --- a/aegisub/libaegisub/common/vfr.cpp +++ b/aegisub/libaegisub/common/vfr.cpp @@ -32,6 +32,7 @@ #include "libaegisub/charset.h" #include "libaegisub/io.h" #include "libaegisub/line_iterator.h" +#include "libaegisub/scoped_ptr.h" namespace std { template<> void swap(agi::vfr::Framerate &lft, agi::vfr::Framerate &rgt) { @@ -39,6 +40,8 @@ namespace std { } } +static const int64_t default_denominator = 1000000000; + namespace agi { namespace vfr { @@ -148,9 +151,9 @@ static void v1_fill_range_gaps(std::list &ranges, double fps) { /// @param file Iterator of lines in the file /// @param line Header of file with assumed fps /// @param[out] timecodes Vector filled with frame start times -/// @param[out] time Unrounded time of the last frame -/// @return Assumed fps -static double v1_parse(line_iterator file, std::string line, std::vector &timecodes, double &time) { +/// @param[out] last Unrounded time of the last frame +/// @return Assumed fps times one million +static int64_t v1_parse(line_iterator file, std::string line, std::vector &timecodes, int64_t &last) { using namespace std; double fps = atof(line.substr(7).c_str()); if (fps <= 0.) throw BadFPS("Assumed FPS must be greater than zero"); @@ -164,7 +167,7 @@ static double v1_parse(line_iterator file, std::string line, std::v v1_fill_range_gaps(ranges, fps); timecodes.reserve(ranges.back().end); - time = 0.; + double time = 0.; for (list::iterator cur = ranges.begin(); cur != ranges.end(); ++cur) { for (int frame = cur->start; frame <= cur->end; frame++) { timecodes.push_back(int(time + .5)); @@ -172,35 +175,59 @@ static double v1_parse(line_iterator file, std::string line, std::v } } timecodes.push_back(int(time + .5)); - return fps; + last = int64_t(time * fps * default_denominator); + return int64_t(fps * default_denominator); } Framerate::Framerate(Framerate const& that) -: fps(that.fps) +: numerator(that.numerator) +, denominator(that.denominator) , last(that.last) , timecodes(that.timecodes) { } -Framerate::Framerate(double fps) : fps(fps), last(0.) { +Framerate::Framerate(double fps) +: denominator(default_denominator) +, numerator(int64_t(fps * denominator)) +, last(0) +{ if (fps < 0.) throw BadFPS("FPS must be greater than zero"); if (fps > 1000.) throw BadFPS("FPS must not be greater than 1000"); + timecodes.push_back(0); +} + +Framerate::Framerate(int64_t numerator, int64_t denominator) +: denominator(denominator) +, numerator(numerator) +, last(0) +{ + if (numerator <= 0 || denominator <= 0) + throw BadFPS("Numerator and denominator must both be greater than zero"); + if (numerator / denominator > 1000) throw BadFPS("FPS must not be greater than 1000"); + timecodes.push_back(0); +} + +void Framerate::SetFromTimecodes() { + validate_timecodes(timecodes); + normalize_timecodes(timecodes); + denominator = default_denominator; + numerator = (timecodes.size() - 1) * denominator * 1000 / timecodes.back(); + last = (timecodes.size() - 1) * denominator * 1000; } Framerate::Framerate(std::vector const& timecodes) : timecodes(timecodes) { - validate_timecodes(timecodes); - normalize_timecodes(this->timecodes); - fps = (timecodes.size() - 1) * 1000. / timecodes.back(); - last = timecodes.back(); + SetFromTimecodes(); } Framerate::~Framerate() { } void Framerate::swap(Framerate &right) throw() { - std::swap(fps, right.fps); + std::swap(numerator, right.numerator); + std::swap(denominator, right.denominator); std::swap(last, right.last); std::swap(timecodes, right.timecodes); } @@ -213,24 +240,24 @@ Framerate &Framerate::operator=(double fps) { return *this = Framerate(fps); } -Framerate::Framerate(std::string const& filename) : fps(0.) { +Framerate::Framerate(std::string const& filename) +: denominator(default_denominator) +, numerator(0) +{ using namespace std; - auto_ptr file(agi::io::Open(filename)); + scoped_ptr file(agi::io::Open(filename)); string encoding = agi::charset::Detect(filename); string line = *line_iterator(*file, encoding); if (line == "# timecode format v2") { copy(line_iterator(*file, encoding), line_iterator(), back_inserter(timecodes)); - validate_timecodes(timecodes); - normalize_timecodes(timecodes); - fps = (timecodes.size() - 1) * 1000. / timecodes.back(); - last = timecodes.back(); + SetFromTimecodes(); return; } if (line == "# timecode format v1" || line.substr(0, 7) == "Assume ") { if (line[0] == '#') { line = *line_iterator(*file, encoding); } - fps = v1_parse(line_iterator(*file, encoding), line, timecodes, last); + numerator = v1_parse(line_iterator(*file, encoding), line, timecodes, last); return; } @@ -248,10 +275,6 @@ void Framerate::Save(std::string const& filename, int length) const { } } -static int round(double value) { - return int(value + .5); -} - int Framerate::FrameAtTime(int ms, Time type) const { // With X ms per frame, this should return 0 for: // EXACT: [0, X - 1] @@ -273,17 +296,13 @@ int Framerate::FrameAtTime(int ms, Time type) const { return FrameAtTime(ms - 1); } - if (timecodes.empty()) { - return (int)floor(ms * fps / 1000.); - } - if (ms < 0) { - return (int)floor(ms * fps / 1000.); - } - if (ms > timecodes.back()) { - return round((ms - timecodes.back()) * fps / 1000.) + (int)timecodes.size() - 1; - } + if (ms < 0) + return int((ms * numerator / denominator - 999) / 1000); - return (int)std::distance(std::lower_bound(timecodes.rbegin(), timecodes.rend(), ms, std::greater()), timecodes.rend()) - 1; + if (ms > timecodes.back()) + return int((ms * numerator - last + denominator - 1) / denominator / 1000) + (int)timecodes.size() - 1; + + return (int)distance(lower_bound(timecodes.rbegin(), timecodes.rend(), ms, std::greater()), timecodes.rend()) - 1; } int Framerate::TimeAtFrame(int frame, Time type) const { @@ -293,22 +312,21 @@ int Framerate::TimeAtFrame(int frame, Time type) const { // + 1 as these need to round up for the case of two frames 1 ms apart return prev + (cur - prev + 1) / 2; } + if (type == END) { int cur = TimeAtFrame(frame); int next = TimeAtFrame(frame + 1); return cur + (next - cur + 1) / 2; } - if (timecodes.empty()) { - return (int)ceil(frame / fps * 1000.); + if (frame < 0) + return (int)(frame * denominator * 1000 / numerator); + + if (frame >= (signed)timecodes.size()) { + int64_t frames_past_end = frame - (int)timecodes.size() + 1; + return int((frames_past_end * 1000 * denominator + last + numerator / 2) / numerator); } - if (frame < 0) { - return (int)ceil(frame / fps * 1000.); - } - if (frame >= (signed)timecodes.size()) { - return round((frame - timecodes.size() + 1) * 1000. / fps + last); - } return timecodes[frame]; } diff --git a/aegisub/libaegisub/include/libaegisub/vfr.h b/aegisub/libaegisub/include/libaegisub/vfr.h index 9ab878bd0..f377abb29 100644 --- a/aegisub/libaegisub/include/libaegisub/vfr.h +++ b/aegisub/libaegisub/include/libaegisub/vfr.h @@ -23,6 +23,8 @@ #ifndef LAGI_PRE #include #include + +#include #endif #include @@ -62,16 +64,31 @@ DEFINE_SIMPLE_EXCEPTION_NOINNER(UnorderedTimecodes, Error, "vfr/timecodes/order" /// @brief Class for managing everything related to converting frames to times /// or vice versa class Framerate { - /// Average FPS for v2, assumed FPS for v1, fps for CFR - double fps; - /// Unrounded time of the last frame in a v1 override range. Needed to - /// match mkvmerge's rounding - double last; + /// Denominator of the FPS + /// + /// For v1 VFR, the assumed FPS is used, for v2 the average FPS + int64_t denominator; + /// Numerator of the FPS + /// + /// For v1 VFR, the assumed FPS is used, for v2 the average FPS + int64_t numerator; + + /// Unrounded frame-seconds of the final frame in timecodes. For CFR and v2, + /// this is simply frame count * denominator, but for v1 it's the + /// "unrounded" frame count, since override ranges generally don't exactly + /// cover timebase-unit ranges of time. This is needed to match mkvmerge's + /// rounding past the end of the final override range. + int64_t last; + /// Start time in milliseconds of each frame std::vector timecodes; + + /// Set FPS properties from the timecodes vector + void SetFromTimecodes(); public: /// Copy constructor Framerate(Framerate const&); + /// @brief VFR from timecodes file /// @param filename File with v1 or v2 timecodes /// @@ -80,13 +97,23 @@ public: /// mkvmerge-style rounding is applied, while setting a constant frame rate /// uses truncation. Framerate(std::string const& filename); + /// @brief CFR constructor /// @param fps Frames per second or 0 for unloaded Framerate(double fps = 0.); + + /// @brief CFR constructor with rational timebase + /// @param numerator Timebase numerator + /// @param denominator Timebase denominator + Framerate(int64_t numerator, int64_t denominator); + /// @brief VFR from frame times /// @param timecodes Vector of frame start times in milliseconds Framerate(std::vector const& timecodes); + + /// Destructor ~Framerate(); + /// Atomic assignment operator Framerate &operator=(Framerate); /// Atomic CFR assignment operator @@ -130,9 +157,9 @@ public: /// be otherwise sensible. void Save(std::string const& file, int length = -1) const; - bool IsVFR() const {return !timecodes.empty(); } - bool IsLoaded() const { return !timecodes.empty() || fps; }; - double FPS() const { return fps; } + bool IsVFR() const {return timecodes.size() > 1; } + bool IsLoaded() const { return numerator > 0; }; + double FPS() const { return double(numerator) / denominator; } }; } // namespace vfr diff --git a/aegisub/tests/libaegisub_vfr.cpp b/aegisub/tests/libaegisub_vfr.cpp index 77dd5661b..9e6bc51c7 100644 --- a/aegisub/tests/libaegisub_vfr.cpp +++ b/aegisub/tests/libaegisub_vfr.cpp @@ -20,6 +20,7 @@ #include +#include #include #include @@ -334,7 +335,7 @@ TEST(lagi_vfr, save_vfr_len) { TEST(lagi_vfr, load_v2) { Framerate fps; ASSERT_NO_THROW(fps = Framerate("data/vfr/in/v2_1fps.txt")); - for (int i = 0; i < 9; i++) { + for (int i = 0; i < 30; i++) { EXPECT_EQ(i * 1000, fps.TimeAtFrame(i)); } } @@ -342,7 +343,7 @@ TEST(lagi_vfr, load_v2) { TEST(lagi_vfr, load_v2_comments) { Framerate fps; ASSERT_NO_THROW(fps = Framerate("data/vfr/in/v2_comments.txt")); - for (int i = 0; i < 9; i++) { + for (int i = 0; i < 30; i++) { EXPECT_EQ(i * 1000, fps.TimeAtFrame(i)); } } @@ -350,7 +351,7 @@ TEST(lagi_vfr, load_v2_comments) { TEST(lagi_vfr, load_v2_number_in_comment) { Framerate fps; ASSERT_NO_THROW(fps = Framerate("data/vfr/in/v2_number_in_comment.txt")); - for (int i = 0; i < 9; i++) { + for (int i = 0; i < 30; i++) { EXPECT_EQ(i * 1000, fps.TimeAtFrame(i)); } } @@ -394,3 +395,28 @@ TEST(lagi_vfr, nonzero_start_time) { EXPECT_EQ(50, fps.TimeAtFrame(3, EXACT)); EXPECT_EQ(60, fps.TimeAtFrame(4, EXACT)); } + +TEST(lagi_vfr, rational_timebase) { + Framerate fps; + + ASSERT_NO_THROW(fps = Framerate(30000, 1001)); + for (int i = 0; i < 100000; ++i) { + EXPECT_EQ(i * 1001, fps.TimeAtFrame(i * 30, EXACT)); + EXPECT_EQ(i * 30, fps.FrameAtTime(i * 1001, EXACT)); + } + + ASSERT_NO_THROW(fps = Framerate(24000, 1001)); + for (int i = 0; i < 100000; ++i) { + EXPECT_EQ(i * 1001, fps.TimeAtFrame(i * 24, EXACT)); + EXPECT_EQ(i * 24, fps.FrameAtTime(i * 1001, EXACT)); + } +} + +TEST(lagi_vfr, no_intermediate_overflow) { + Framerate fps; + + ASSERT_NO_THROW(fps = Framerate(1.0)); + int last_frame = INT_MAX / 1000; + EXPECT_EQ(last_frame * 1000, fps.TimeAtFrame(last_frame, EXACT)); + EXPECT_EQ(last_frame, fps.FrameAtTime(last_frame * 1000, EXACT)); +}