Closed Bug 974378 Opened 10 years ago Closed 10 years ago

[Gingerbread] [WebRTC] Audio Input / getUserMedia is broken on Gingerbread

Categories

(Core :: WebRTC, defect)

All
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla30
Tracking Status
firefox27 --- unaffected
firefox28 --- verified
firefox29 --- verified
firefox30 --- verified
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- verified
fennec 28+ ---

People

(Reporter: u421692, Assigned: gcp)

References

Details

(Keywords: regression, reproducible)

Attachments

(5 files, 2 obsolete files)

Attached image actual result
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)
Attached image expected result
Component: General → WebRTC
Product: Firefox for Android → Core
Flags: needinfo?(mihai.g.pop)
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)
tracking-fennec: --- → ?
Assignee: nobody → gpascutto
tracking-fennec: ? → 28+
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)
Attached file webrtc_log (obsolete) —
Flags: needinfo?(mihai.g.pop)
Attached file webrtc-log
From my HTC Nexus One (2.3), this is reproducible
Keywords: reproducible
ERROR     ; (11:51:55:156 |    1) AUDIO DEVICE:    1    99;      2441; OpenSL error: 4

Well that could be a big problem.
Attached file webrtc logs
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)
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.
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.
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.
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)
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.
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.
> 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
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)
Summary: [Gingerbread] [WebRTC] Not able to share microphone on gingerbread devices → [Gingerbread] [WebRTC] Audio Input / getUserMedia is broken on Gingerbread
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 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 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)
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 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+
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
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?
Attachment #8381382 - Flags: approval-mozilla-beta?
Attachment #8381382 - Flags: approval-mozilla-beta+
Attachment #8381382 - Flags: approval-mozilla-aurora?
Attachment #8381382 - Flags: approval-mozilla-aurora+
Verified as fixed on Nightly 30.0a1(2014-02-27).
Flags: needinfo?(mihai.g.pop)
Verified as fixed on FF 29 Aurora and Firefox 28 Beta 10.
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
Depends on: 1227316
This issue has already been verified fixed as referenced in comment 31.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Keywords: verifyme
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.