After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 467667 - GST_FRAMES_TO_CLOCK_TIME() and GST_CLOCK_TIME_TO_FRAMES() should use GstClockTime, not gdouble
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal trivial
: 0.10.15
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2007-08-17 14:23 UTC by Sebastian Dröge (slomo)
Modified: 2007-08-17 15:41 UTC
See Also:
GNOME target: ---
GNOME version: ---

clocktime-frames.diff (947 bytes, patch)
2007-08-17 14:48 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sebastian Dröge (slomo) 2007-08-17 14:23:16 UTC
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.
Comment 1 Tim-Philipp Müller 2007-08-17 14:41:21 UTC
Why is this 0.11 material? Why not just fix it to use gst_util_*()?
Comment 2 Sebastian Dröge (slomo) 2007-08-17 14:45:22 UTC
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 :)
Comment 3 Sebastian Dröge (slomo) 2007-08-17 14:48:15 UTC
Created attachment 93848 [details] [review]

What do you think?
Comment 4 Michael Smith 2007-08-17 14:57:13 UTC
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.

Comment 5 Sebastian Dröge (slomo) 2007-08-17 15:01:03 UTC
For audio we use single values for rates, not fractional numbers so that's fine. For video it's another story.
Comment 6 Michael Smith 2007-08-17 15:05:43 UTC
Oops. Sorry; I thought this was for video. Confusing macro names! I'm fine with the patch then. 
Comment 7 Tim-Philipp Müller 2007-08-17 15:09:07 UTC
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).
Comment 8 Sebastian Dröge (slomo) 2007-08-17 15:22:48 UTC
2007-08-17  Sebastian Dröge  <>

	* 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.