Closed
Bug 1002306
Opened 10 years ago
Closed 10 years ago
WebRTC VP8 doesn't work from b2g to desktop.
Categories
(Core :: WebRTC, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
Tracking | Status | |
---|---|---|
firefox30 | --- | unaffected |
firefox31 | --- | fixed |
firefox32 | --- | fixed |
People
(Reporter: ayang, Assigned: jesup)
Details
(Keywords: verifyme)
Attachments
(1 file, 1 obsolete file)
3.26 KB,
patch
|
ekr
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
B2G VP8 dones't work on M-C 179647. It returns kMediaConduitInvalidSendCodec at http://dxr.mozilla.org/mozilla-central/source/media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp?from=VcmSIPCCBinding.cpp#2130
Reporter | ||
Updated•10 years ago
|
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → Other
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8413793 [details] [diff] [review] don't accidentally disable non-H264 codecs in the OMX code Note: fixed by code inspection; I haven't tested this.
Attachment #8413793 -
Flags: review?(ekr)
Comment 3•10 years ago
|
||
Comment on attachment 8413793 [details] [diff] [review] don't accidentally disable non-H264 codecs in the OMX code Review of attachment 8413793 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/signaling/src/media/VcmSIPCCBinding.cpp @@ +2126,5 @@ > // and will reject registration of those not in it. > // TODO: bug 995884 to support H.264 in WebRTC.org code. > if (config->mName != "I420") { > // Do nothing for non-I420 config. > + return 0; This doesn't seem right. The idea of the more general code is to only add codecs to the SDP that we have support for. So if someone adds "VP9", we don't want to add that if nobody has installed a plugin (the I420 thing is a temporary). So, this should return errors if you ask for an unknown codec. In this case it should: - Shortcut return for VP8 - Create the OMX encoder for H.264 ("I420") - Otherwise return an error.
Attachment #8413793 -
Flags: review?(ekr) → review-
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8413793 -
Attachment is obsolete: true
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8413849 [details] [diff] [review] don't accidentally disable non-H264 codecs in the OMX code Addresses comments; this patch does more than just fix the mistake in the original patch
Attachment #8413849 -
Flags: review?(ekr)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → rjesup
Assignee | ||
Updated•10 years ago
|
status-firefox30:
--- → unaffected
status-firefox31:
--- → affected
status-firefox32:
--- → affected
Whiteboard: [webrtc-uplift]
Comment 6•10 years ago
|
||
Comment on attachment 8413849 [details] [diff] [review] don't accidentally disable non-H264 codecs in the OMX code Review of attachment 8413849 [details] [diff] [review]: ----------------------------------------------------------------- lgtm
Attachment #8413849 -
Flags: review?(ekr) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/32c92d57273a
Target Milestone: --- → mozilla32
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8413849 [details] [diff] [review] don't accidentally disable non-H264 codecs in the OMX code [Approval Request Comment] Bug caused by (feature/regressing bug #): 911046 User impact if declined: video webrtc calls won't work Testing completed (on m-c, etc.): Just landed on inbound; local testing done Risk to taking this patch (and alternatives if risky): Virtually no risk. Obviously broken code; patch makes things much cleaner. String or IDL/UUID changes made by this patch: none
Attachment #8413849 -
Flags: approval-mozilla-aurora?
Comment 9•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/32c92d57273a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Updated•10 years ago
|
Attachment #8413849 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/1521e333cd2a
Comment 11•10 years ago
|
||
Hi Jason, is this something we can help with if it needs QA work from Desktop? Let me or Anthony know. Thanks!
Flags: needinfo?(jsmith)
Keywords: verifyme
Comment 12•10 years ago
|
||
(In reply to Liz Henry :lizzard from comment #11) > Hi Jason, is this something we can help with if it needs QA work from > Desktop? Let me or Anthony know. Thanks! Yes, that should be possible so long as you guys have a Flame device to test with. I think Marc shipped one to Anthony to my understanding.
Flags: needinfo?(jsmith)
Comment 13•10 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #12) > Yes, that should be possible so long as you guys have a Flame device to test > with. I think Marc shipped one to Anthony to my understanding. Shipped last week, still waiting for it to arrive though.
Assignee | ||
Updated•10 years ago
|
Whiteboard: [webrtc-uplift]
You need to log in
before you can comment on or make changes to this bug.
Description
•