[B2G][Gaia] The 'Play' icon in the status bar disappears when entering the Dialer app

RESOLVED FIXED in Firefox 30

Status

defect
P1
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mvaughan, Assigned: mchen)

Tracking

({regression})

unspecified
1.4 S1 (14feb)
ARM
Gonk (Firefox OS)
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox28 wontfix, firefox29 wontfix, firefox30 fixed, b2g-v1.2 unaffected, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 verified)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Posted video Video of Play icon.
Description:
The triangular, white 'Play' icon, that is displayed in the status bar at the top of the screen for music/radio, disappears when launching the Dialer app. The icon will appear again after leaving the Dialer app.

Prerequisite:
Have at least one song on the SD card.

Repro Steps:
1) Update Buri to Master (1.4) BuildID: 20140206040203
2) Launch the Music app > play a song
3) Navigate to the Dialer app

Actual:
The 'Play' icon disappears. If you leave the Dialer app, the icon will reappear.

Expected:
The 'Play' icon is present until another process needs to output audio, like a phone call.

Environmental Variables:
Device: Buri Master (1.4) MOZ RIL
BuildID: 20140206040203
Gaia: ac8a273809b5b160554e616bc5ef2d6fa026ce0e
Gecko: 1e9f169c9715
Version: 30.0a1
Firmware Version: V1.2-device.cfg

Notes:
Repro frequency: 3/3
See attached: PlayIcon.mp4
This issue reproduces on the 02/06/14 1.3 build, but does NOT reproduce on the 02/06/14 1.2 build.

Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20140206004002
Gaia: 467ef8c9145d9a57d35b0619db541d23b522b958
Gecko: a1fa925c40c2
Version: 28.0
Firmware Version: V1.2-device.cfg
QA Contact: mvaughan
This issue started reproducing on the 11/07/13 1.3 build.

- Works -
Device: Buri v1.3 MOZ RIL
BuildID: 20131106040203
Gaia: 3b5fe429f2414f2a9d7241b311b399033bb10612
Gecko: 9ba3faa35c96
Version: 28.0a1
Firmware Version: V1.2-device.cfg

