GNOME Bugzilla – Bug 467667
GST_FRAMES_TO_CLOCK_TIME() and GST_CLOCK_TIME_TO_FRAMES() should use GstClockTime, not gdouble
Last modified: 2007-08-17 15:41:19 UTC
Hi, currently GST_FRAMES_TO_CLOCK_TIME() and GST_CLOCK_TIME_TO_FRAMES() use double as computation data type. This will lead to rounding errors with large timestamps and all current uses (from a fast look) use the result as a GstClockTime or guint64 anyway. Also we currently have many, many places where gst_util_uint64_scale* is used for converting frames to clock time and the other way around which probably just don't use these macros because of the above reason. Would be nice if we could change this to calling gst_util_uint64_scale instead in 0.11.
Why is this 0.11 material? Why not just fix it to use gst_util_*()?
Right, the double is casted into a GstClockTime. I had something different in memory... so that's actually something we can change right now already. Patch will come in a few minutes, thanks :)
Created attachment 93848 [details] [review] clocktime-frames.diff What do you think?
Doesn't look usable in 0.10. I'd defer this to 0.11. The framerate here is passed as a single parameter. In 0.10, we use fractional framerates, so we need to pass a numerator and denominator. I'd mark it as deprecated API in 0.10 and leave it as-is. Adding differently-named macros that work appropriately would be nice too.
For audio we use single values for rates, not fractional numbers so that's fine. For video it's another story.
Oops. Sorry; I thought this was for video. Confusing macro names! I'm fine with the patch then.
Looks ok to me too. I think you might even be able to use gst_util_uint64_scale_int() in both cases. I'd keep the (GstClockTime) cast of the return value for the first case (I realise it's typedef'ed to guint64, but I'm not sure if c++ compilers might still complain about it. It's clearer in any case IMHO).
2007-08-17 Sebastian Dröge <slomo@circular-chaos.org> * gst-libs/gst/audio/audio.h: Use gst_util_uint64_scale() instead of doing the math with double for GST_FRAMES_TO_CLOCK_TIME() and GST_CLOCK_TIME_TO_FRAMES(). For large timestamps this prevents rounding errors. Fixes #467667. I only changed the first macro to cast to GstClockTime as before.