830942 - getUserMedia fails to use video capture devices because it doesn't attach to a JVM

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect, P2)

Description

[Dan Mosedale (:dmosedale, :dmose)]

Various sorts of logging shows failures around NumberOfCaptureDevices and its descendents because we're not yet building and packing the appropriate Java classes and then configuring them with SetAndroidObjects.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Before we land this, we're going to want to do some stuff to make sure that everything's suitably compatible between WebRTC code and Fennec expectations
around

* threading models (and stuff that comes from the JNIEnv?)
* why Fennec's activity context is needed/used by the WebRTC code, and how Android's behavior around killing activities will come into play

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Doesn't block gum desktop release, obviously needed for Android

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP patch to wire up the Java objects for capture, v1

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP patch to wire up the JVM for capture, v2

Updated patch. Watching logcat on a build with this patch (and a few other less relevant ones) reveals:

D/*WEBRTCN*( 3550): SetAndroidObjects: could not find java class

which is being worked on over in bug 835973.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP patch to wire up the JVM for capture, v3

Tweaks some of the logging to be a bit clearer.  Practically speaking, these changes are really part of bug 835973.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP patch to wire up the JVM for capture, v4

Updates from GCP to actually call GetContext rather than passing in a null context, now that we've got a working GetContext patch.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: WIP patch to wire up JVM for gUM capture, v5 (landed on alder)

Updated to call the newly renamed GetGlobalContextRef so that we get a symbol that continues to be valid for other threads, and remove unnecessary logging changes.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

Landed on alder:

https://hg.mozilla.org/projects/alder/rev/92e321a18e82

Next steps: rebase to trunk, fix global context reference leak, request review.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

Before landing on trunk, we'll also need to fix the fact that this breaks FirefoxOS:

/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:33:27: error: AndroidBridge.h: No such file or directory
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp: In member function 'virtual void mozilla::MediaEngineWebRTC::EnumerateVideoDevices(nsTArray<nsRefPtr<mozilla::MediaEngineVideoSource> >*)':
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:49: error: 'mozilla::AndroidBridge' has not been declared
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:52: error: 'JavaVM' was not declared in this scope
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:52: error: 'jvm' was not declared in this scope
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:52: error: 'mozilla::AndroidBridge' has not been declared
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:55: error: 'JNIEnv' was not declared in this scope
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:55: error: 'env' was not declared in this scope
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:56: error: 'jint' was not declared in this scope
/builds/slave/a-ics-armv7a-g-dbg/build/content/media/webrtc/MediaEngineWebRTC.cpp:56: error: expected ';' before 'res'

Should be some simple ifdef tweaking.

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Attach VideoEngine to JVM

Rebased, fixed the leaks by deleting the reference immediately after use, and removing the unused static global one.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22cd5ee33178

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/22cd5ee33178