Closed Bug 1218593 Opened 4 years ago Closed 4 years ago

Dialer touch tones sounds intermittently stop playing audio in Dialer.

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
mozilla45
blocking-b2g 2.5+
Tracking Status
firefox45 --- fixed
b2g-v2.2 --- unaffected
b2g-master --- affected

People

(Reporter: Marty, Assigned: alwu)

References

()

Details

(Keywords: regression, Whiteboard: [2.5-Daily-Testing][Spark])

Attachments

(3 files)

Description:
When using the Dialer app, touching keys on the keypad will intermittently stop playing the audible touch tones.  This generally seems to occur after the user types several keys, then deletes one or more numbers, and begins typing again, or after waiting a few seconds and typing numbers again.

Note: This is distinct from the behavior addressed and fixed in bug 1183033, which would only ever play the first touch tone.
Multiple touch tones can be heard, but will stop intermittently.

Repro Steps:
1) Update a Aries to 20151026111709
2) Open the dialer app
3) Type in several numbers into dialer
4) Delete the numbers input into dialer
5) Wait several seconds
6) Repeat steps 3-5 several times

Actual:
Touch tone sounds will intermittently stop playing audio

Expected:
Touch tone sounds will always properly play audio.

Environmental Variables:
Device: Aries 2.5
Build ID: 20151026111709
Gaia: a677ddd3aa3a81058775938bd56008d96dbc78b0
Gecko: 5ca03a00d26823ce91ee0eaa2937bed605bd53c1
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 44.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Repro frequency: 
See attached: Logcat
This issue DOES occur on the latest Flame 2.5 Master build.
Touch tone sounds will intermittently stop playing audio

Environmental Variables:
Device: Flame 2.5
BuildID: 20151026030217
Gaia: a677ddd3aa3a81058775938bd56008d96dbc78b0
Gecko: 5ca03a00d26823ce91ee0eaa2937bed605bd53c1
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 44.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

*********************************************************

This issue does NOT occur on Flame 2.2 builds
Touch tone sounds will always properly play audio.

Environmental Variables:
Device: Flame 2.2
BuildID: 20151026032516
Gaia: 885647d92208fb67574ced44004ab2f29d23cb45
Gecko: 9e0aa88fea7a
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
[Blocking Requested - why for this release]:

This is a regression. Dial tones should always work properly.
blocking-b2g: --- → 2.5?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
QA Contact: jthomas
blocking-b2g: 2.5? → 2.5+
Priority: -- → P2
Currently I am unable to provide a window on this issue. After going back far enough I am blocked by Bug 1201133. There is window for that in the 4th comment on that bug. https://bugzilla.mozilla.org/show_bug.cgi?id=1201133#c4. 

Here the first broken I found regarding this bug about the disappearing of the touch tone sounds after pressing several keypad numbers.

Environmental Variables: [First Affected by this bug's issue]
Device: Flame 2.5
BuildID: 20151007103239
Gaia: 77d463a009a1425e413edaae92b237e116708560
Gecko: 49d87bbe0122d894c8e45f0b409c42dfe1c36737
Version: 44.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
Andrea can you take a look at this issue.  Please see comment 3.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado) → needinfo?(amarchesini)
After reproduced the bug, I found out the sound cannot be played when gaia does `channel.setMuted(false)`.

Looks like it is a gecko issue.
Component: Gaia::System::Audio Mgmt → AudioChannel
I supposed bug 1183033 can fix this.
Assignee: nobody → alwu
After preliminary analysis, the problem seems in the MSG.
The audio channel status of the AudioDestinationNode is correct.
I'll investigate more details.
The reason of no dialer sound is because the MSG switches to the "system clock driver". This behavior would turn off the Android's audio track.

Still finding the reason why we switched the graph driver at that time.
Component: AudioChannel → Audio/Video: MSG/cubeb/GMP
Product: Firefox OS → Core
From the log in comment8, we can see that all streams are removed at the same time.
Therefore, we have no audio stream in MSG, so we change to the "system clock driver".
Assignee: alwu → nobody
Assignee: nobody → alwu
Here is the problem : why we don't switch back to the AudioCallBackDriver when the dialer sound is created again?
The reason we didn't switch back to the AudioCallBackDriver is because the MediaStream didn't have the AudioOutput, and then we cancel our checking [1].

However, I found something very weird, that is even we can heard the dialer sound, the MediaStream still doesn't have the AudioOutput! The AudioOutput would be destroyed in soon after we created AudioDestinationNode.


