use position_ms to determine timestamps

[goatzillax]

Description of Problem & Solution Hi, smee again!

I have some pretty unclean video files due to network/terrible hardware issues which have a lot, lot, lot of dropped frames, which totally wreaks havoc on timestamps and the "copy" output. Preprocessing the files through ffmpeg was unsavory as well as... didn't totally fix the situation.

Opencv appears to have position_ms which seems to fix this. As a test I plumbed in a position_ms to VideoJoiner to pluck it from the VideoStreamCv2 member, and that seems to work. It produces correct timestamps that ffmpeg is able to correctly use to copy clips with.

TBH I don't know how this would interact with the other options, but for my single-file case it seems to work so far.

[Breakthrough]

Interesting, this is very useful to know. We should definitely look into this some more.

I can certainly add a config file option to switch what to use for internal timestamp tracking (so users can select either property), but it sounds like timestamp_ms might be more reliable for most users.

Thanks for letting me know, feel free to submit any PRs as well if you'd like.

[Breakthrough]

The following snippet shows a potential definition for a new timecode type that would support VFR videos as well:

class Timestamp:
    def __init__(self):
        self._frame_index: int = 0                    # Index of this frame
        self._time_ms: ty.Union[int, float] = 0       # Timestamp of this frame as int or float, milliseconds
        # TODO: Do we need the following? (e.g. we shouldn't allow addition by X frames if the video is VFR, but we can compare frames, and we can add by seconds)
        self._framerate: float = 0.0                  # Fixed or average for VFR
        self._vfr: bool = False                       # If True, video is VFR, and _framerate is average

    @property
    def frames(self) -> int:
        return self._frame_index

    @property
    def seconds(self) -> float:
        return self._time_ms / 1000.0

    @property
    def timecode(self) -> str:
        pass  # TODO

This should solve both the issue of dropped frames as well as the ability to handle VFR videos. This will likely involve a lot of changes to the pipeline to integrate cleanly, but it should be feasible, and solves a lot of other issues around time handling throughout the codebase. Ideally we can replace most of the conditional statements around the timecode handling (using frame numbers vs. position_ms) with a dedicated type like the above.

Thanks again for your PR which helped start this work off.

[goatzillax]

I felt kind of bad smearing all those conditionals all over the code but figured a full changeover would be pretty structural. So I'm all for the Timestamp.

Framenum doesn't seem that useful in some other detection code I've been working on (with neural net results). It seems useful for picking a frame out of a list (i.e. highscore thumbnail or something), but if that's the only use, the frame list might as well be a dict() that's keyed by a timestamp. If you use the timestamps straight without doing math on them as keys, that seems to be OK.

Thanks for this software; it's put in a lot of work clipping events and making thumbnails from a ton of archival footage, and I learned quite a bit from looking through it.