Closed
Bug 974378
Opened 10 years ago
Closed 10 years ago
[Gingerbread] [WebRTC] Audio Input / getUserMedia is broken on Gingerbread
Categories
(Core :: WebRTC, defect)
Tracking
()
VERIFIED
FIXED
mozilla30
People
(Reporter: u421692, Assigned: gcp)
References
Details
(Keywords: regression, reproducible)
Attachments
(5 files, 2 obsolete files)
51.31 KB,
image/png
|
Details | |
50.48 KB,
image/png
|
Details | |
11.23 KB,
text/plain
|
Details | |
32.00 KB,
text/plain
|
Details | |
17.52 KB,
patch
|
jesup
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Environment: Device: HTC Desire S (Android 2.3.3) Build: Firefox 28 Beta 4 Steps to reproduce: 1. Open http://mysecondwebrtc.appspot.com/ or http://mozilla.github.io/webrtc-landing/gum_test.html 2. Choose to share both audio and video Expected result: A pop-up is displayed with the option to share microphone and video Actual result: A pop-up is displayed with the option to share only the video(please see attached screenshot)
Updated•10 years ago
|
Component: General → WebRTC
Product: Firefox for Android → Core
Updated•10 years ago
|
Flags: needinfo?(mihai.g.pop)
Keywords: regressionwindow-wanted
Good build: 2013-11-08 Bad build: 2013-11-09 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f003c386c77a&tochange=9e571ad29946 Probably bug 932112 caused this regression.
Flags: needinfo?(mihai.g.pop)
Keywords: regressionwindow-wanted
Updated•10 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → gpascutto
Updated•10 years ago
|
tracking-fennec: ? → 28+
Comment 3•10 years ago
|
||
Can we get logs from a GB device with NSPR_LOG_MODULES=getusermedia:5,mediamanager:5 and if possible webrtc_trace:65535 and WEBRTC_TRACE_FILE=someplace? (Upstream doesn't really support GB...) Thanks! (I'll take logs from anyone who can get them - I no longer have a GB device)
Flags: needinfo?(mihai.g.pop)
Comment 5•10 years ago
|
||
From my HTC Nexus One (2.3), this is reproducible
Updated•10 years ago
|
Keywords: reproducible
Comment 6•10 years ago
|
||
ERROR ; (11:51:55:156 | 1) AUDIO DEVICE: 1 99; 2441; OpenSL error: 4 Well that could be a big problem.
Attachment #8379729 -
Attachment is obsolete: true
(In reply to Mihai Pop from comment #7) > Created attachment 8379820 [details] > webrtc logs Logs from HTC Desire S (Android 2.3.3)
Assignee | ||
Comment 9•10 years ago
|
||
The logs don't have enough info to nail down what exactly is the problem here, but it reproduces on my Galaxy S2 2.3.x so I can deal with it.
Assignee | ||
Comment 10•10 years ago
|
||
E/libOpenSLES( 6100): slCreateEngine while another engine 0x2719d8 is active Pretty sure this is the problem. There's an assertion in the webrtc.org code to ensure some audio parts aren't initialized twice that we hit in debug mode, but which is harmless on newer devices.
Assignee | ||
Comment 11•10 years ago
|
||
So what I think happened is this: in Firefox 27/old OpenSL code, we had a single OpenSL audio driver: media/webrtc/trunk/webrtc/modules/audio_device/audio_device_opensles.cc: int32_t res = f_slCreateEngine(&sles_engine_, 1, EngineOption, 0, NULL, NULL); In Firefox 28/new OpenSL code, we have one for input and output: media/webrtc/trunk/webrtc/modules/audio_device/android/opensles_output.cc: OPENSL_RETURN_ON_FAILURE(f_slCreateEngine(&sles_engine_, 1, kOption, 0, media/webrtc/trunk/webrtc/modules/audio_device/android/opensles_input.cc: OPENSL_RETURN_ON_FAILURE(f_slCreateEngine(&sles_engine_, 1, kOption, 0, (Additionally there's potentially one in libcubeb as well!) It looks like the OpenSL ES implementation in Android 2.3 can't handle this.
Assignee | ||
Comment 12•10 years ago
|
||
padenot raised the question of why we have the output code there at all, as we use libcubeb. It seems to be used to set this info: http://dxr.mozilla.org/mozilla-central/source/media/webrtc/trunk/webrtc/modules/audio_device/audio_device_buffer.cc?from=SetVQEData&case=true#293 Patching away the entire opensles_output.cc part makes the loopback test work, but with very crappy quality. (Which might be an input issue, as this bug shows that has never been tested on Android 2.3)
Assignee | ||
Comment 13•10 years ago
|
||
Forcing the audio backend to Java JNI makes it work too, and with better audio quality. I think the opensles_input.cc isn't tuned very well for such old devices. This is the least impact, and given that this bug is in beta, I think I prefer that as a fix.
Assignee | ||
Comment 14•10 years ago
|
||
Buh, the JNI links aren't set up at the moment the webrtc code initializes the audio backends. That makes switching them based on the SDK version rather tricky - there's no native way to probe the API level.
Assignee | ||
Comment 15•10 years ago
|
||
> It looks like the OpenSL ES implementation in Android 2.3 can't handle this. Well darn, I already concluded this almost a year ago: https://bugzilla.mozilla.org/show_bug.cgi?id=859805#c7
Assignee | ||
Comment 16•10 years ago
|
||
Removes the OpenSL output code. This will make OpenSL input work on Gingerbread. Note also that libcubeb will fail to open with OpenSL for the same reason, but it has a working fallback to AudioTrack. (See the bug just before this comment). Increasing the buffer to the value we had before the webrtc.org uplift makes audio OK on my SGS2. I also verified that this doesn't seem to increase latency.
Attachment #8380790 -
Flags: review?(rjesup)
Attachment #8380790 -
Flags: feedback?(paul)
Assignee | ||
Updated•10 years ago
|
Summary: [Gingerbread] [WebRTC] Not able to share microphone on gingerbread devices → [Gingerbread] [WebRTC] Audio Input / getUserMedia is broken on Gingerbread
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8380790 [details] [diff] [review] Patch 1. Remove OpenSL output engine Review of attachment 8380790 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/webrtc/trunk/webrtc/modules/audio_device/android/audio_device_opensles_android.cc @@ +89,4 @@ > } > > bool AudioDeviceAndroidOpenSLES::PlayoutIsInitialized() const { > + return -1; Welp. return false.
Comment 18•10 years ago
|
||
Comment on attachment 8380790 [details] [diff] [review] Patch 1. Remove OpenSL output engine Review of attachment 8380790 [details] [diff] [review]: ----------------------------------------------------------------- Way too intrusive to take upstream. I'd like to not carry all these mods forever. Let's try something that at least has a chance of upstreaming, and then file an upstream bug with our patch (and mention the alternatives). ::: media/webrtc/trunk/webrtc/modules/audio_device/android/audio_device_opensles_android.cc @@ -16,5 @@ > namespace webrtc { > > AudioDeviceAndroidOpenSLES::AudioDeviceAndroidOpenSLES(const int32_t id) > - : output_(id), > - input_(id, &output_) { Add a build var for OpenSL output enabled, and if it's not set don't Init() it and pass null to input_(). Then either make output_ a pointer and don't create it and add null-checks in all the spots below, or use ifdefs in all the spots below to avoid referencing output_, or leave it as it is and add "if (output_.Initialized()) {...}" around all of them. I suspect the ifdef might be the most upstream-acceptable implementation; just a guess. ::: media/webrtc/trunk/webrtc/modules/audio_device/android/audio_device_opensles_android.h @@ -154,5 @@ > virtual int32_t SetLoudspeakerStatus(bool enable); > virtual int32_t GetLoudspeakerStatus(bool& enable) const; > > private: > - OpenSlesOutput output_; leave or modify this ::: media/webrtc/trunk/webrtc/modules/audio_device/android/opensles_input.cc @@ +553,5 @@ > int8_t* audio = fifo_->Pop(); > audio_buffer_->SetRecordedBuffer(audio, buffer_size_samples()); > + // XXX: Get the correct value from our output layer. > + // audio_buffer_->SetVQEData(delay_provider_->PlayoutDelayMs(), > + // recording_delay_, 0); Instead: audio_buffer_->SetVQEData(delay_provider ? delay_provider_->PlayoutDelayMs() : 0, recording_delay_, 0);
Attachment #8380790 -
Flags: review?(rjesup) → review-
Comment 19•10 years ago
|
||
Comment on attachment 8380790 [details] [diff] [review] Patch 1. Remove OpenSL output engine Review of attachment 8380790 [details] [diff] [review]: ----------------------------------------------------------------- What jesup said :-)
Attachment #8380790 -
Flags: feedback?(paul)
Assignee | ||
Comment 20•10 years ago
|
||
I also prefer the #ifdef variant as it avoids the unused code from being compiled (in).
Attachment #8380790 -
Attachment is obsolete: true
Attachment #8381382 -
Flags: review?(rjesup)
Comment 21•10 years ago
|
||
Comment on attachment 8381382 [details] [diff] [review] Patch 1. v2 Disable the OpenSL output engine Review of attachment 8381382 [details] [diff] [review]: ----------------------------------------------------------------- r+ with one nit ::: media/webrtc/trunk/webrtc/modules/audio_device/audio_device.gypi @@ +168,5 @@ > ], > }], > + ['enable_android_opensl_output==1', { > + 'sources': [ > + 'opensl/opensles_output.cc' missing 'defines' section to define WEBRTC_ANDROID_OPENSLES_OUTPUT
Attachment #8381382 -
Flags: review?(rjesup) → review+
Assignee | ||
Comment 22•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/953f462e19ac
Comment 23•10 years ago
|
||
Please verify once this lands on central and beta.
Flags: needinfo?(mihai.g.pop)
Keywords: verifyme
https://hg.mozilla.org/mozilla-central/rev/953f462e19ac
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Assignee | ||
Comment 25•10 years ago
|
||
Comment on attachment 8381382 [details] [diff] [review] Patch 1. v2 Disable the OpenSL output engine [Approval Request Comment] Bug caused by (feature/regressing bug #): Bug 932112 update from upstream User impact if declined: No WebRTC+getUserMedia on Android 2.3 (~20% of users) Testing completed (on m-c, etc.): Just landed on m-c Risk to taking this patch (and alternatives if risky): Slight risk that we introduce new getUserMedia/WebRTC breakage. It's low because this disables code that we aren't supposed to use. Supposed to... :)
Attachment #8381382 -
Flags: approval-mozilla-beta?
Attachment #8381382 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Updated•10 years ago
|
Attachment #8381382 -
Flags: approval-mozilla-beta?
Attachment #8381382 -
Flags: approval-mozilla-beta+
Attachment #8381382 -
Flags: approval-mozilla-aurora?
Attachment #8381382 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 26•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/446339e15b97
Reporter | ||
Comment 27•10 years ago
|
||
Verified as fixed on Nightly 30.0a1(2014-02-27).
Flags: needinfo?(mihai.g.pop)
Assignee | ||
Comment 28•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/525d9af770e7
Updated•10 years ago
|
status-b2g-v1.3:
--- → fixed
Reporter | ||
Comment 30•10 years ago
|
||
Verified as fixed on FF 29 Aurora and Firefox 28 Beta 10.
Updated•10 years ago
|
status-b2g-v1.3T:
--- → fixed
status-b2g-v1.4:
--- → fixed
Comment 31•10 years ago
|
||
1.4 Buri: User is asked permission to use microphone and camera. Buri only has 1 camera so choice is not presented. 1.4 Flame: User is asked permission to use microphone and camera. User is given option of which camera to use. 1.3 Buri: User is asked permission to use microphone only. 1.3 Environmental Variables: Device: Buri 1.3 MOZ BuildID: 20140528024003 Gaia: 0ce948e378cab7ed3db20231281dd7ca2eb99779 Gecko: ce0dd450bffb Version: 28.0 Firmware Version: v1.2-device.cfg 1.4 Environmental Variables: Device: Buri 1.4 MOZ BuildID: 20140529000204 Gaia: 7bc1c15c67661a0b8e35f18f15a9d03d1d2cfcd5 Gecko: 2181cac4d0fc Version: 30.0 Firmware Version: v1.2-device.cfg 1.4 Environmental Variables: Device: Flame 1.4 MOZ BuildID: 20140529000204 Gaia: 7bc1c15c67661a0b8e35f18f15a9d03d1d2cfcd5 Gecko: 2181cac4d0fc Version: 30.0 Firmware Version: 10g-2
Comment 32•8 years ago
|
||
This issue has already been verified fixed as referenced in comment 31.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Keywords: verifyme
Updated•8 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
You need to log in
before you can comment on or make changes to this bug.
Description
•