[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaStreamGraph.cpp#579
The problem is that we got fail [1], so we can't switch back to the AudioCallbakcDriver.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaStreamGraph.cpp#583
[Bug analysis]

1. Play dialer sound
When we click a dialer button, we used the AudioContext to generate sound, and then the MediaStreams was added to the MSG. 


2. Switch to the SystemClockDriver
After a long while, the MediaStreams would be removed from the MSG (it might be removed by G.C.). 
However, the MediaStream with the AudioOutput doesn't be removed, it would be suspended.
When there was no streams, we would switch to the SystemClockDriver.

3. Click dialer button again
The MediaStream with the AudioOutput would be resumed.
Since it have already had the AudioOutputStream, and also got FALSE by GetAndResetTracksDirty(), we got the fail at [1], and we didn't switch back to the AudioCallbackDriver.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaStreamGraph.cpp#583
Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again
Comment on attachment 8683023 [details]
MozReview Request: Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again

Hi, Paul,
Sorry to bother you,
Could you give me some suggestions about this patch?
The analysis is in the comment 13.
Very appreciate!!
Attachment #8683023 - Flags: feedback?(padenot)
This sounds like bug 1216417. It would be great if you can test and verify Alastor.
Flags: needinfo?(alwu)
After discuss with Andreas, it's different issue.
I'm looking for the better solution.
Flags: needinfo?(alwu)
Bug 1216417 was about not keeping MSG's AudioOutputStreams in sync with an MSG-MediaStream's AudioOutputs.

This on the other hand is an issue where we fail to switch to the AudioCallbackDriver after Resume() on an AudioContext. There's code to switch to SystemCallbackDriver after Suspend() but not the inverse.
Hmm, we were looking at [1] for the switching to SystemCallbackDriver after Suspend, but the code for switching back to AudioCallbackDriver is actually at [2]. Perhaps there's an ordering issue with these events? Well, that's just guessing.


[1] https://dxr.mozilla.org/mozilla-central/rev/6077f51254c69a1e14e1b61acba4af451bf1783e/dom/media/MediaStreamGraph.cpp#352
[2] https://dxr.mozilla.org/mozilla-central/rev/6077f51254c69a1e14e1b61acba4af451bf1783e/dom/media/MediaStreamGraph.cpp#3013
Attachment #8683023 - Flags: feedback?(padenot)
The reason is that the stream was suspended/resumed by calling MediaStream::Suspend()/Resume() directly, instead of calling the AudioContext's functions.

The cycle collection would suspend the |mStream| of the AudioContext [1]. When we add the new node, |mStream| would be resumed [2].

However, in this case, we doesn't have any checking for switching back to the AudioCallbackDriver.

Therefore, we need to check this situation in UpdateStreamOrder().

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/webaudio/AudioDestinationNode.cpp#715
[2] https://dxr.mozilla.org/mozilla-central/source/dom/media/webaudio/AudioDestinationNode.cpp#723
Flags: needinfo?(amarchesini)
Comment on attachment 8683023 [details]
MozReview Request: Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24181/diff/1-2/
Attachment #8683023 - Flags: review?(padenot)
Hi, Paul,
Could you help me review this patch?
The analysis is on the comment20.
Very appreciate!
Attachment #8683023 - Flags: review?(padenot) → review+
Comment on attachment 8683023 [details]
MozReview Request: Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again

https://reviewboard.mozilla.org/r/24181/#review21879
Ah, we get the test crash, need to figure out the problem.
Comment on attachment 8683023 [details]
MozReview Request: Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24181/diff/2-3/
Attachment #8683023 - Flags: review+ → review?(padenot)
Hi, Paul,
Could you help me review this patch again?
I added one condition checking to ensure that the driver is not preparing to switch.
This change can fix the test fail.
Very appreciate!
Attachment #8683023 - Flags: review?(padenot) → review+
Comment on attachment 8683023 [details]
MozReview Request: Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again

https://reviewboard.mozilla.org/r/24181/#review22011
Thanks Paul!

--

Try-server result is in the comment26.
Keywords: checkin-needed
Comment on attachment 8683023 [details]
MozReview Request: Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24181/diff/3-4/
Attachment #8683023 - Flags: review+ → review?(padenot)
Hi, Paul,
Could you help me review this patch again?
I add one more checking condition, As we would only use the SystemClock/AudioCallbackDriver on the RealTimeAudioContext.
Very appreciate!!
Flags: needinfo?(alwu)
Comment on attachment 8683023 [details]
MozReview Request: Bug 1218593 - switch back to the AudioCallbackDriver when there is new audio again

https://reviewboard.mozilla.org/r/24181/#review22197
Attachment #8683023 - Flags: review?(padenot) → review+
Here is the try-server result for all platform.
Although we have two fails, but I think it's not related with my changing.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba6b36b93c72
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7cd6a77d330c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Depends on: 1234230
Duplicate of this bug: 1253578
Duplicate of this bug: 1239890
You need to log in before you can comment on or make changes to this bug.