finish and exit from Flush() even if MFTManager rejects sample

RESOLVED FIXED in Firefox 38.0.5

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

Trunk
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox37 unaffected, firefox38 wontfix, firefox38.0.5 fixed, firefox39 fixed, firefox40 fixed, firefox-esr38 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

4 years ago
No description provided.
Assignee

Comment 1

4 years ago
Posted patch patchSplinter Review
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a88c28090502
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5c8639988e4

This is not the cause of the hangs I was seeing in
https://bugzilla.mozilla.org/show_bug.cgi?id=1129455#c18
because those hangs still had threads in WMFVideoMFTManager::Output().
Attachment #8598908 - Flags: review?(cpearce)
Attachment #8598908 - Flags: review?(cpearce) → review+
https://hg.mozilla.org/mozilla-central/rev/f0bd0f42f57a
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee

Comment 4

4 years ago
Comment on attachment 8598908 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1155432, which was uplifted to 38

[User impact if declined]: potential shutdown hang, perhaps showing up earlier as media failing to play.
I have seen some shutdown hang reports tracked in bug 1129455 without WMFVideoMFTManager::Output() on any stacks, but that may be just because the stack unwinding failed.

[Describe test coverage new/current, TreeHerder]:
None for this particular line, as I don't know how to trigger the error path.

[Risks and why]: Low because the code is simple and getting to this line of code would currently be producing a hang anyway.

[String/UUID change made/needed]: none.
Attachment #8598908 - Flags: approval-mozilla-beta?
Attachment #8598908 - Flags: approval-mozilla-aurora?
Comment on attachment 8598908 [details] [diff] [review]
patch

Approved for uplift to aurora since this has had time on m-c for the last week and this may help to improve the rate of shutdown hangs that show up in 39.
Attachment #8598908 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8598908 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Looks like this is for 38.0.5. Updating the approval flag to reflect that.
Attachment #8598908 - Flags: approval-mozilla-beta? → approval-mozilla-release?
Comment on attachment 8598908 [details] [diff] [review]
patch

Taking it as it should improve the quality of Firefox.
Attachment #8598908 - Flags: approval-mozilla-release?
Attachment #8598908 - Flags: approval-mozilla-release+
Attachment #8598908 - Flags: approval-mozilla-esr38+
You need to log in before you can comment on or make changes to this bug.