From f7e5e5c5a4adeb1df6242cbe656f6ec32c7b9d98 Mon Sep 17 00:00:00 2001 From: Daniel Flores Date: Thu, 31 Jul 2025 10:53:31 -0400 Subject: [PATCH 1/2] Remove av_q2d, remove test rounding --- src/torchcodec/_core/SingleStreamDecoder.cpp | 3 ++- test/VideoDecoderTest.cpp | 16 ++++++++-------- test/test_ops.py | 4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/torchcodec/_core/SingleStreamDecoder.cpp b/src/torchcodec/_core/SingleStreamDecoder.cpp index 96898de68..e27948270 100644 --- a/src/torchcodec/_core/SingleStreamDecoder.cpp +++ b/src/torchcodec/_core/SingleStreamDecoder.cpp @@ -18,7 +18,8 @@ namespace facebook::torchcodec { namespace { double ptsToSeconds(int64_t pts, const AVRational& timeBase) { - return static_cast(pts) * av_q2d(timeBase); + // av_q2d is intentionally not used for increased precision + return static_cast(pts) * timeBase.num / timeBase.den; } int64_t secondsToClosestPts(double seconds, const AVRational& timeBase) { diff --git a/test/VideoDecoderTest.cpp b/test/VideoDecoderTest.cpp index 88ffc94cd..0c21f0d46 100644 --- a/test/VideoDecoderTest.cpp +++ b/test/VideoDecoderTest.cpp @@ -267,9 +267,9 @@ TEST_P(SingleStreamDecoderTest, SeeksCloseToEof) { ourDecoder->addVideoStream(-1); ourDecoder->setCursorPtsInSeconds(388388. / 30'000); auto output = ourDecoder->getNextFrame(); - EXPECT_DOUBLE_EQ(output.ptsSeconds, 388'388. / 30'000); + EXPECT_EQ(output.ptsSeconds, 388'388. / 30'000); output = ourDecoder->getNextFrame(); - EXPECT_DOUBLE_EQ(output.ptsSeconds, 389'389. / 30'000); + EXPECT_EQ(output.ptsSeconds, 389'389. / 30'000); EXPECT_THROW(ourDecoder->getNextFrame(), std::exception); } @@ -300,7 +300,7 @@ TEST_P(SingleStreamDecoderTest, GetsFramePlayedAtTimestamp) { // Sanity check: make sure duration is strictly positive. EXPECT_GT(kPtsPlusDurationOfLastFrame, kPtsOfLastFrameInVideoStream); output = ourDecoder->getFramePlayedAt(kPtsPlusDurationOfLastFrame - 1e-6); - EXPECT_DOUBLE_EQ(output.ptsSeconds, kPtsOfLastFrameInVideoStream); + EXPECT_EQ(output.ptsSeconds, kPtsOfLastFrameInVideoStream); } TEST_P(SingleStreamDecoderTest, SeeksToFrameWithSpecificPts) { @@ -311,7 +311,7 @@ TEST_P(SingleStreamDecoderTest, SeeksToFrameWithSpecificPts) { ourDecoder->setCursorPtsInSeconds(6.0); auto output = ourDecoder->getNextFrame(); torch::Tensor tensor6FromOurDecoder = output.data; - EXPECT_DOUBLE_EQ(output.ptsSeconds, 180'180. / 30'000); + EXPECT_EQ(output.ptsSeconds, 180'180. / 30'000); torch::Tensor tensor6FromFFMPEG = readTensorFromDisk("nasa_13013.mp4.time6.000000.pt"); EXPECT_TRUE(torch::equal(tensor6FromOurDecoder, tensor6FromFFMPEG)); @@ -327,7 +327,7 @@ TEST_P(SingleStreamDecoderTest, SeeksToFrameWithSpecificPts) { ourDecoder->setCursorPtsInSeconds(6.1); output = ourDecoder->getNextFrame(); torch::Tensor tensor61FromOurDecoder = output.data; - EXPECT_DOUBLE_EQ(output.ptsSeconds, 183'183. / 30'000); + EXPECT_EQ(output.ptsSeconds, 183'183. / 30'000); torch::Tensor tensor61FromFFMPEG = readTensorFromDisk("nasa_13013.mp4.time6.100000.pt"); EXPECT_TRUE(torch::equal(tensor61FromOurDecoder, tensor61FromFFMPEG)); @@ -347,7 +347,7 @@ TEST_P(SingleStreamDecoderTest, SeeksToFrameWithSpecificPts) { ourDecoder->setCursorPtsInSeconds(10.0); output = ourDecoder->getNextFrame(); torch::Tensor tensor10FromOurDecoder = output.data; - EXPECT_DOUBLE_EQ(output.ptsSeconds, 300'300. / 30'000); + EXPECT_EQ(output.ptsSeconds, 300'300. / 30'000); torch::Tensor tensor10FromFFMPEG = readTensorFromDisk("nasa_13013.mp4.time10.000000.pt"); EXPECT_TRUE(torch::equal(tensor10FromOurDecoder, tensor10FromFFMPEG)); @@ -364,7 +364,7 @@ TEST_P(SingleStreamDecoderTest, SeeksToFrameWithSpecificPts) { ourDecoder->setCursorPtsInSeconds(6.0); output = ourDecoder->getNextFrame(); tensor6FromOurDecoder = output.data; - EXPECT_DOUBLE_EQ(output.ptsSeconds, 180'180. / 30'000); + EXPECT_EQ(output.ptsSeconds, 180'180. / 30'000); EXPECT_TRUE(torch::equal(tensor6FromOurDecoder, tensor6FromFFMPEG)); EXPECT_EQ(ourDecoder->getDecodeStats().numSeeksAttempted, 1); // We cannot skip a seek here because timestamp=6 has a different keyframe @@ -379,7 +379,7 @@ TEST_P(SingleStreamDecoderTest, SeeksToFrameWithSpecificPts) { ourDecoder->setCursorPtsInSeconds(kPtsOfLastFrameInVideoStream); output = ourDecoder->getNextFrame(); torch::Tensor tensor7FromOurDecoder = output.data; - EXPECT_DOUBLE_EQ(output.ptsSeconds, 389'389. / 30'000); + EXPECT_EQ(output.ptsSeconds, 389'389. / 30'000); torch::Tensor tensor7FromFFMPEG = readTensorFromDisk("nasa_13013.mp4.time12.979633.pt"); EXPECT_TRUE(torch::equal(tensor7FromOurDecoder, tensor7FromFFMPEG)); diff --git a/test/test_ops.py b/test/test_ops.py index bd1242f1f..d86cb2437 100644 --- a/test/test_ops.py +++ b/test/test_ops.py @@ -781,7 +781,7 @@ def test_next(self, asset): frame, asset.get_frame_data_by_index(frame_index) ) frame_info = asset.get_frame_info(frame_index) - assert pts_seconds == pytest.approx(frame_info.pts_seconds) + assert pts_seconds == frame_info.pts_seconds assert duration_seconds == frame_info.duration_seconds frame_index += 1 @@ -985,7 +985,7 @@ def test_pts(self, asset): frames, asset.get_frame_data_by_index(frame_index) ) - assert pts_seconds == pytest.approx(start_seconds) + assert pts_seconds == start_seconds def test_sample_rate_conversion(self): def get_all_frames(asset, sample_rate=None, stop_seconds=None): From 9d0857ed7ec8c37d9b28772ffd5347727d630d90 Mon Sep 17 00:00:00 2001 From: Daniel Flores Date: Thu, 31 Jul 2025 11:06:26 -0400 Subject: [PATCH 2/2] update comment --- src/torchcodec/_core/SingleStreamDecoder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/torchcodec/_core/SingleStreamDecoder.cpp b/src/torchcodec/_core/SingleStreamDecoder.cpp index e27948270..6c2cdd490 100644 --- a/src/torchcodec/_core/SingleStreamDecoder.cpp +++ b/src/torchcodec/_core/SingleStreamDecoder.cpp @@ -18,7 +18,7 @@ namespace facebook::torchcodec { namespace { double ptsToSeconds(int64_t pts, const AVRational& timeBase) { - // av_q2d is intentionally not used for increased precision + // To perform the multiplication before the division, av_q2d is not used return static_cast(pts) * timeBase.num / timeBase.den; }