MediaStreamGraphImpl hangs onto last displayed frame until GC

RESOLVED FIXED in Firefox 18

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mikeh, Assigned: mikeh)

Tracking

unspecified
mozilla20
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed, firefox20 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

This is a new bug to address the MediaStreamGraphImpl component of bug 809259.

In short: the Gonk camera has 9 buffers, and every stopped instance of a DOMMediaStream holds onto one of these buffers until the GC mops them up.  If the GC doesn't happen quickly enough, we can run out of buffers.
Created attachment 683162 [details] [diff] [review]
MediaStreamGraphImpl no longer indefinitely keeps the last-played video frame

roc, this patch got left hanging in the review in bug 809259.  There are more changes necessary to fix that issue, but this will be an important part of it that I'd like to address separately while I work on the rest.
Attachment #683162 - Flags: review?(roc)
blocking-basecamp: --- → ?
Comment on attachment 683162 [details] [diff] [review]
MediaStreamGraphImpl no longer indefinitely keeps the last-played video frame

This is OK but you should probably also add to MediaStreamGraphImpl::PlayVideo to not cache the last played frame in mLastPlayedVideoFrame if mNotifiedFinished is true.
Attachment #683162 - Flags: review?(roc) → review+
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #2)
> Comment on attachment 683162 [details] [diff] [review]
> MediaStreamGraphImpl no longer indefinitely keeps the last-played video frame
> 
> This is OK but you should probably also add to
> MediaStreamGraphImpl::PlayVideo to not cache the last played frame in
> mLastPlayedVideoFrame if mNotifiedFinished is true.

Now that you mention it, this change does look a little thin.  I wonder where the rest went.
Created attachment 683585 [details] [diff] [review]
MediaStreamGraphImpl no longer indefinitely keeps the last-played video frame

This is the correct and complete patch.  Sorry about that last one.
Attachment #683162 - Attachment is obsolete: true
Attachment #683585 - Flags: review?(roc)

Updated

6 years ago
blocking-basecamp: ? → +
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f912278fcd61
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Whiteboard: needs-checkin-aurora
https://hg.mozilla.org/releases/mozilla-aurora/rev/71c49c2616d3
https://hg.mozilla.org/releases/mozilla-beta/rev/21fa20987f04
status-firefox18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
Whiteboard: needs-checkin-aurora
You need to log in before you can comment on or make changes to this bug.