[Breakthrough]

PyAV sets AVFMT_FLAG_GENPTS on every input (see PyAV-Org/PyAV#933, PyAV-Org/PyAV#2477, and PyAV-Org/PyAV#2478).

On containers that store DTS only (AVI, some MPEG-TS) and contain B-frames, the generated PTS comes out permuted in decode order after reordering B-frames. This currently causes the PyAV backend to report non-monotonic timestamps, resulting in shifted or missing cuts (#590). Furthermore, streams without timestamps stay stuck at time 0. Frame.best_effort_timestamp (PyAV 19.1+, Python 3.12+) does not help on its own, since with genpts on it is still wrong for the initial few frames (until FFmpeg's heuristic switches to DTS).

This PR sets fflags=-genpts when opening containers to override the +genpts flag from PyAV on all PyAV versions, and places each frame at its PTS, falling back to its DTS when the PTS is missing (best_effort_timestamp is not used on any version). This is the same per-frame rule OpenCV's FFmpeg backend uses. With genpts off, FFmpeg's best_effort_timestamp gives the same result on streams with monotonic PTS values. FFmpeg's heuristic switches to DTS once PTS has gone backwards more often than DTS - this PR does not to keep things simpler for now. We follow the PTS and warn. A frame with neither timestamp is placed one (nominal) frame time after the previous frame. A single start offset is decided from the first frame that carries a timestamp such that frame 1 is at time 0. This is the stream's start_time when frame 1 has a PTS (as before), otherwise the timestamp minus the nominal duration of the frames before it. This covers DTS-only streams, as the first DTS includes the decoder's reordering delay, as well as streams that lack timestamps at the beginning.

VideoStreamAv.seek() changes:

• Raises SeekError on streams that cannot be seeked, instead of a backend-specific exception. As documented on VideoStream.seek(), the stream position is undefined after a SeekError until reset() is called.
• Seek targets are computed relative to the same offset as reported positions (previously start_time, which differs on DTS-only streams and streams whose first frames lack timestamps).
• If the keyframe landed on has no timestamp, we keep reading until a frame with one is found, before checking it against the target. This ensures the next read() will never land before the target.
• If we can't find any frames MAX_SEEK_PROBE_FRAMES with a valid timestamp, or the first timestamp we get is past the target, we reset back to the beginning of the stream and slow seek from there. This should only happen on malformed inputs whose landing keyframe lacks timestamps.
• seek(0) re-opens the stream while the offset is still unknown, since seeking the container could land on a later keyframe and anchor the offset there.

A one-time warning is logged if positions are still observed going backwards, including when a timestamp lands before the estimated position of the frames without timestamps preceding it.

Test fixtures (on the resources branch):

• issue-590-bframes.avi / .h264: first 480 frames of goldeneye.mp4 with B-frames, as a DTS-only container and as a raw elementary stream.
• issue-590-missing-pts.ts: goldeneye-vfr-drop3.mp4 remuxed to MPEG-TS with the PTS/DTS of the first six video PES packets, and two in every seven after the tenth, replaced by PES stuffing bytes. After decoding, frames 1 through 4 have no timestamps, frame 5 only a DTS, and later frames sporadically lack PTS, DTS or both.

Follow-ups kept as TODOs:

• AVI tick rate and frame counts in _get_duration still need some work.
• MPEG-TS seeks that land after the target position now fall back to reading from the start. This is correct, but slow when the seek target is distant. Seeking to an earlier keyframe would be much faster.

[MuhammadBilalKhan267]

The policy selects a source based on the first frame in the order PTS -> DTS -> frames. This approach is mostly sound, but I noticed a few issues:

Example 1:

Assume the source time base is 1/fps and m means missing.

PTS: m 2 5 7
DTS: m m 5 7
Result: 0 1 2 3

Since both PTS and DTS are missing for the first frame, we choose frames mode. This ignores the valid timestamps on later frames and can produce positions that differ from the source timestamps.

Example 2:

Assume a fixed time base and a zero stream start offset.

PTS: 1 4 m m 7
Result: 1 4 7 10 7

Extrapolation causes the output timestamps to become non-monotonic, even though the available source PTS are monotonic.

[MuhammadBilalKhan267]

If we are going to use a new policy for handling missing timestamps, I think the same policy should be applied across all backends. The main issue here is that OpenCV's FFmpeg backend doesn't expose separate raw decoded PTS or DTS through VideoCapture, and chooses between PTS and DTS before returning the timestamp via CAP_PROP_POS_MSEC. MoviePy receives raw video pixels from FFmpeg via image2pipe. The reader then consumes fixed size pixel data and returns an image array. MoviePy's get_frame_number also uses fps to calculate the frame number at time t. Our backend uses an internal frame counter to calculate the current timestamp.

This means that, under the current backend interfaces, such a fixed-source policy can only be applied directly to PyAV. Either applied across all versions or only versions that do not expose best_effort_timestamp. The latter means different handling for different versions since our 'Fixed Source' policy is different from FFmpeg's best_effort_timestamp.

My opinion is that this change is too complex for either case, especially since the new policy has its own problems. We can use best_effort_timestamp for Python 3.12+ once a PyAV release includes it, and a PyAV backport could enable us to use it for 3.10 and 3.11 too.
I've already requested PyAV to backport best_effort_timestamp to PyAV 17 and 18. PyAV-Org/PyAV#2478

[MuhammadBilalKhan267]

I'd prefer we wait for PyAV's response on the backport request. I think this policy is worthwhile only if we can address the extrapolation issues.

[Breakthrough]

Thanks for taking the time to go through this so carefully. Both examples you gave are correct, I was able to reproduce them with real videos. Remuxing a VFR clip to MPEG TS and removing the PTS/DTS in some of the PES headers demonstrates the two examples you gave. I added a regression test (issue-590-missing-pts.ts) for this in the resources branch and updated this PR to use it. I've also greatly simplified the timestamp policy, have a look at the latest changes.

With these changes I think it addresses the extrapolation issue you pointed out. Those examples should now result in:

Example 1:  PTS m 2 5 7      ->    0 1 4 6     (source timestamps)
Example 2:  PTS 1 4 m m 7    ->    1 4 5 6 7   (monotonic)

Both are in test_missing_timestamps_policy.

On waiting for best_effort_timestamp, I don't want to rely on that. We need the -genpts fix regardless, and once genpts is off the property is just PTS else DTS. A backport would be welcome but I'd rather not wait for it.

OpenCV already applies this rule internally (re: CAP_PROP_POS_MSEC), and MoviePy has no timestamps at all (it just counts frames). The PyAV backend should now match the best each backend can do, rather than creating a new policy only it can use. Frames without a timestamp are still handled a bit differently (OpenCV reports position 0, and negative positions on the leading-gap file), but I think it's the right call to estimate from the previous frame here.

[MuhammadBilalKhan267]

Added a few notes. This policy should be fine, but I’d suggest using it across all versions rather than using best_effort_timestamp when PyAV adds it in a new release, unless it’s also backported for Python 3.10 and 3.11.

[Breakthrough]

Thanks again, agreed on all five comments and reproduced the cases you described. Pushed fixes for everything you pointed out and updated the outdated PR description.

[MuhammadBilalKhan267]

Thanks for addressing the earlier cases. I found two more seek issues.

[MuhammadBilalKhan267]

On issue-590-missing-pts.ts, seek(1) followed by read() returns the first image instead of the second. After the reset, both the position and adjusted target are 0, so the loop doesn't read the first frame. Could we read one frame after resetting before entering the loop?

[MuhammadBilalKhan267]

After reading five frames from issue-590-missing-pts.ts, seeking back to frame 0 returns frame 61 on the next read. This also happens on main.

Could we reset the stream whenever seeking to frame 0? Currently, we only reset when the offset is unknown, so the container seek can land later in the video.