Closed Bug 1367955 Opened 6 years ago Closed 6 years ago

Intermittent dom/media/test/test_mediarecorder_record_changing_video_resolution.html | called finish() multiple times

Categories

(Core :: Audio/Video: Recording, defect, P5)

Product:
Core
Component:
Audio/Video: Recording
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: bryce)

References

(Depends on 1 open bug)

Details

(Keywords: intermittent-failure, Whiteboard: [stockwell unknown])

Attachments

(2 files, 1 obsolete file)

Patch adding extra logging to help diagnose issue
1.90 KB, patch
Details | Diff | Splinter Review
Bug 1367955 - Remove onended handler during ended handling to avoid video.ended race issues.
59 bytes, text/x-review-board-request
pehrsons
: review+
Details
Component: Audio/Video: Playback → Audio/Video: Recording
Rank: 25
Priority: -- → P2
Quick look at this and it's being caused by the onended function being called multiple times. If the video has already ended and seekToNextFrame() is called the ended event will fire a second time. I'm pretty sure video.ended check in seekThroughFrames races, and rarely another seekThroughFrames() will be queued when we're on the last frame: I've confirmed this locally with extra logging. I would have thought this couldn't happen: that if the video has ended as a result of seekToNextFrame, that by the time the then() is run video.ended should consistently return true.

Took me 152 iterations to hit the issue. Will attach patch for others to repro.
Depends on: 1386489
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3
Bulk priority update of open intermittent test failure bugs. 

P3 => P5

https://bugzilla.mozilla.org/show_bug.cgi?id=1381960
Priority: P3 → P5
Uptick in failure rate is odd. There have been some moderate sized recorder changes which may have impacted this.

Provided the reason we're failing is the save as discussed in the above comments: IMO this test shouldn't be disabled and these failures represent an real underling issue, though not the one the test is designed to test. I would prefer the test can remain as is, but since we have Bug 1386489 to track the issue causing this test to fail, I could band-aid hack the test to avoid the failure.

Since Bug 1386489 looks stalled, I'll band-aid this if failures remain high.
:singingtree, thanks for commenting on this- I agree, if this remains failing so frequently it would warrant some attention.  There is also a QuantumRender only failure for this test in bug 1367879- not sure if it is all related.
See Also: → 1367879
Whiteboard: [stockwell needswork]
this stopped failing on october 6th, possibly this is fixed by disabling the test on linux64-webrender in bug 1367879.
Whiteboard: [stockwell needswork] → [stockwell unknown]
this has started failing again on windows 10* and linux64 debug/asan.  The previous comment of this error stopping was true, we had 3 days with no failures (oddly a friday-sunday).

:singingtree- could you take another look at this bug when you get a chance?
Flags: needinfo?(bvandyk)
Whiteboard: [stockwell unknown] → [stockwell needswork]
gentle ping :singingtree
Apologies for the delay. Let's see if this bandaid helps.
Flags: needinfo?(bvandyk)
Comment on attachment 8922473 [details]
Bug 1367955 - Remove onended handler during ended handling to avoid video.ended race issues.

https://reviewboard.mozilla.org/r/193546/#review198950
Attachment #8922473 - Flags: review?(apehrson) → review+
Attachment #8922472 - Attachment is obsolete: true
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a30a745cfbcf
Remove onended handler during ended handling to avoid video.ended race issues. r=pehrsons
https://hg.mozilla.org/mozilla-central/rev/a30a745cfbcf
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee: nobody → bvandyk
status-firefox57: --- → affected
You need to log in before you can comment on or make changes to this bug.