+++ This bug was initially created as a clone of Bug #52697 +++ The current GStreamer MediaPlayer backend doesn't use frame accurate seeking. In addition to using the GST_SEEK_FLAG_ACCURATE flag it should support a frame step event when possible (positive playback rate, frame requested is ahead of current playback position).
Created attachment 83807 [details] proposed patch
Attachment 83807 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:552: Missing spaces around | [whitespace/operators] [3] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 83808 [details] proposed patch
Hi Sebastian, Can you have a look at this patch when you get time? I did my tests with the website linked to that bug. Thanks in advance for any feedback :)
Comment on attachment 83808 [details] proposed patch Pulling out of review, I have a new patch, simpler, without using the step event. Will upload it soon :)
Created attachment 85331 [details] proposed patch
Comment on attachment 85331 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=85331&action=review Looks good to me, though I think the time calculation logic could be cleaned up a bit to make it clearer. > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:530 > + glong microSeconds; > + float microSecondsf = modf(time, &seconds); > + microSecondsf *= 1000000; > + microSeconds = static_cast<glong>(roundf(microSecondsf / 10000) * 10000); > + timeValue.tv_sec = static_cast<glong>(seconds); > + timeValue.tv_usec = microSeconds; > + > + GstClockTime clockTime = GST_TIMEVAL_TO_TIME(timeValue); > + LOG_VERBOSE(Media, "Seek: %" GST_TIME_FORMAT, GST_TIME_ARGS(clockTime)); I think you should probably just dispense with the microseconds variable altogether and rename microsecondsf to microseconds. float seconds; float microseconds = modf(time, &seconds) * 1000000; timeValue.tv_sec = static_cast<glong>(seconds); timeValue.tv_usec = static_cast<glong>(roundf(microSecondsf / 10000) * 10000); I'm still a bit confused by this logic, because you multiply microsecondsf by 1000000 and then immediately divide by 10000.
(In reply to comment #7) > > I think you should probably just dispense with the microseconds variable altogether and rename microsecondsf to microseconds. > Agreed. > float seconds; > float microseconds = modf(time, &seconds) * 1000000; > timeValue.tv_sec = static_cast<glong>(seconds); > timeValue.tv_usec = static_cast<glong>(roundf(microSecondsf / 10000) * 10000); > > I'm still a bit confused by this logic, because you multiply microsecondsf by 1000000 and then immediately divide by 10000. An example of time is 2.155999 seconds. modf() will return 0.155999 seconds. So to get the microseconds value we multiply by 1000000. The division and multiplication are used to round. If microseconds is something like 155999: roundf(15.5999) * 10000 = 160000 microseconds I'll have another look at this tomorrow morning, thanks for the review anyway :)
Created attachment 85789 [details] proposed patch
Comment on attachment 85789 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=85789&action=review Looks good to me! > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:524 > + GTimeVal timeValue; > + float seconds; > + float microSeconds = modf(time, &seconds) * 1000000; > + timeValue.tv_sec = static_cast<glong>(seconds); > + timeValue.tv_usec = static_cast<glong>(roundf(microSeconds / 10000) * 10000); Super nit here, but do you mind moving GTimeVal timeValue right down to where you first use it?
Committed r81228: <http://trac.webkit.org/changeset/81228>