1144491 - Disable skip-to-keyframe logic during regular automation and test it separately

Status: RESOLVED WONTFIX

(Core :: Audio/Video, defect)

Description

[Bobby Holley (:bholley)]

We have a hypothesis that a large number of intermittent failures in media tests are caused by the skip-to-keyframe logic kicking in on overloaded VMs and slow mobile hardware. When activated, it can cause us to miss events etc.

Last week, we decided that we should build a mechanism to test the skip-to-keyframe logic separately, and disable it during other media tests.

I'm going to take a crack at fixing this sometime in the next month.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Good to hear that! I tried to write a test which tests the duration/end time of a captured media element and found it failed intermittently due to frame-skip.

Comment 2

[Jean-Yves Avenard [:jya]]

I think it would be unfortunate to disable it as it would be hiding a real problem (albeit intermittent)

the test timeouts because we don't fire ended. Even if we entered the skip-to-keyframe we should still fire ended once we reach the end of the video.

I believe I have found the reason for it and tracking it in bug 1143971

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Instead of disabling skip-to-next-keyframe (and then not testing what we ship) we could change the HTMLMediaElement.readyState to be calculated based on the buffered ranges, rather than calculated based on how many frames we have decoded. Then HAVE_CURRENT_DATA and HAVE_FUTURE_DATA would not be dependent on whether the decode can keep up with playback.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

(I haven't looked at the spec to see whether my idea of making readyState a function of the buffered ranges is an acceptable solution, and I dunno how well this would work in the case of live/infinite streams...)

Comment 5

[Bobby Holley (:bholley)]

My beef with skip-to-keyframe is that it's extremely non-deterministic, and depends heavily on VM load etc. I think that we should aim to make it more deterministic, and test it explicitly. This helps us across the board, both to make sure that skip-to-keyframe doesn't add to randomness, _and_ to make sure that we properly test skip-to-keyframe everywhere. 

One approach would be to set a pref that makes skip-to-keyframe script-controllable, and add certain tests to manifest.js that trigger it explicitly at a given point.

Comment 6

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Bobby Holley (:bholley) from comment #5)
> My beef with skip-to-keyframe is that it's extremely non-deterministic.

+1

Comment 7

[Bobby Holley (:bholley)]

Instead of doing this, we should just move everything to new-style keyframe skipping per bug 1158236. This is much more understandable and deterministic behavior, and something we couldn't disable even if we wanted to. Ideally, we'd run as much of the logical state off the audio clock as possible, though that may be tricky with readyState/drawImage.

We'll test new-style keyframe skipping in bug 1158237.