In the player code we currently use 0.0 for float variables. But 0.0 is usually for doubles. We should use 0.0f instead. There are also missing casts around GST_SECOND.
Created attachment 55549 [details] proposed patch
Comment on attachment 55549 [details] proposed patch Please don't use static_cast<float>. narrowPrecisionToFloat is better.
As discussed on IRC with krit, if we were to use narrowPrecisionToFloat() we'd need to add templates in FloatConversion.h for the glib types. It doesn't seem to make much sense as that function only performs a static_cast<float> underneath. It just complicates code readability in my opinion ;)
Comment on attachment 55549 [details] proposed patch Asking for another review as per previous comment.
I'm not sure why narrowPrecisionToFloat exists.
Comment on attachment 55549 [details] proposed patch Just a couple of comments: >- width = gst_util_uint64_scale_int(originalHeight, displayWidth, displayHeight); >+ width = static_cast<int>(gst_util_uint64_scale_int(originalHeight, displayWidth, displayHeight)); > height = originalHeight; There's precision loss in this kind of casting. >- float currentPosition = playbackPosition(m_playBin) * GST_SECOND; >+ float currentPosition = playbackPosition(m_playBin) * static_cast<float>(GST_SECOND); This seems strange to me; for one I'd assume the casting would happen automatically, but if it does not, it seems more reasonable to cast the whole operation instead of just one operand (which also happens to be a constant).
Created attachment 57263 [details] proposed patch
Comment on attachment 57263 [details] proposed patch New version tackling the issues pointed out by Xan
Comment on attachment 57263 [details] proposed patch - GstClockTime sec = (GstClockTime)(time * GST_SECOND); + GstClockTime sec = (GstClockTime)(time * static_cast<float>(GST_SECOND)); You forgot this, please fix when landing.
Landed in http://trac.webkit.org/changeset/60442 Thanks for the review :)