- Broken -
Device: Buri v1.3 MOZ RIL
BuildID: 20131107040200
Gaia: 42bbe26a72e030faf07a6fc297f61a3a8ccda25b
Gecko: 70de5e24d79b
Version: 28.0a1
Firmware Version: V1.2-device.cfg
blocking-b2g: --- → 1.3?
Component: Gaia → Gaia::Music
Flags: needinfo?(squibblyflabbetydoo)
Flags: needinfo?(dkuo)
This is probably an audio channel issue since the dialer opens a Web Audio context for the dialer tones. I don't think it would count as blocking though, and Dominic has mentioned that we'll be rewriting the audio competing policy stuff in Gaia for 1.4 anyway.
Flags: needinfo?(squibblyflabbetydoo)
(In reply to Jim Porter (:squib) from comment #3)
> This is probably an audio channel issue since the dialer opens a Web Audio
> context for the dialer tones. I don't think it would count as blocking
> though, and Dominic has mentioned that we'll be rewriting the audio
> competing policy stuff in Gaia for 1.4 anyway.

I have to disagree - this breaks a basic flow of showing a play notification when a song is being played, as visiting one particular app causes it to not show. Think about it - why even bother having a feature like this if you aren't going to show the play notification consistently while you were within different apps? We need to be consistent here to be effective, so this needs to be fixed.
As far as I know, it only happens in the dialer, which presumably you're opening up to make a call. Once you are on the call, the icon *shouldn't* be shown (since music isn't playing). I think trying to fix this would be too risky given the current (awful) state of the audio channel code, unless we just backed out the Web Audio patch from dialer, which would happily fix this issue and probably a couple other open ones (like the glitches in the touch tone audio).
(In reply to Jim Porter (:squib) from comment #5)
> As far as I know, it only happens in the dialer, which presumably you're
> opening up to make a call. Once you are on the call, the icon *shouldn't* be
> shown (since music isn't playing). I think trying to fix this would be too
> risky given the current (awful) state of the audio channel code, unless we
> just backed out the Web Audio patch from dialer, which would happily fix
> this issue and probably a couple other open ones (like the glitches in the
> touch tone audio).

That isn't specific to the dialer app - I reproduced this with the Web Audio test app here as well (http://mozilla.github.io/qa-testcase-data/webapi/webaudio/). This is actually a general problem that can happen with any application making use of web audio while music is being played in the background - you'll lose the play notification as soon as Web Audio APIs start to be used by the app. This is going to be problematic for games (including one preinstalled partner app), as many of them make use of web audio. There is a valid use case for games where a user decides to keep music active while playing a game.
If this is web audio specific though, then my money is on that this is a problem in web audio's integration with the audio competing policy, not the audio competing policy itself.
Component: Gaia::Music → Web Audio
Product: Firefox OS → Core
Version: unspecified → 28 Branch
(In reply to Jason Smith [:jsmith] from comment #7)
> If this is web audio specific though, then my money is on that this is a
> problem in web audio's integration with the audio competing policy, not the
> audio competing policy itself.

It's possible, although the old way (mozAudio) was special-cased in Gecko in a bunch of places, and the behavior might not be exactly the same, although I think it tried to be.
Rob, Karl -- This is about to become a 1.3 blocker.  It was just moved to "web audio" this morning.  Can one of you take a look and provide feedback?  Is this a web audio problem?  What are our options for fixing it (given that we now have a tight timeline for v1.3)?  Thanks.
Flags: needinfo?(roc)
Flags: needinfo?(karlt)
Priority: -- → P1
Clearing needinfo on dkuo as I don't think this is a front-end bug
Flags: needinfo?(dkuo)
This is almost certainly an audio channel issue. Neither Karl nor I have done any work on that.
Flags: needinfo?(roc)
Flags: needinfo?(karlt)
Flags: needinfo?(amarchesini)
Thanks, Rob.  I'll change the component.
Component: Web Audio → AudioChannel
Product: Core → Firefox OS
Version: 28 Branch → unspecified
blocking-b2g: 1.3? → 1.3+
also needinfo? Marco
Flags: needinfo?(mchen)
Yeah, as :roc mentions, this is probably most closely related to audio channels, although it's curious that this wasn't an issue until we switched from mozAudio to WebAudio, even though both respect audio channels. We should figure out what the differences are. I think the specific issue is caused by the dialer creating an audio context for the touch tones, which confuses the play icon (I believe the play icon listens for changes in the current audio channel). It's strange that the audio channels are smart enough to keep playing the music, but not smart enough to inform the play icon of what channel is playing audio. I could be mistaken on this, though.

There are some riskier options for this, like tying the play icon to the "now playing" notification in the notifications area, but that would regress us in other ways, since we wouldn't get the play icon when you're listening to FM radio, for instance.

Maybe in the long term, it will be easier to handle all of this when the audio competing policy code is in Gaia instead of Gecko.
This is an issue on AudioChannelService. 
http://mxr.mozilla.org/mozilla-central/source/dom/audiochannel/AudioChannelService.cpp#485

System app depends on the mozChromeEvent - "audio-channel-changed" fired by AudioChannelService to notify the most higher and playing channel. The current logic will ignore background audio if there is at least an one on foreground.
This is why dialer app will have normal channel associated with Web Audio then music app with content channel in the background will be ignored.

Will submit a patch later.
Flags: needinfo?(mchen)
Posted patch Patch v1 (obsolete) — Splinter Review
Hi Andrea,

Currently we have two mozChromeEvent for system app to get audio status. audio-channel-change & visible-audio-channel-changed.

Apparently the visible one should take care foreground app only but the other didn't count in background's audio when there is an audio on the foreground.

So this patch just try to find out the highest channel among foreground and background audio.

Thanks.
Attachment #8374673 - Flags: review?(amarchesini)
Attachment #8374673 - Attachment is patch: true
Assignee: nobody → mchen
Comment on attachment 8374673 [details] [diff] [review]
Patch v1

Review of attachment 8374673 [details] [diff] [review]:
-----------------------------------------------------------------

With this patch, is there is something playing, with default audio Channel, we emit 'none'. This seems wrong. Right?
Attachment #8374673 - Flags: review?(amarchesini) → review-
(In reply to Andrea Marchesini (:baku) from comment #17)
> Comment on attachment 8374673 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 8374673 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> With this patch, is there is something playing, with default audio Channel,
> we emit 'none'. This seems wrong. Right?

Hi,

We didn't allow web content to use "default" and the real default value of audiochanneltype is normal.
And we invalidate the value of audio channel type in AudioChannelAgent.cpp, there didn't allow "AUDIO_CHANNEL_DEFAULT". So may I know which case will hit this problem?

By the way, the AUDIO_CHANNEL_DEFAULT is designed to be used by AudioChannelManager::SetVolumeControlChannel(...) and will be used inside the Gecko not expose to Web content.
> We didn't allow web content to use "default" and the real default value of
> audiochanneltype is normal.
> And we invalidate the value of audio channel type in AudioChannelAgent.cpp,
> there didn't allow "AUDIO_CHANNEL_DEFAULT". So may I know which case will
> hit this problem?

OK. Can you write a comment to explain how this works in detail? Thanks.
Flags: needinfo?(amarchesini)
(In reply to Andrea Marchesini (:baku) from comment #19)
> OK. Can you write a comment to explain how this works in detail? Thanks.

Ok, of course.

The audio-agent-change event should report the highest audio channel type and count in the audio in both foreground and background.
The visible-audio-agent-change event is similar but count in foreground only.

audio-agent-change:
The current code will stop to check background ones when it found there is any audio in the foreground.
This is wrong. ex: fg: one normal bg: one content -> it reports normal.

The patch will check background ones as well but limit to check the channel type higher then foreground one or check types higher then normal if nothing in the foreground.
ex: fg: one normal bg: one content -> it reports content.
Flags: needinfo?(amarchesini)
Attachment #8374673 - Flags: review- → review+
Flags: needinfo?(amarchesini)
Add reviewer's name into commit comment.
Attachment #8374673 - Attachment is obsolete: true
Attachment #8377051 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/686c461c61aa
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S1 (14feb)
Please request approval-mozilla-b2g28 on the patch for uplift to v1.3.
Flags: needinfo?(ryanvm)
Hi Ryan,

is not a V1.3+ flag enough to uplift? Thanks.
(In reply to Marco Chen [:mchen] from comment #26)
> Hi Ryan,
> 
> is not a V1.3+ flag enough to uplift? Thanks.

Nope. You need approval-mozilla-b2g28 as well now.

See https://wiki.mozilla.org/Release_Management/B2G_Landing#Blocker.2FApproval_Queries_4.
Flags: needinfo?(ryanvm)
Comment on attachment 8377051 [details] [diff] [review]
Patch : Checkin Version

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 828283. It tried to add a new visible-audio-agent-change then introduce this one.
User impact if declined: User will be confused about whether music app is on playing or not when any web audio is triggered.
Testing completed: Manually testing only.
Risk to taking this patch (and alternatives if risky): I think it is low enough and this patch just recover the function not introduce new one.
String or UUID changes made by this patch:
Attachment #8377051 - Flags: approval-mozilla-b2g28?
Adding verifyme for QA to help with verification on 1.4
Keywords: verifyme
Verified it's fixed on the latest master build.
The "play" icon doesn't disappear anymore when open the "Dialer" app

Device: Buri 1.4 MOZ
BuildID: 20140219040204
Gaia: ac06cfbd2baf6494ffbb668cc599e3892cd5e17b
Gecko: bf0e76f2a7d4
Version: 30.0a1
Firmware Version: v1.2-device.cfg
Keywords: verifyme
Attachment #8377051 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Hi Andrea,

I am not sure whether I should open a new bug for this test case or not so I put it here first.

And please help to review this test case for mozChromeEvent - audio-channel-change/visible-audio-channel-change. And the rough idea is

  1. Use SystemProxy.jsm to relay mozChromeEvent from parent to child process.
     (because mochitest run on emulator is on child process).

  2. file_audio.html is another OOP process which is responsible for playing each types of audio channel then stopping them one by one.

  3. test_audioChannelChange.html tried to launch file_audio.html and control it's visible status so we can verify audio playing in the foreground or background or both.

Thanks.
Attachment #8407406 - Flags: review?(amarchesini)
Comment on attachment 8407406 [details] [diff] [review]
TestCaseforAudioChannelChangeEvent v1

Review of attachment 8407406 [details] [diff] [review]:
-----------------------------------------------------------------

Cancel the review because I found one issue on AudioChannelService so open a new bug for it.
Attachment #8407406 - Flags: review?(amarchesini)
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.