Closed Bug 1137515 Opened 5 years ago Closed 5 years ago

Enable WebRTC on Lollipop Gonk

Categories

(Core :: WebRTC, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
blocking-b2g 2.2+
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: diego, Assigned: sotaro)

References

Details

(Keywords: verifyme, Whiteboard: [caf priority: p1][CR 801198])

Attachments

(3 files, 4 obsolete files)

MOZ_WEBRTC was disabled during Lollipop bring up [1]. It's time to bring it back!

I get several WebrtcOMXH264VideoCodec.cpp compilation errors when I try building it [2].

[1] https://mxr.mozilla.org/mozilla-central/source/configure.in#5112
[2] https://pastebin.mozilla.org/8823524
Hey Randell,

Do you know who can take a look at this?
blocking-b2g: --- → 2.2?
Flags: needinfo?(rjesup)
Hardware: x86 → ARM
Whiteboard: [CR 801198]
Whiteboard: [CR 801198] → [caf priority: p2][CR 801198]
I'll try to look at it, thanks (or have someone do so).
Flags: needinfo?(rjesup)
Hi Randell,

Thanks for taking this on. How's this progressing? We track these CAF issues weekly and haven't seen an update here in 5 days.

Thanks,
Mike
Flags: needinfo?(rjesup)
blocking-b2g: 2.2? → 2.2+
I confirmed the build failure on nexus-5-l build. I take a look.
Assignee: nobody → sotaro.ikeda.g
Attachment #8573283 - Attachment is obsolete: true
Attached patch patch part 2 - Change to media (obsolete) — Splinter Review
sotaro: what can I do to help? Do these patches cover the entire thing?
Flags: needinfo?(rjesup) → needinfo?(sotaro.ikeda.g)
(In reply to Randell Jesup [:jesup] from comment #8)
> sotaro: what can I do to help? Do these patches cover the entire thing?

Thanks for your offer! If I faced the problem that realted to general WebRTC things, I am going to ask your help.

The patches fixed the build failure, but getUserMedia still do not work. It seems related to gonk dependent problem. I am going to investigate about it.
Flags: needinfo?(sotaro.ikeda.g)
Attached patch patch part 2 - Change to media (obsolete) — Splinter Review
Disable InitDirectMediaBuffer() on lollipop. This is a hack added by bug 938034 for camera recording capability via gUM. It uses GonkCameraSource for recording high resolution and correct time stamp video farmes. GonkCameraSource request video data as non-metadata mode, but some hardware like neuxu-5 seems to support only by metadata mode.

By the patch, I conformed that the following sample works.
http://webrtc.github.io/samples/src/content/getusermedia/gum/
Attachment #8573354 - Attachment is obsolete: true
But with applying the patches, trying WebRTC caused a crash under nss_InitModules(). I test it by using the following sample.

http://webrtc.github.io/samples/src/content/peerconnection/pc1/
crash stack of comment 11
GDB actually showed the following when the crash happen. The symptom seems similar to bug 974227.

> Program received signal SIGSYS, Bad system call.
> readlinkat () at bionic/libc/arch-arm/syscalls/readlinkat.S:9
See Also: → 938034
(In reply to Sotaro Ikeda [:sotaro] from comment #13)
> GDB actually showed the following when the crash happen. The symptom seems
> similar to bug 974227.
> 
> > Program received signal SIGSYS, Bad system call.
> > readlinkat () at bionic/libc/arch-arm/syscalls/readlinkat.S:9

When I disabled the sandbox, I did not see the crash and confirmed WebRTC video call without enabling H.264.

https://apprtc.appspot.com/
See Also: → 974227
Depends on: 1140111
I enabled H.264 by adding the following. It caused the crash.

> pref("media.peerconnection.video.h264_enabled", true);
Whiteboard: [caf priority: p2][CR 801198] → [caf priority: p1][CR 801198]
(In reply to Sotaro Ikeda [:sotaro] from comment #15)
> I enabled H.264 by adding the following. It caused the crash.
> 
> > pref("media.peerconnection.video.h264_enabled", true);

This is a regression of Bug 1109248. EncOutputDrain::SendEncodedDataToCallback() need to add handling of RTPFragmentationHeader. This is not a gonk L specific problem. It seems better to create a new bug for it.
See Also: → 1109248
Depends on: 1140677
(In reply to Sotaro Ikeda [:sotaro] from comment #16)
> (In reply to Sotaro Ikeda [:sotaro] from comment #15)
> > I enabled H.264 by adding the following. It caused the crash.
> > 
> > > pref("media.peerconnection.video.h264_enabled", true);
> 
> This is a regression of Bug 1109248.
> EncOutputDrain::SendEncodedDataToCallback() need to add handling of
> RTPFragmentationHeader. This is not a gonk L specific problem. It seems
> better to create a new bug for it.

This second issue wouldn't affect 2.2/b2g37, note, since it was caused by the update that landed in 38.
Yeah, I notice it after I created Bug 1140677.
Blocks: 1141311
Attachment #8573353 - Flags: review?(mwu)
No longer blocks: 1141311
Depends on: 1141311
Attachment #8573353 - Flags: review?(mwu) → review+
Attached patch patch part 2 - Change to media (obsolete) — Splinter Review
Attachment #8573440 - Attachment is obsolete: true
Attachment #8575401 - Flags: review?(rjesup)
Comment on attachment 8575401 [details] [diff] [review]
patch part 2 - Change to media

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

::: dom/media/webrtc/MediaEngineGonkVideoSource.cpp
@@ +245,5 @@
>      }
>    }
>  
> +  // XXX some devices support recording camera frame only in metadata mode.
> +  // But GonkCameraSource requests non-metadata recording mode. 

trailing space
Attachment #8575401 - Flags: review?(rjesup) → review+
Hmm, lollipop build option seems to be changed more strict. It seems like -Werror was enabled.
Fix build failure. Carry "r=jesup".
Attachment #8575401 - Attachment is obsolete: true
Attachment #8576171 - Flags: review+
No longer blocks: CAF-v3.0-FL-metabug
https://hg.mozilla.org/mozilla-central/rev/ebcfd00277da
https://hg.mozilla.org/mozilla-central/rev/6058c65ec299
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Comment on attachment 8573353 [details] [diff] [review]
patch part 1 - Change to configure.in

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: WebRTC can not be used on lollipop gonk device.
Testing completed: locally tested
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none
Attachment #8573353 - Flags: approval-mozilla-b2g37?
Attachment #8573353 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Keywords: verifyme
So with the patches landed on v2.2 I can now establish a WebRTC session. Progress!

Loopback works fine [1].

In an actual apprtc [2] session the outgoing video of the L-gonk phone looks garbled on the remote KK phone side [3]. The incoming video looks fine on the L-gonk phone[4]. I tried this with L-phone to KK-gonk phone and L-gonk phone to Desktop. I got the same result.

Did you get a chance to try a phone to phone session on v2.2 Lollipop?

[1] http://mozilla.github.io/webrtc-landing/pc_test.html
[2] https://apprtc.appspot.com/
[3] https://www.dropbox.com/s/u9uum0yk0gy73zw/2015-03-13%2014.29.27.jpg?dl=0
[4] https://www.dropbox.com/s/umi9zo4kvw7gd8n/2015-03-13%2014.29.32.jpg?dl=0
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Diego Wilson [:diego] from comment #29)
> So with the patches landed on v2.2 I can now establish a WebRTC session.
> Progress!
> 
> Loopback works fine [1].
> 
> In an actual apprtc [2] session the outgoing video of the L-gonk phone looks
> garbled on the remote KK phone side [3]. The incoming video looks fine on
> the L-gonk phone[4]. I tried this with L-phone to KK-gonk phone and L-gonk
> phone to Desktop. I got the same result.
> 
> Did you get a chance to try a phone to phone session on v2.2 Lollipop?

I did h.264 WebRTC session with [2] between v2.2 nexus-5-l and v2.2 flame-kk, but I did not saw such problem.
Flags: needinfo?(sotaro.ikeda.g)
diego, can you create a but for it?
Flags: needinfo?(dwilson)
Blocks: 1143694
(In reply to Sotaro Ikeda [:sotaro] from comment #31)
> diego, can you create a but for it?

Done
Flags: needinfo?(dwilson)
You need to log in before you can comment on or make changes to this bug.