Closed Bug 1125047 Opened 7 years ago Closed 7 years ago

doesn't response to decoder error in GMP module


(Core :: Audio/Video, defect)

Not set



Tracking Status
firefox37 - fixed
firefox38 --- fixed


(Reporter: ruil2, Assigned: ruil2)



(1 file)

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
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.
it needs to work with openh264 commit  6afdf36
Attachment #8553601 - Attachment is patch: true
This appears to match up with this pull req -

I assume that this could be landed with the current plugin without breaking the current functionality?
Assignee: nobody → ruil2
yes, it will not  break the current functionality.
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:
Attachment #8553601 - Flags: review?(rjesup)
Comment on attachment 8553601 [details] [diff] [review]

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
Closed: 7 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).
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]

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]

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