Update stall detection - don't use internal string, which changed and broke existing stall detection.

RESOLVED FIXED

Status

Testing Graveyard
external-media-tests
RESOLVED FIXED
3 years ago
4 months ago

People

(Reporter: maja_zf, Assigned: sydpolk)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
The output of video.mozMediaSourceObject.mozDebugReaderData has changed, so we can no longer detect a playback failure by counting the number of "active" streams. 

New debug reader data is described here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1186943#c5
(Assignee)

Updated

3 years ago
Summary: Update test_video_playback to be compatible with debug data produced by new MSE implementation → Update stall detection - don't use internal string, which changed and broke existing stall detection.
(Assignee)

Comment 1

3 years ago
Created attachment 8641112 [details] [review]
Check progression of current time instead of using internal debug date for stall detection.
Attachment #8641112 - Flags: review?(mjzffr)
(Reporter)

Comment 2

3 years ago
Comment on attachment 8641112 [details] [review]
Check progression of current time instead of using internal debug date for stall detection.

The new stall-detection code looks good and I like the refactor of partial playback. :) Some small things need to be fixed and I've made suggestions to refine the definition of a failure due to a stall, among other things. Please flag me again for review once you've addressed all the comments in the Github PR. Thanks!
Attachment #8641112 - Flags: review?(mjzffr) → review-
(Assignee)

Comment 3

3 years ago
Comment on attachment 8641112 [details] [review]
Check progression of current time instead of using internal debug date for stall detection.

Maja, please review latest changes.
Attachment #8641112 - Flags: review- → review?(mjzffr)
(Reporter)

Comment 4

3 years ago
Comment on attachment 8641112 [details] [review]
Check progression of current time instead of using internal debug date for stall detection.

lgtm - I'll accept the PR after a couple more small changes. See Github comments.
Attachment #8641112 - Flags: review?(mjzffr) → review+
(Assignee)

Comment 5

3 years ago
Pushed latest branch with nits, flake8 nits, and a squashed commit.
(Reporter)

Comment 6

3 years ago
Merged; rebased pf-jenkins production branch.

https://github.com/mjzffr/firefox-media-tests/commit/7c602c5fbd632ad9a767164d431fd656270b5ec5
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED

Updated

4 months ago
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.