diff --git a/scenedetect/backends/pyav.py b/scenedetect/backends/pyav.py index 2e3b4195..34514a04 100644 --- a/scenedetect/backends/pyav.py +++ b/scenedetect/backends/pyav.py @@ -15,6 +15,7 @@ import os import typing as ty import warnings +from enum import Enum, auto from fractions import Fraction from logging import getLogger @@ -30,11 +31,45 @@ framerate_to_fraction, ) from scenedetect.platform import StrPath, get_file_name -from scenedetect.video_stream import FrameRateUnavailable, VideoOpenFailure, VideoStream +from scenedetect.video_stream import ( + FrameRateUnavailable, + SeekError, + VideoOpenFailure, + VideoStream, +) logger = getLogger("pyscenedetect") VALID_THREAD_MODES = ["NONE", "SLICE", "FRAME", "AUTO"] +# FFmpeg options passed to `av.open()`. By default, PyAV enables `AVFMT_FLAG_GENPTS` (`+genpts`) on +# every input container. For containers that only store decode timestamps (e.g. AVI), `+genpts` +# synthesizes missing presentation timestamps (PTS) from packet decode timestamps (DTS), which are +# in decode order rather than presentation order. When the video contains B-frames, the decoder +# reorders frames into presentation order while keeping those decode-order PTS values attached, +# causing `frame.pts` (and `best_effort_timestamp` on the initial frames) to jump backwards and +# forwards. Disabling `genpts` leaves `frame.pts` as `None` on DTS-only streams so we can fall back +# to `frame.dts` cleanly. See https://scenedetect.com/issues/590 and +# https://github.com/PyAV-Org/PyAV/pull/2478. +_CONTAINER_OPTIONS = {"fflags": "-genpts"} + + +class _TimeSource(Enum): + """The origin of the stream's frame position properties. + + Determined once per stream from the first decoded frame and used for all subsequent frames. + PTS and DTS are never mixed within a stream because DTS leads PTS by the decoder's B-frame + reordering delay. + """ + + PTS = auto() + """Frames carry presentation timestamps (`frame.pts`).""" + DTS = auto() + """Frames only carry decode timestamps (`frame.dts`, e.g. AVI or some MPEG-TS streams).""" + FRAMES = auto() + """Frames carry no timestamps at all (e.g. raw Annex-B H.264); positions are derived from the + number of decoded frames.""" + + MAX_CONSECUTIVE_DECODE_FAILURES = 8 """Number of consecutive frame decode failures after which `VideoStreamAv.read()` gives up. Isolated corrupt frames are skipped; this bound ensures a truncated file still terminates.""" @@ -117,6 +152,25 @@ def __init__( self._reopened = True self._decode_failures = 0 self._warning_displayed = False + # Presentation time of `_frame` relative to the start of the stream, or None before the + # first `read()` (and after `close()` / `reset()`). + self._timecode: Timecode | None = None + # Timestamp source selected from the first decoded frame. Preserved across `reset()` since + # it is a property of the input stream, not the container instance. + self._time_source: _TimeSource | None = None + # In `_TimeSource.DTS` mode, the DTS (and time base) of the first frame decoded at the + # start of the stream. Subtracted from subsequent `frame.dts` values so frame 1 starts at + # time 0 (accounting for both stream `start_time` and initial B-frame decoder delay). + self._dts_offset: int = 0 + self._dts_offset_time_base: Fraction | None = None + # Ensures we only log a warning once per stream if timestamps go backwards. + self._warned_non_monotonic = False + # 1-based count of frames decoded in the current stream pass (used as the position in + # `_TimeSource.FRAMES` mode and for diagnostic logging). + self._frames_decoded = 0 + # Most recent positive timestamp step between consecutive frames (in `_timecode.time_base` + # ticks), used by `_extrapolated_timecode()` when a frame is missing a timestamp. + self._last_delta = 0 # True if `_io` was opened by this object (from a path) and must be closed in `close()`. # Set before anything below can raise so a partially constructed object is safe to finalize. self._owns_io = False @@ -128,6 +182,7 @@ def __init__( raise ValueError( f"Invalid threading mode! Must be one of: {VALID_THREAD_MODES}" ) from None + self._threading_mode = threading_mode if not suppress_output: logger.debug("Restoring default ffmpeg log callbacks.") @@ -144,11 +199,11 @@ def __init__( else: self._io = path_or_io - self._container: av.container.InputContainer = av.open(self._io) # type: ignore[attr-defined] - if threading_mode is not None: - self._video_stream.thread_type = threading_mode + self._container: av.container.InputContainer = self._open_container() # type: ignore[attr-defined] + if self._threading_mode is not None: + self._video_stream.thread_type = self._threading_mode self._reopened = False - logger.debug("Threading mode set: %s", threading_mode) + logger.debug("Threading mode set: %s", self._threading_mode) except OSError: raise except Exception as ex: @@ -211,6 +266,7 @@ def close(self) -> None: # retried. The last decoded frame is dropped, since `position` is undefined once closed. self._closed = True self._frame = None + self._timecode = None # # VideoStream Methods/Properties @@ -257,18 +313,17 @@ def position(self) -> FrameTimecode: This can be interpreted as presentation time stamp, thus frame 1 corresponds to the presentation time 0. Returns 0 even if `frame_number` is 1.""" - if self._frame is None or self._frame.pts is None or not self._frame.time_base: + if self._timecode is None: return self.base_timecode - timecode = Timecode(pts=self._normalized_pts(), time_base=self._frame_time_base) - return FrameTimecode(timecode=timecode, fps=self.frame_rate) + return FrameTimecode(timecode=self._timecode, fps=self.frame_rate) @property def position_ms(self) -> float: """Current position within stream as a float of the presentation time in milliseconds. The first frame has a PTS of 0.""" - if self._frame is None or self._frame.pts is None or not self._frame.time_base: + if self._timecode is None: return 0.0 - return float(self._normalized_pts() * self._frame_time_base) * 1000.0 + return float(self._timecode.seconds) * 1000.0 @property def frame_number(self) -> int: @@ -276,10 +331,9 @@ def frame_number(self) -> int: Will return 0 until the first frame is `read`. For VFR video this is an approximation derived from PTS * framerate; use `position` for accurate PTS-based timing.""" - if self._frame is None or self._frame.pts is None or not self._frame.time_base: + if self._timecode is None: return 0 - seconds = float(self._normalized_pts() * self._frame_time_base) - return round(seconds * float(self.frame_rate)) + 1 + return round(float(self._timecode.seconds) * float(self.frame_rate)) + 1 @property def rate(self) -> Fraction | None: @@ -316,13 +370,15 @@ def seek(self, target: TimecodeLike) -> None: seek(0) followed by read(). If we want to seek to the 5th frame, we call seek(4) followed by read(), at which point frame_number will be 5. - May not be supported on all input codecs (see `is_seekable`). + May not be supported on all input codecs (see `is_seekable`). Elementary streams usually + cannot be seeked, and may raise `SeekError`. Arguments: target: Target position in video stream to seek to. If float, interpreted as time in seconds. If int, interpreted as frame number. Raises: + SeekError: Stream does not support seeking, or target cannot be seeked to. ValueError: `target` is not a valid value (i.e. it is negative), or the stream is closed. """ @@ -331,22 +387,60 @@ def seek(self, target: TimecodeLike) -> None: target = FrameTimecode(target, self.frame_rate) if target < 0: raise ValueError("Target cannot be negative!") + if not self.is_seekable: + raise SeekError("Stream is not seekable.") beginning = target == 0 target = self.base_timecode + target if target >= 1: target = target - 1 + # If `seek()` is called before any frame has been read, decode the first frame at the + # start of the stream before seeking away. This initializes `_time_source` and, in + # `_TimeSource.DTS` mode, records `_dts_offset` from frame 1 (which cannot be determined + # after seeking into the middle of the stream). If the seek subsequently fails, reset + # back to the initial unread state. + probed_first_frame = self._time_source is None and not beginning + if probed_first_frame: + self.read(decode=False) + if self._time_source == _TimeSource.FRAMES and not beginning: + if probed_first_frame: + with contextlib.suppress(Exception): + self.reset() + raise SeekError("Unable to seek in stream without timestamps.") + stream_time_base = self._video_stream.time_base + if not stream_time_base: + if probed_first_frame: + with contextlib.suppress(Exception): + self.reset() + raise SeekError("Unable to seek in stream: stream time_base is unset.") target_pts = (self._video_stream.start_time or 0) + int( - (self.base_timecode + target).seconds / _to_fraction(self._video_stream.time_base) + (self.base_timecode + target).seconds / _to_fraction(stream_time_base) ) + # TODO(https://scenedetect.com/issues/590): For some containers (e.g. MPEG-TS), we can end + # up *after* the `target_pts`. For example, seeking to frame 100 may have us end up on 151. + # This means the loop below never gets to walk forward to the exact frame. We need to seek + # to an earlier keyframe and then read forward to fix this. + try: + self._container.seek(target_pts, stream=self._video_stream) + except av.error.FFmpegError as ex: # type: ignore[attr-defined] + if probed_first_frame: + with contextlib.suppress(Exception): + self.reset() + raise SeekError(f"Unable to seek in stream: {ex}") from ex self._frame = None self._decoder = None - self._container.seek(target_pts, stream=self._video_stream) + self._timecode = None + self._last_delta = 0 + # Seed `_frames_decoded` with the target frame before walking forward from the keyframe so + # `_extrapolated_timecode()` has a fallback if the first post-seek frame lacks a timestamp, + # then sync it to `self.frame_number` once the walk completes. + self._frames_decoded = 0 if beginning else max(target.frame_num, 0) if not beginning: self.read(decode=False) while self.position < target: if self.read(decode=False) is False: break + self._frames_decoded = self.frame_number def reset(self): """Close and re-open the VideoStream (should be equivalent to calling `seek(0)`).""" @@ -354,8 +448,15 @@ def reset(self): self._container.close() self._frame = None self._decoder = None + self._timecode = None + self._frames_decoded = 0 + self._last_delta = 0 + self._warned_non_monotonic = False try: - self._container = av.open(self._path if self._path else self._io) + self._container = self._open_container(rewind=True) + if self._threading_mode is not None: + self._video_stream.thread_type = self._threading_mode + self._reopened = False except Exception as ex: raise VideoOpenFailure() from ex @@ -363,16 +464,16 @@ def read(self, decode: bool = True) -> np.ndarray | bool: self._check_open() consecutive_failures = 0 while True: - # Reuse a persistent decoder generator so the codec's internal frame buffer (used for - # B-frame reordering) is never flushed prematurely. Creating a new generator each call - # caused the last buffered frame to be lost at EOF. + # Reuse the same decode generator for the lifetime of the container. Creating a new + # generator on each call would flush the codec's internal buffer at EOF and drop the + # last buffered frame. if self._decoder is None: self._decoder = self._container.decode(video=0) try: last_frame = self._frame assert self._decoder is not None self._frame = next(self._decoder) - # NOTE: EOFError subclasses FFmpegError, so this clause must come first. + # EOFError subclasses FFmpegError, so this clause must come first. except av.error.EOFError: # type: ignore[attr-defined] self._frame = last_frame if self._handle_eof(): @@ -381,12 +482,12 @@ def read(self, decode: bool = True) -> np.ndarray | bool: except StopIteration: return False except av.error.FFmpegError as ex: # type: ignore[attr-defined] - # `next()` raised before assignment, so `self._frame` is still the last good - # frame and position/frame_number are unaffected by the skipped packet. + # `next()` raised before assignment, so `self._frame` and `self._timecode` still + # reflect the last valid frame. self._decode_failures += 1 consecutive_failures += 1 - # The decoder generator is closed once an exception propagates through it; - # recreating it (next loop iteration) resumes demuxing after the bad packet. + # A generator cannot be resumed once an exception propagates out of it; recreating + # it on the next iteration continues demuxing after the corrupt packet. self._decoder = None if consecutive_failures >= MAX_CONSECUTIVE_DECODE_FAILURES: logger.error( @@ -401,6 +502,8 @@ def read(self, decode: bool = True) -> np.ndarray | bool: logger.warning("Failed to decode some frames, results may be inaccurate.") continue assert self._frame is not None + self._frames_decoded += 1 + self._update_timecode() return self._frame.to_ndarray(format="bgr24") if decode else True # @@ -423,17 +526,104 @@ def _frame_time_base(self) -> Fraction: assert self._frame is not None and self._frame.time_base return _to_fraction(self._frame.time_base) - def _normalized_pts(self) -> int: - """PTS of the current frame relative to the start of the stream. Some files have a - nonzero stream start_time (e.g. from edit lists); other backends report the first - frame's presentation time as 0, so we must do the same.""" - assert self._frame is not None and self._frame.pts is not None - start_time = self._video_stream.start_time or 0 - if start_time and self._video_stream.time_base != self._frame.time_base: - start_time = int( - start_time * _to_fraction(self._video_stream.time_base) / self._frame_time_base + def _open_container(self, rewind: bool = False) -> av.container.InputContainer: # type: ignore[attr-defined] + """Open `_io` as a PyAV container with `_CONTAINER_OPTIONS`. + + All container opens (initial open, `reset()`, and `_handle_eof()`) must go through this + helper so container flags like `-genpts` stay applied after re-opening. + + Arguments: + rewind: Seek `_io` back to byte 0 before opening (used when re-opening a stream). + """ + if rewind and self._io.seekable(): + self._io.seek(0) + return av.open(self._io, container_options=_CONTAINER_OPTIONS) # type: ignore[attr-defined] + + def _update_timecode(self): + """Update `_timecode` with the presentation time of `_frame` relative to the first frame. + + On the first decoded frame, selects the stream's `_TimeSource`: + * `_TimeSource.PTS`: `frame.pts` relative to `_video_stream.start_time`. + * `_TimeSource.DTS`: `frame.dts` relative to the first frame's DTS (`_dts_offset`). + * `_TimeSource.FRAMES`: `(frames_decoded - 1) / frame_rate`. + + Once selected, the source is never mixed across frames. If a subsequent frame in `PTS` or + `DTS` mode is missing its timestamp (e.g. a flushed frame at EOF), its position is + extrapolated from the previous frame via `_extrapolated_timecode()`. + """ + frame = self._frame + assert frame is not None + if self._time_source is None: + if frame.pts is not None and frame.time_base: + self._time_source = _TimeSource.PTS + elif frame.dts is not None and frame.time_base: + # When a stream has B-frames, the decoder buffers several packets before outputting + # the first frame, so the first output frame's DTS is already past the stream's + # start_time by that reordering delay. Recording the first frame's DTS as the + # offset normalizes frame 1 to time 0. + self._time_source = _TimeSource.DTS + self._dts_offset, self._dts_offset_time_base = frame.dts, self._frame_time_base + logger.debug( + "Stream has no PTS, using DTS with offset %d (time base %s).", + self._dts_offset, + self._dts_offset_time_base, + ) + else: + self._time_source = _TimeSource.FRAMES + logger.debug("Stream has no timestamps, positions are derived from frame count.") + + if self._time_source == _TimeSource.FRAMES: + self._timecode = Timecode( + pts=max(self._frames_decoded - 1, 0), time_base=1 / self.frame_rate + ) + return + if self._time_source == _TimeSource.PTS: + timestamp = frame.pts + offset = self._video_stream.start_time or 0 + offset_time_base = ( + _to_fraction(self._video_stream.time_base) if self._video_stream.time_base else None ) - return self._frame.pts - start_time + else: + timestamp = frame.dts + offset, offset_time_base = self._dts_offset, self._dts_offset_time_base + if timestamp is None or not frame.time_base: + # Frames flushed from the decoder at EOF (or occasional packets without timestamps) + # may have `timestamp=None`. Extrapolate from the previous frame so positions stay + # monotonic instead of resetting to 0 or stalling. + self._timecode = self._extrapolated_timecode() + return + time_base = self._frame_time_base + if offset and offset_time_base is not None and offset_time_base != time_base: + offset = int(offset * offset_time_base / time_base) + timecode = Timecode(pts=timestamp - offset, time_base=time_base) + previous = self._timecode + if previous is not None and previous.time_base == timecode.time_base: + if timecode.pts > previous.pts: + self._last_delta = timecode.pts - previous.pts + elif timecode.pts < previous.pts and not self._warned_non_monotonic: + # With `-genpts` set on the container, non-monotonic timestamps indicate either a + # genuinely malformed input or unexpected demuxer/decoder behavior. + self._warned_non_monotonic = True + logger.warning( + "Timestamps went backwards at frame %d (%.3fs -> %.3fs, PyAV %s); results " + "may be inaccurate. See https://scenedetect.com/issues/590", + self._frames_decoded, + float(previous.seconds), + float(timecode.seconds), + av.__version__, + ) + self._timecode = timecode + + def _extrapolated_timecode(self) -> Timecode: + """Advance `_timecode` by one frame using `_last_delta`, or the nominal frame period + `1 / frame_rate` if no inter-frame delta has been observed yet.""" + previous = self._timecode + if previous is None: + return Timecode(pts=max(self._frames_decoded - 1, 0), time_base=1 / self.frame_rate) + delta = self._last_delta + if delta <= 0: + delta = max(round(1 / (self.frame_rate * previous.time_base)), 1) + return Timecode(pts=previous.pts + delta, time_base=previous.time_base) def _get_duration(self) -> int: """Get video duration as number of frames based on the video and set framerate.""" @@ -441,6 +631,11 @@ def _get_duration(self) -> int: # handle time calculations internally and which time base to use. assert self.frame_rate is not None, "Frame rate must be set before calling _get_duration!" # See if we can obtain the number of frames directly from the stream itself. + # TODO(https://scenedetect.com/issues/590): Some AVI files remuxed by ffmpeg set the stream + # rate to a high tick rate (e.g. 600/1), causing `stream.frames` to report tick count + # rather than actual frame count (e.g. 12012 instead of 480). OpenCV reports the same value + # via CAP_PROP_FRAME_COUNT, but `duration_sec * frame_rate` would be more accurate when + # `stream.frames` disagrees with the stream duration and codec frame rate. if self._video_stream.frames > 0: return self._video_stream.frames # Calculate based on the reported container duration. @@ -473,18 +668,31 @@ def _handle_eof(self): return False self._reopened = True # Don't re-open the video if we can't seek or aren't in AUTO/FRAME thread_type mode. - if not self.is_seekable or self._video_stream.thread_type not in ("AUTO", "FRAME"): + thread_type = getattr( + self._video_stream.thread_type, "name", self._video_stream.thread_type + ) + if ( + not self.is_seekable + or self._time_source == _TimeSource.FRAMES + or thread_type not in ("AUTO", "FRAME") + ): return False - last_pos_secs = self.position.seconds + last_pos = self.position if self._timecode is not None else None orig_pos = self._io.tell() try: - self._io.seek(0) - container = av.open(self._io) + container = self._open_container(rewind=True) except: self._io.seek(orig_pos) raise self._container.close() self._container = container self._decoder = None - self.seek(last_pos_secs) + if last_pos is not None: + try: + # `seek(target)` positions the stream one frame before `target` so the next `read()` + # returns `target`. Pass `last_pos + 1` so the reopened stream lands on `last_pos` + # and the caller's subsequent `read()` advances to the next frame. + self.seek(last_pos + 1) + except SeekError: + return False return True diff --git a/tests/conftest.py b/tests/conftest.py index dee805a4..b14102e0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -130,6 +130,20 @@ def test_vfr_drop3_video() -> str: return check_exists("tests/resources/goldeneye-vfr-drop3.mp4") +@pytest.fixture +def test_no_pts_avi() -> str: + """First 480 frames of goldeneye.mp4 re-encoded with B-frames (libx264, -bf 3 -g 48) and + remuxed into an AVI, so its packets only carry decode timestamps (no PTS). See issue #590.""" + return check_exists("tests/resources/issue-590-bframes.avi") + + +@pytest.fixture +def test_raw_h264() -> str: + """Same bitstream as `test_no_pts_avi` as a raw Annex-B H.264 elementary stream, which carries + no timestamps at all. See issue #590.""" + return check_exists("tests/resources/issue-590-bframes.h264") + + @pytest.fixture def corrupt_video_file() -> str: """Video containing a corrupted frame causing a decode failure.""" diff --git a/tests/test_backend_pyav.py b/tests/test_backend_pyav.py index b9d910e3..8e44cc55 100644 --- a/tests/test_backend_pyav.py +++ b/tests/test_backend_pyav.py @@ -24,7 +24,15 @@ import av import pytest -from scenedetect.backends.pyav import MAX_CONSECUTIVE_DECODE_FAILURES, VideoStreamAv, _to_fraction +from scenedetect import SceneManager +from scenedetect.backends.pyav import ( + MAX_CONSECUTIVE_DECODE_FAILURES, + VideoStreamAv, + _TimeSource, + _to_fraction, +) +from scenedetect.detectors import ContentDetector +from scenedetect.video_stream import SeekError def test_video_stream_pyav_bytesio(test_video_file: str): @@ -56,12 +64,20 @@ def test_video_stream_pyav_del_closes_owned_file_handle(test_video_file: str): def test_close_leaves_caller_file_open(test_video_file: str, auto_close): - """`close()` must not close a file-like object supplied by the caller (#568).""" + """`close()` must not close a file-like object supplied by the caller (#568), and must drop + the last decoded frame and position.""" with open(test_video_file, mode="rb") as video_file: stream = auto_close(VideoStreamAv(path_or_io=video_file, threading_mode=None)) - assert stream.read() is not False + assert stream.read(decode=False) is not False + assert stream.read(decode=False) is not False + assert stream.frame_number == 2 + assert stream.position > stream.base_timecode + assert stream.position_ms > 0.0 stream.close() assert not video_file.closed + assert stream.frame_number == 0 + assert stream.position == stream.base_timecode + assert stream.position_ms == 0.0 def test_close_releases_owned_file_handle( @@ -250,3 +266,201 @@ def test_duration_fallback_unset_time_base( monkeypatch.setattr(VideoStreamAv, "_video_stream", property(lambda self: fake_stream)) assert stream._get_duration() == 0 assert any("time_base" in record.message for record in caplog.records) + with pytest.raises(SeekError): + stream.seek(10) + assert stream.frame_number == 0 + + +# +# Streams without presentation timestamps +# +# By default, PyAV enables AVFMT_FLAG_GENPTS on every input container. For containers that only +# store decode timestamps (e.g. AVI), GENPTS synthesizes PTS from decode-order packet DTS, which +# produces out-of-order frame PTS once B-frames are reordered by the decoder. Raw elementary +# streams have no timestamps at all. Both fixtures below contain the first 480 frames of +# `goldeneye.mp4` re-encoded with B-frames, so the cuts detected on `goldeneye.mp4` over that +# range serve as the reference. +# + +_BFRAMES_NUM_FRAMES = 480 + + +def _detect_cuts(path: str, num_frames: int | None = None) -> list: + scene_manager = SceneManager() + scene_manager.add_detector(ContentDetector()) + with VideoStreamAv(path) as video: + end_time = None if num_frames is None else video.base_timecode + num_frames + scene_manager.detect_scenes(video, end_time=end_time) + return [scene[0] for scene in scene_manager.get_scene_list()[1:]] + + +def test_no_pts_avi_positions_monotonic(test_no_pts_avi: str, auto_close): + """With GENPTS disabled the AVI falls back to DTS; positions must start at 0 and increase by + one frame per read instead of following the decoder's reordering.""" + stream = auto_close(VideoStreamAv(test_no_pts_avi)) + assert stream.read() is not False + assert stream._time_source == _TimeSource.DTS + assert stream.position == stream.base_timecode + assert stream.position_ms == pytest.approx(0.0) + assert stream.frame_number == 1 + previous = stream.position + for i in range(2, 100): + assert stream.read() is not False + assert stream.position > previous + assert stream.frame_number == i + previous = stream.position + + +def test_no_pts_avi_matches_mp4(test_no_pts_avi: str, test_movie_clip: str): + """Cuts detected on the AVI must land on the same frames as on the mp4 it was encoded from. + Before the fix the AVI cuts were one frame late and frame numbers were permuted.""" + expected = _detect_cuts(test_movie_clip, _BFRAMES_NUM_FRAMES) + actual = _detect_cuts(test_no_pts_avi) + assert len(expected) > 0 + assert [cut.frame_num for cut in actual] == [cut.frame_num for cut in expected] + for cut, oracle in zip(actual, expected, strict=True): + # The AVI time base (1/600) cannot represent 24000/1001 exactly, so allow a few ms. + assert cut.seconds == pytest.approx(oracle.seconds, abs=0.005) + + +def test_no_pts_avi_seek(test_no_pts_avi: str, auto_close): + """Seeking must give the same positions whether or not a frame was read first, since the DTS + offset has to be probed separately when `seek()` is the first call.""" + read_first = auto_close(VideoStreamAv(test_no_pts_avi)) + assert read_first.read() is not False + read_first.seek(50) + seek_first = auto_close(VideoStreamAv(test_no_pts_avi)) + seek_first.seek(50) + for stream in (read_first, seek_first): + assert stream.frame_number == 50 + assert stream._frames_decoded == 50 + frame_ms = 1000.0 / float(stream.frame_rate) + assert stream.position_ms == pytest.approx(49 * frame_ms, abs=frame_ms / 2) + assert stream.read() is not False + assert stream.frame_number == 51 + assert stream._frames_decoded == 51 + assert read_first.position == seek_first.position + + +def test_no_pts_avi_reopen(test_no_pts_avi: str, auto_close): + """Re-opening the container (`reset()`, and the AUTO threading EOF re-open) must keep GENPTS + disabled, otherwise positions go out of order again partway through the video.""" + stream = auto_close(VideoStreamAv(test_no_pts_avi, threading_mode="AUTO")) + for _ in range(10): + assert stream.read() is not False + stream.reset() + assert stream.frame_number == 0 + for _ in range(10): + assert stream.read() is not False + assert stream.frame_number == 10 + previous, frames = stream.position, stream.frame_number + assert stream._handle_eof() is True + assert stream.frame_number == 10 + assert stream.position == previous + while stream.read() is not False: + frames += 1 + assert stream.position > previous + previous = stream.position + # Not compared to `stream.duration`: this AVI declares a 600 Hz stream rate, so ffmpeg (and + # OpenCV) report its frame count in ticks. See the TODO in `VideoStreamAv._get_duration`. + assert frames == _BFRAMES_NUM_FRAMES + + +def test_raw_stream_without_timestamps_counts_frames( + test_raw_h264: str, test_movie_clip: str, auto_close +): + """A raw elementary stream has neither PTS nor DTS. Positions are then derived from the number + of decoded frames (like the OpenCV backend) instead of every frame reporting time 0, and + failed seeks raise `SeekError` without corrupting the current stream state.""" + stream = auto_close(VideoStreamAv(test_raw_h264)) + with pytest.raises(SeekError): + stream.seek(50) + assert stream.frame_number == 0 + for i in range(1, 11): + assert stream.read() is not False + assert stream.frame_number == i + assert stream.position == stream.base_timecode + (i - 1) + assert stream._time_source == _TimeSource.FRAMES + for target in (50, 0): + with pytest.raises(SeekError): + stream.seek(target) + assert stream.frame_number == 10 + assert stream.position == stream.base_timecode + 9 + assert stream.read() is not False + assert stream.frame_number == 11 + expected = _detect_cuts(test_movie_clip, _BFRAMES_NUM_FRAMES) + actual = _detect_cuts(test_raw_h264) + assert [cut.frame_num for cut in actual] == [cut.frame_num for cut in expected] + + +# +# Timestamp source and monotonicity +# + + +def test_time_source_does_not_mix_pts_and_dts( + test_video_file: str, monkeypatch: pytest.MonkeyPatch, caplog, auto_close +): + """Once the first frame selects PTS or DTS mode, subsequent frames must read only from that + source (extrapolating when None) rather than mixing PTS and DTS offsets.""" + stream = auto_close(VideoStreamAv(test_video_file)) + fake_stream = _AttrOverridingProxy(stream._video_stream, start_time=0) + monkeypatch.setattr(VideoStreamAv, "_video_stream", property(lambda self: fake_stream)) + time_base = Fraction(1, 24) + + # DTS mode: sporadic PTS values must be ignored so the DTS reorder offset is not subtracted + # from a PTS value. + dts_positions = [] + for pts, dts in ((None, 2), (1, 3), (None, 4), (3, 5)): + stream._frame = SimpleNamespace(pts=pts, dts=dts, time_base=time_base) + stream._frames_decoded += 1 + stream._update_timecode() + dts_positions.append(stream._timecode.pts) + assert stream._time_source == _TimeSource.DTS + assert dts_positions == [0, 1, 2, 3] + + # PTS mode: a frame missing PTS must extrapolate rather than falling back to DTS (which leads + # PTS by the reorder delay). + stream._time_source = None + stream._timecode = None + stream._frames_decoded = 0 + stream._last_delta = 0 + pts_positions = [] + for pts, dts in ((0, -2), (1, -1), (None, 2), (3, 1)): + stream._frame = SimpleNamespace(pts=pts, dts=dts, time_base=time_base) + stream._frames_decoded += 1 + stream._update_timecode() + assert stream._timecode is not None + pts_positions.append(stream._timecode.pts) + assert stream._time_source == _TimeSource.PTS + assert pts_positions == [0, 1, 2, 3] + assert not [record for record in caplog.records if "backwards" in record.message] + + # An unset time_base on the first frame degrades to FRAMES mode instead of asserting. + stream._time_source = None + stream._timecode = None + stream._frames_decoded = 1 + stream._frame = SimpleNamespace(pts=None, dts=2, time_base=None) + stream._update_timecode() + assert stream._time_source == _TimeSource.FRAMES + assert stream.frame_number == 1 + + +def test_non_monotonic_timestamps_warn_once( + test_video_file: str, monkeypatch: pytest.MonkeyPatch, caplog, auto_close +): + """A timestamp going backwards is reported once per stream, and only when it actually goes + backwards: repeated timestamps (as in VFR video) stay silent.""" + stream = auto_close(VideoStreamAv(test_video_file)) + fake_stream = _AttrOverridingProxy(stream._video_stream, start_time=0) + monkeypatch.setattr(VideoStreamAv, "_video_stream", property(lambda self: fake_stream)) + time_base = Fraction(1, 1000) + for pts in (0, 40, 40, 0, 80, 20): + stream._frame = SimpleNamespace(pts=pts, dts=None, time_base=time_base) + stream._frames_decoded += 1 + stream._update_timecode() + warnings = [record for record in caplog.records if "backwards" in record.message] + assert len(warnings) == 1 + assert "frame 4" in warnings[0].message + # Positions keep following the input; the warning does not alter them. + assert stream.position_ms == pytest.approx(20.0) diff --git a/website/pages/changelog.md b/website/pages/changelog.md index 0a336e81..9fc7c45f 100644 --- a/website/pages/changelog.md +++ b/website/pages/changelog.md @@ -825,4 +825,8 @@ Development - [feature] `VideoStream` has a new `close()` method to release the file handles and decoders a video holds, and can be used as a context manager (`with open_video(path) as video:`) to close it automatically. Calling `close()` more than once has no effect. Once an OpenCV, PyAV, MoviePy or concatenated stream is closed, `read()`, `seek()` and `reset()` raise `ValueError`, while properties describing the video (`path`, `name`, `frame_rate`, `duration`, `frame_size`, `aspect_ratio`) remain available. The default implementation does nothing, so existing `VideoStream` subclasses are unaffected, and `VideoCaptureAdapter` leaves the caller's `VideoCapture` open [#568](https://github.com/Breakthrough/PySceneDetect/issues/568) - [bugfix] `VideoStreamConcat` no longer keeps its input videos open when they can't be concatenated (e.g. mismatched resolutions), and now closes each input explicitly once it is no longer needed instead of relying on garbage collection [#568](https://github.com/Breakthrough/PySceneDetect/issues/568) - [bugfix] `split-video` with `-o` now logs the output folder instead of repeating the filename template [#595](https://github.com/Breakthrough/PySceneDetect/pull/595) - - [bugfix] Allow `-d`/`--downscale` on the CLI and `downscale` in the config file to be set to `0` for automatic downscaling \ No newline at end of file + - [bugfix] Allow `-d`/`--downscale` on the CLI and `downscale` in the config file to be set to `0` for automatic downscaling + - [bugfix] Fix `pyav` backend timing for videos that lack presentation timestamps (e.g. AVI files, elementary H.264 streams), which could lead to frame positions being out of order or stuck at zero and shifting cuts by one or more frames ([#590](https://github.com/Breakthrough/PySceneDetect/issues/590), [PyAV#933](https://github.com/PyAV-Org/PyAV/discussions/933), [PyAV#2478](https://github.com/PyAV-Org/PyAV/pull/2478)) + - Videos are now opened with FFmpeg's `genpts` flag disabled, falling back to decode timestamps or the decoded frame count. The `pyav` backend matches the current behaviour of `opencv` and `moviepy` in this regard. + - A one-time warning is emitted if timestamps are still detected going backwards. + - `VideoStreamAv.seek()` no longer probes frames on streams that do not support seeking. Previously, `SeekError` was raised *after* consuming a frame if the stream didn't support seeking.