Reject out-of-range seeks without decoding to EOF

src/VideoReader.cpp

@@ -327,6 +327,16 @@ bool VideoReader::seek(int64_t frame_number) {
         return false;
     }
 
+    // Reject a knowably out-of-range target before doing any work. Without
+    // this the seek scans forward decoding every remaining frame only to fail
+    // at EOF, which is O(frames remaining) of full decode for a request that
+    // cannot succeed. frame_count_ is -1 when the container does not report
+    // one (common with MKV and some MP4), and then the scan really is the only
+    // way to find out, so the guard stays conditional.
+    if (frame_count_ > 0 && frame_number >= frame_count_) {
+        return false;
+    }
+
     if (current_frame_ == frame_number) {
         return true;
     }
@@ -469,7 +479,9 @@ void VideoReader::cleanup() {
     width_ = 0;
     height_ = 0;
     fps_ = 0.0;
-    frame_count_ = 0;
+    // -1 means "unknown", matching open() and what getFrameCount() documents.
+    // 0 would be a legitimate-looking count for a closed reader.
+    frame_count_ = -1;
     current_frame_ = 0;
     current_timestamp_ = 0.0;
 }

tests/test_video_reader.cpp

@@ -258,6 +258,43 @@ TEST_CASE("VideoReader seek past the end fails", "[reader][seek]") {
     CHECK_FALSE(reader.seek(kSeekFixtureFrames + 100));
 }
 
+// A seek that cannot succeed should be rejected outright, not by scanning to
+// EOF and discovering the failure there. The observable difference is the
+// position: rejecting early leaves the reader where it was, whereas the old
+// scan consumed the rest of the file first. See #67.
+TEST_CASE("VideoReader rejects out-of-range seek without moving", "[reader][seek]") {
+    const std::string path = fixtures + "/seek_numbered.mp4";
+    const std::vector<int> ref = seekReference(path);
+
+    framewright::VideoReader reader;
+    REQUIRE(reader.open(path));
+    REQUIRE(reader.getFrameCount() == kSeekFixtureFrames);
+
+    cv::Mat frame;
+    for (int i = 0; i < 5; i++) REQUIRE(reader.read(frame));
+    REQUIRE(reader.getCurrentFrameNumber() == 5);
+
+    CHECK_FALSE(reader.seek(kSeekFixtureFrames));       // one past the last valid index
+    CHECK_FALSE(reader.seek(kSeekFixtureFrames + 100));
+
+    // Still at frame 5, and the next read still returns frame 5.
+    CHECK(reader.getCurrentFrameNumber() == 5);
+    REQUIRE(reader.read(frame));
+    CHECK(identifyFrame(frame, ref) == 5);
+}
+
+TEST_CASE("VideoReader reports unknown frame count after close", "[reader]") {
+    framewright::VideoReader reader;
+    REQUIRE(reader.open(fixtures + "/seek_numbered.mp4"));
+    CHECK(reader.getFrameCount() > 0);
+
+    reader.close();
+
+    // -1 is "unknown", as documented on getFrameCount(). 0 would read as a
+    // real count of zero frames.
+    CHECK(reader.getFrameCount() == -1);
+}
+
 #endif // HAVE_SEEK_FIXTURE
 
 TEST_CASE("VideoReader close and reopen", "[reader]") {