Rename timestamps (and related operations) to container_timestamps #7
Conversation
examples/flexible_video_access.py
Outdated
| assert frame.time == ts and frame.index == 10 | ||
|
|
||
| frames = video.by_timestamp[ts : ts + 10] | ||
| frames = video.by_video_time[ts : ts + 10] |
There was a problem hiding this comment.
by_video_time or by_container_timestamps?
src/pupil_labs/video/reader.py
Outdated
|
|
||
| Timestamps = NPTimestamps | list[int | float] | ||
| "Timestamps for frames in video time seconds" | ||
| PTSArray = npt.NDArray[np.float64] |
There was a problem hiding this comment.
I would expect PTSArray to be integer PTS in stream time_base, ie. what frame.pts returns, it's confusing otherwise.
src/pupil_labs/video/writer.py
Outdated
| # TODO: why does mypy fail the above check? | ||
| if self.fps is None: | ||
| stream = self.container.add_stream("h264_nvenc") # type: ignore | ||
| # TODO: why does mypy fail the above check? |
There was a problem hiding this comment.
mypy fails because av hardcodes the h264 case to return a VideoStream but returns Stream otherwise
I would refactor this stream = self.container.add_stream(stream_name, rate=self.fps)
src/pupil_labs/video/writer.py
Outdated
|
|
||
| stream.codec_context.time_base = Fraction(1, 90000) | ||
| # stream.codec_context.time_base = Fraction(1, 90000) |
There was a problem hiding this comment.
This should be set if the frame rate is not constant to maximize precision of the variable frame rate frames.
src/pupil_labs/video/reader.py
Outdated
| Large slices are returned as a lazy view, which avoids immediately loading all | ||
| frames into RAM. | ||
| Note that numerical imprecisions of float numbers can lead to issues when | ||
| accessing individual frames by their container timestamp. I is recommended to |
src/pupil_labs/video/reader.py
Outdated
| stream: Literal["video"] = "video", | ||
| pre_loaded_container_timestamps: Optional[PTSArray | list[float]] | None = None, |
There was a problem hiding this comment.
Do we really need the pre_loaded here?
src/pupil_labs/video/reader.py
Outdated
| to specify which stream to use, e.g. `("audio", 2)` to use the audio | ||
| stream at index `2`. | ||
| pre_loaded_container_timestamps: Array containing the PTS of the video | ||
| seconds. If not provided, PTS will be read from the video container. |
There was a problem hiding this comment.
PTS to me and anyone that works with pyav imples integer values in stream time_base, not float video seconds, it's confusing.
There was a problem hiding this comment.
I updated the description. Do you think this is better?
marc-tonsen
force-pushed
the
no_abs_ts
branch
from
b50d4ad to
2fb115d
Compare
No description provided.