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

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: patch

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().

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f0bd0f42f57a

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f0bd0f42f57a

Comment 4

[Karl Tomlinson (:karlt)]

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.

Comment 5

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/99df7384dfc2

Comment 7

[Liz Henry (:lizzard) (relman/hg->git project)]

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.

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8598908 [details] [diff] [review]
patch

Taking it as it should improve the quality of Firefox.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/825e8ac4ab29

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr38/rev/b2b65269a5e9