776069 - Audio played from content process (main thread) causing dialer to hang

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Dave Hylands [:dhylands]]

If I run the Dialer App OOP, it seems to be completely unresponsive to touches. I can't enter a phone number or otherwise get the UI to respond in any way.

Comment 1

[Dave Hylands [:dhylands]]

All core apps need to run OOP

Comment 2

[Dave Hylands [:dhylands]]

I tried again on the latest codebase, and this particular problem seems to be resolved.

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Dave Hylands [:dhylands] from comment #2)
> I tried again on the latest codebase, and this particular problem seems to
> be resolved.

It didn't work for me, neither on otoro nor on sgs2-ics.


gaia commit: f7c95943b9bf49dcd52c8c336dc1f93717ebd7d2
(I marked 'Dialer' in the OOP blacklist)
gecko commit: f7c95943b9bf49dcd52c8c336dc1f93717ebd7d2

Comment 4

[Dave Hylands [:dhylands]]

I tried again on my otoro, and it seems to still be broken. Maybe I wasn't really testing OOP previously. So I'm re-opening

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

I found that the main cause seems resulting from "nsHTMLAudioElement::MozWriteAudio()", because |mAudioStream->Write()| [1] does not return. So the app is stuck when calling 'this._audio.mozWriteAudio(soundData);' in "keypad.js" [2]. 


[1] http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLAudioElement.cpp#172
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/dialer/js/keypad.js#L51

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Thanks for diagnosing, Hsin-Yi!

Comment 7

[Matthew Gregan [:kinetik]]

MozWriteAudio uses nsAudioStream::Available to ensure writes are non-blocking.  It turns out that Available() isn't implemented correctly on nsRemotedAudioStream and always returns a fixed size: https://mxr.mozilla.org/mozilla-central/source/content/media/nsAudioStream.cpp#697

This wasn't obvious in the past because nsRemotedAudioStream::Write was broken in such a way that it never blocked; that was fixed by bug 695612 was fixed.

A high level question: we needed this audio remoting to work around the fact that JNI wasn't available in content processes in Android XUL.  Since we're using native audio APIs on B2G, can we just disable this audio remoting code altogether?

Comment 8

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Not entirely, but that was part of it.

We can do that, but
 - it requires giving content processes access to a larger surface area of system resources
 - we want to do mixing in gecko anyway eventually, which would use this (well, a successor) remoting code

Comment 9

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Attachment: Disable remote audio for gonk. Sigh

With this patch, OOP dialer works.  Remote audio badly needs a rewrite, but I'm not sure anyone has bandwidth for that for v1.  Since it seems too broken, we'll need to trade some security for functionality.

But audio played from browser tabs *still* doesn't work.  Argh!  http://people.mozilla.com/~cjones/media.html

Comment 10

[Matthew Gregan [:kinetik]]

Comment on attachment 653115 [details] [diff] [review]
Disable remote audio for gonk. Sigh

We *could* implement remote Available() instead, but I just don't know what else we're going to uncover, and rewriting this the sane way seems like the best use of effort.

Comment 11

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I agree.

Comment 12

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

https://hg.mozilla.org/integration/mozilla-inbound/rev/41e3045d37b0

Comment 13

[Thinker Li [:sinker]]

(In reply to Matthew Gregan [:kinetik] from comment #10)
> Comment on attachment 653115 [details] [diff] [review]
> Disable remote audio for gonk. Sigh
> 
> We *could* implement remote Available() instead, but I just don't know what
> else we're going to uncover, and rewriting this the sane way seems like the
> best use of effort.

I think we can keep a counter for RemotedAudioStream to track how many AudioWriteEvents are not acked.  Once it exceeds a threshold, Available() always return 0.  I guess the threshold is 1.

Comment 14

[Thinker Li [:sinker]]

(In reply to Thinker Li [:sinker] from comment #13)
> (In reply to Matthew Gregan [:kinetik] from comment #10)
> > Comment on attachment 653115 [details] [diff] [review]
> > Disable remote audio for gonk. Sigh
> > 
> > We *could* implement remote Available() instead, but I just don't know what
> > else we're going to uncover, and rewriting this the sane way seems like the
> > best use of effort.
> 
> I think we can keep a counter for RemotedAudioStream to track how many
> AudioWriteEvents are not acked.  Once it exceeds a threshold, Available()
> always return 0.  I guess the threshold is 1.

I have missed something. We should also stop blocking calling Write() if the counter does not exceeds the threshold.  It is an easy and fast way to resolve the problem in the front of us.

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/41e3045d37b0