doesn't response to decoder error in GMP module

RESOLVED FIXED in Firefox 37

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ruil2, Assigned: ruil2)

Tracking

Trunk
mozilla38
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox37- fixed, firefox38 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_1) AppleWebKit/600.2.5 (KHTML, like Gecko) Version/8.0.2 Safari/600.2.5
(Assignee)

Comment 1

4 years ago
issue description:
Firefox squared client calls with iOS squared client.FF client's video suffers from frequent and long freezes. 
root cause:
decoder failed status doesn't be got in WebrtcGmpVideoCodec.cpp. so there is no IDR request.
(Assignee)

Comment 2

4 years ago
Created attachment 8553601 [details] [diff] [review]
decoder_patch.txt
(Assignee)

Comment 3

4 years ago
it needs to work with openh264 commit  6afdf36

Updated

4 years ago
Attachment #8553601 - Attachment is patch: true

Comment 4

4 years ago
This appears to match up with this pull req - https://github.com/cisco/openh264/pull/1751

I assume that this could be landed with the current plugin without breaking the current functionality?
Assignee: nobody → ruil2
(Assignee)

Comment 5

4 years ago
yes, it will not  break the current functionality.

Comment 6

4 years ago
OK, it worked for me with the current plugin.  I'll request a review from Randell.  Here's a try of some desktop builds to try it out:

https://tbpl.mozilla.org/?tree=Try&rev=79a6a7d8b2f0

Updated

4 years ago
Attachment #8553601 - Flags: review?(rjesup)
Comment on attachment 8553601 [details] [diff] [review]
decoder_patch.txt

Review of attachment 8553601 [details] [diff] [review]:
-----------------------------------------------------------------

r+, since Error() and Decode_g() are both on the GMP thread
Attachment #8553601 - Flags: review?(rjesup) → review+
Component: Untriaged → Video/Audio
Product: Firefox → Core
https://hg.mozilla.org/mozilla-central/rev/1d696936757e
Status: UNCONFIRMED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
[Tracking Requested - why for this release]:
This would enable pushing OpenH264 1.4 for Fx37. 1.4 will have a fix for interop issues with Chromebook's H.264 support.  (Note that OpenH264 releases separately from Firefox, but this is an enabling (and very simple/low-risk) patch).
status-firefox37: --- → affected
status-firefox38: --- → fixed
tracking-firefox37: --- → ?
Randell, are you asking for this to be uplifted to beta? 
If so, can you ask for approval for uplift on the patch?
Flags: needinfo?(rjesup)
I'm going to request it as soon as ehugg verifies these with OpenH264 1.4; he's working on that now
Flags: needinfo?(rjesup)
Comment on attachment 8553601 [details] [diff] [review]
decoder_patch.txt

Approval Request Comment
[Feature/regressing bug #]: N/A

[User impact if declined]: Inability to ship OpenH264 1.4 for 37

[Describe test coverage new/current, TreeHerder]: On 38/39 for a fair while; manually tested with OpenH264 1.4 (and just retested with 1.3 and current 1.4 build)

[Risks and why]: Virtually no risk.  Trivial patch to ensure error returns are passed up the chain so we can recover properly from decoding errors.

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

This fix has already been verified and improves our options for interop. Beta+
Attachment #8553601 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
This doesn't need to be tracked.
tracking-firefox37: ? → -
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf49b6b96324
Tested Windows and Linux debug & opt builds from the try manually with the current 1.3 plugin; ehugg tested with 1.3 and 1.4-candidate, both existing and new profiles.  Tested repeated calls, and also manually crashing the plugin.
You need to log in before you can comment on or make changes to this bug.