servo / gecko-media

Firefox's media playback stack in a stand alone Rust crate
Mozilla Public License 2.0
6 stars 9 forks source link

Timestamp inconsistencies in the Rust Player impl #83

Open philn opened 6 years ago

philn commented 6 years ago

We use f64 values for duration and time_update but the video frame timestamps use a custom TimeStampimpl. Should we uniformize this and use TimeStamp everywhere we need to represent a timestamp or "amount of time"?

cpearce commented 6 years ago

The duration and time_update are seconds in doubles because the HTMLMediaElement spec has the time values it exposes as floats. So we're matching the target API's spec here.

If we wanted to export duration and time_update as some kind of TimeStamp, we'd be converting to a TimeStamp in gecko-media, and then converting back to a floating point value when Servo exposed them to JavaScript content.

We should certainly not use floats for time values everywhere, as we're liable to lose precision if we do; we've been burned by that in the past when we were a little greener.

Note that the TimeStamps on frames represent a wall-clock absolute time point, whereas the duration represents a range of time, and the time_update time represents an offset from 0. So they're not semantically all the same concept of time anyway. So rather than unifying into a single concept of TimeStamp, it would make more sense to create TimeDuration and TimeOffset structs for the duration and time_update values rather than making them all TimeStamps.