Bug 90734

With the current implementation 6.027200 seconds rounds to 6.030000. Kind of far fetched rounding.

Created attachment 151154 [details] Patch

Can you give an example of how this affects rounding? Before, 6.0272 was rounded to 6.03. What is it rounded to now? Does this affect tests at all?

(In reply to comment #3) > Can you give an example of how this affects rounding? Before, 6.0272 was rounded to 6.03. Yes. Example when running media/video-seek-past-end-playing.html Seek to 6.027200: 6.000000 secs 30000 usecs Seek: 0:00:06.030000000 > What is it rounded to now? Now, same test with the patch: Seek to 6.027200: 6.000000 secs 27200 usecs Seek: 0:00:06.027200000 > Does this affect tests at all? Yes, well with the patch media/video-frame-accurate-seek.html still passes. The issue with the previous rounding "strategy" is that some values were correctly rounded and some not, as you can see above. Maybe I can update the layout test!

Created attachment 153007 [details] Patch

Comment on attachment 153007 [details] Patch Attachment 153007 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13273552 New failing tests: media/video-frame-accurate-seek.html

Created attachment 153014 [details] Archive of layout-test-results from gce-cr-linux-06 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-06 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

Created attachment 153019 [details] Patch

Comment on attachment 153019 [details] Patch Attachment 153019 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13282548 New failing tests: media/video-frame-accurate-seek.html

Created attachment 153023 [details] Archive of layout-test-results from gce-cr-linux-05 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-05 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

Comment on attachment 153019 [details] Patch I need to rework/rebase this patch

I think there might be some small bugs in the GStreamer decoders, making sure currentTime() is as accurate as the seek method I can still see the frame displayed is either the one preceding or following the one requested. The video-frame-accurate-seek test actually shows the issue too, 3rd video should show frame #14 but shows frame #15 instead.

Created attachment 198003 [details] Patch

Comment on attachment 198003 [details] Patch Attachment 198003 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/47297 New failing tests: http/tests/ssl/ping-with-unsafe-redirect.html media/video-frame-accurate-seek.html

Created attachment 198030 [details] Archive of layout-test-results from webkit-ews-02 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-02 Port: mac-mountainlion Platform: Mac OS X 10.8.2

Comment on attachment 198003 [details] Patch Attachment 198003 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/139189 New failing tests: http/tests/ssl/ping-with-unsafe-redirect.html media/video-frame-accurate-seek.html

Created attachment 198037 [details] Archive of layout-test-results from webkit-ews-03 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-03 Port: mac-mountainlion Platform: Mac OS X 10.8.2

Comment on attachment 198003 [details] Patch Attachment 198003 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/30262 New failing tests: media/video-frame-accurate-seek.html

Created attachment 198040 [details] Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-12 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.2

Created attachment 200245 [details] Patch

Created attachment 238351 [details] Patch Rebased.

Comment on attachment 238351 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238351&action=review > Source/WebCore/platform/graphics/gstreamer/GStreamerUtilities.cpp:158 > + timeValue.tv_usec = static_cast<glong>(floor(microSeconds = 0.5)); oops.

Created attachment 238352 [details] Patch

Comment on attachment 238352 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=238352&action=review The code changes in this patch look fine, but I don't understand the test. > LayoutTests/media/video-frame-accurate-seek.html:8 > + <p>Test that setting currentTime is frame-accurate. Four videos below should be showing up, The first three at frames 12, 13, and 14. The last video should display at 6.0272 seconds.</p> The -> the > LayoutTests/platform/gtk/media/video-frame-accurate-seek-expected.txt:9 > + text run at (0,17) width 356: "and 14. The last video should display at 6.0272 seconds." It seems like something is wrong with this test. The expected result for the first three videos shows 12, 13, and 15, but the test says it's expected to show 12, 13, and 14. The last video displays 6.01 seconds, not 6.0272 or 6.03. > LayoutTests/platform/mac/TestExpectations:1392 > +webkit.org/b/90734 media/video-frame-accurate-seek.html [ Failure ] I think you should file new bugs for the EFL and Mac rebaselines, else they probably won't happen if you add an expectation to hide the failures.

(In reply to comment #24) > Comment on attachment 238352 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=238352&action=review > > The code changes in this patch look fine, but I don't understand the test. > > > LayoutTests/media/video-frame-accurate-seek.html:8 > > + <p>Test that setting currentTime is frame-accurate. Four videos below should be showing up, The first three at frames 12, 13, and 14. The last video should display at 6.0272 seconds.</p> > > The -> the > > > LayoutTests/platform/gtk/media/video-frame-accurate-seek-expected.txt:9 > > + text run at (0,17) width 356: "and 14. The last video should display at 6.0272 seconds." > > It seems like something is wrong with this test. The expected result for the > first three videos shows 12, 13, and 15, but the test says it's expected to > show 12, 13, and 14. The last video displays 6.01 seconds, not 6.0272 or > 6.03. > See comment 12.

(In reply to comment #24) > Comment on attachment 238352 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=238352&action=review > > The code changes in this patch look fine, but I don't understand the test. > > > LayoutTests/media/video-frame-accurate-seek.html:8 > > + <p>Test that setting currentTime is frame-accurate. Four videos below should be showing up, The first three at frames 12, 13, and 14. The last video should display at 6.0272 seconds.</p> > > The -> the > > > LayoutTests/platform/gtk/media/video-frame-accurate-seek-expected.txt:9 > > + text run at (0,17) width 356: "and 14. The last video should display at 6.0272 seconds." > > It seems like something is wrong with this test. The expected result for the > first three videos shows 12, 13, and 15, but the test says it's expected to > show 12, 13, and 14. This now works as expected \o/ > The last video displays 6.01 seconds, not 6.0272 or > 6.03. > That's because 6.01 means (6 * 25) + 1 frames = 151. And not 0.01 seconds. So we do a seek to 6.0272 seconds which means 6 seconds and one half-frame (0.02 = 1/50) but the video is at 25fps so the demuxer choses instead to jump to the next full frame, if I understand correctly.

Created attachment 282241 [details] patch

Committed r202558: <http://trac.webkit.org/changeset/202558>