933284 - crash in @0x0 | audiotrack_get_max_channel_count

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Ioana Chiorean]

This bug was filed from the Socorro interface and is 
report bp-c17d7ac4-13f7-44b4-ae3f-b15f12131031.

Steps:
1. Go to http://mozilla.github.com/webrtc-landing/gum_test.html and select Audio & Video with a built-in device camera, built-in device microphone
2. When pop up prompted choose share.

Reproducing it only on Aurora 27.0a2 10/30
=============================================================
 		@0x0 	
1 	libxul.so 	audiotrack_get_max_channel_count 	media/libcubeb/src/cubeb_audiotrack.c
2 	libxul.so 	cubeb_get_preferred_sample_rate 	media/libcubeb/src/cubeb.c
3 	libxul.so 	mozilla::AudioStream::PreferredSampleRate() 	content/media/AudioStream.cpp
4 	libxul.so 	mozilla::MediaStreamGraphImpl::RunThread() 	content/media/MediaStreamGraph.h
5 	libxul.so 	mozilla::::MediaStreamGraphInitThreadRunnable::Run 	content/media/MediaStreamGraph.cpp
6 	libxul.so 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
7 	libxul.so 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
8 	libxul.so 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp
9 	libnss3.so 	_pt_root 	nsprpub/pr/src/pthreads/ptthread.c
10 	libc.so 	libc.so@0x11e22 	
11 	libc.so 	libc.so@0x119ee

Comment 1

[Ioana Chiorean]

Device: Galaxy R (Android 2.3.4)

Comment 2

[Matthew Gregan [:kinetik]]

1 	libxul.so 	audiotrack_get_max_channel_count 	media/libcubeb/src/cubeb_audiotrack.c
2 	libxul.so 	cubeb_get_preferred_sample_rate 	media/libcubeb/src/cubeb.c
3 	libxul.so 	mozilla::AudioStream::PreferredSampleRate() 	content/media/AudioStream.cpp

That stack must be confused, I can't see how it got from cubeb_get_preferred_sample_rate into audiotrack_get_max_channel_count.  The cubeb_ops initialization for audiotrack looks correct.  Also, audiotrack_get_max_channel_count is very simple, asserting two things and then storing a fixed value in a pointer.  The only caller (AudioStream::MaxNumberOfChannels) passes in valid pointers.

So, assuming this is really inside audiotrack_get_preferred_sample_rate: we can be fairly sure ctx and rate are valid pointers, so the most likely problem is that ctx->klass.get_output_samplingrate is null.  That's set in audiotrack_init (which initializes the ctx), but only when audiotrack_version_is_froyo is true (which should be false on this device, given it's 2.3.4, but it depends on the libmedia.so present on the device), but is called without testing ctx->klass.get_output_samplingrate for null or that audiotrack_version_is_froyo is true when called in audiotrack_get_preferred_sample_rate.

Can you please try reproducing the crash with a debug build and attaching the output of adb logcat? There should be a bunch of "Gecko - Cubeb" log entries present.  That should confirm what I've written.

We can try changing audiotrack_init to initialize ctx->klass.get_output_samplingrate for non-Froyo, but I'll need to double check the function signature first.

Comment 3

[Matthew Gregan [:kinetik]]

Initialize ctx->klass.get_output_samplingrate unconditionally: https://tbpl.mozilla.org/?tree=Try&rev=e1cb202d97ba

Comment 4

[Matthew Gregan [:kinetik]]

Attachment: bug933284_v0.patch

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Reproduces on my 2.3.6 Galaxy S2 with Nightly.

I/ALSAModule( 2588): Initialized ALSA CAPTURE device hifi
E/libOpenSLES(17791): slCreateEngine while another engine 0x2fcb08 is active
I/Gecko - Cubeb(17791): _ZN7android10AudioTrackC1EijiiijPFviPvS1_ES1_iitm: OK
I/Gecko - Cubeb(17791): _ZN7android10AudioTrackD1Evtm: OK
I/Gecko - Cubeb(17791): _ZNK7android10AudioTrack7latencyEvtm: OK
I/Gecko - Cubeb(17791): _ZNK7android10AudioTrack9initCheckEvtm: OK
I/Gecko - Cubeb(17791): error while loading _ZN7android10AudioTrack16getMinFrameCountEPi19audio_stream_type_tjtm: Symbol not found: load_library[1105]: Library 'libc.so.6' not foundtm
I/Gecko - Cubeb(17791): _ZN7android10AudioTrack16getMinFrameCountEPiijtm: OK
I/Gecko - Cubeb(17791): _ZN7android10AudioTrack5startEvtm: OK
I/Gecko - Cubeb(17791): _ZN7android10AudioTrack5pauseEvtm: OK
I/Gecko - Cubeb(17791): _ZN7android10AudioTrack11getPositionEPjtm: OK
I/Gecko - Cubeb(17791): _ZN7android10AudioTrack17setMarkerPositionEjtm: OK
E/yamaha::media::Parameters( 2588): SalesCode = XEN
I/libfimc ( 2704): bool SecFimc::create(SecFimc::FIMC_DEV, fimc_overlay_mode, unsigned int):: Fimc reserved memory =0x57da0000
E/Surface (17791): Surface::init token -2 identity 479
D/WEBRTC-JC(17791): VideoCaptureAndroid::surfaceCreated
D/WEBRTC-JC(17791): VideoCaptureAndroid::surfaceChanged
D/WEBRTC-JC(17791): tryStartCapture -1 height -1 frame rate -1 isCaptureRunning false isSurfaceReady true isCaptureStarted false
I/WindowManager( 2704):   CREATE SURFACE Surface(name=SurfaceView, identity=479, mNativeSurface=0) IN SESSION android.view.SurfaceSession@40e4bec8: pid=17791 format=4 flags=0x200 / Window{40b18590 SurfaceView paused=false}
I/DEBUG   (14692): debuggerd committing suicide to free the zombie!

Comment 6

[Matthew Gregan [:kinetik]]

Gian-Carlo and Ioana, can you please confirm that the try build at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-e1cb202d97ba/try-android/ helps?

Comment 7

[Ioana Chiorean]

(In reply to Matthew Gregan [:kinetik] from comment #6)
> Gian-Carlo and Ioana, can you please confirm that the try build at
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgregan@mozilla.
> com-e1cb202d97ba/try-android/ helps?

I've tries the build and works fine on Samsung Galaxy R with 2.3.4

Comment 8

[Matthew Gregan [:kinetik]]

Comment on attachment 826525 [details] [diff] [review]
bug933284_v0.patch

Thanks for confirming, Ioana!

nm -D on an Android 4.x libmedia.so has the same signature for getOutputSamplingRate, so this should be fine there too.

Comment 9

[Matthew Gregan [:kinetik]]

Comment on attachment 826525 [details] [diff] [review]
bug933284_v0.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 918861
User impact if declined: Crash on some Android 2.3 systems
Testing completed (on m-c, etc.): tested via try
Risk to taking this patch (and alternatives if risky): Audio will be safely disabled (with error logged to logcat) instead of either working or crashing
String or IDL/UUID changes made by this patch: none

Comment 10

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e5ad0efa2149

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e5ad0efa2149

Comment 12

[bhavana bajaj [:bajaj]]

Comment on attachment 826525 [details] [diff] [review]
bug933284_v0.patch

Requesting QA to have a look at crash-stats and help verify that the signatures disappear.

Comment 13

[Matthew Gregan [:kinetik]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8c087322d449