Switching back and forth between the music app and FM radio (using home key) causes some unexpected behavior.

VERIFIED FIXED in Firefox 21

Status

defect
P1
critical
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: nsarkar, Assigned: baku)

Tracking

unspecified
B2G C4 (2jan on)
Other
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 fixed)

Details

(Whiteboard: [cr 442632])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

6 years ago
These are the steps to reproduce the issue seen -
1. Insert wired headset to the phone.
2. Play Music. 
3. Press Home Key. Music plays on the homescreen.
4. Open FM. FM music plays.
5. Press home key and go back to music and play. Music resumes playing.
6. Again press home key in music screen.
7. Now the music doesn't play in the homescreen (as expected similar to step 3)

Expected beahvior: Music should play even after pressing home key.

Also, killing the FM app seems to fix the problem. Music starts playing in the homescreen. Seems like FM app is causing some issues here.
(Reporter)

Updated

6 years ago
Summary: Switching between music app and FM has some issues. → Switching back and forth between the music app and FM radio (using home key) causes some unexpected behavior.
(Reporter)

Updated

6 years ago
Blocks: 808607
(Reporter)

Updated

6 years ago
blocking-b2g: --- → tef?
Whiteboard: [cr 442632]
Andrea, can you have a look here?
Assignee: nobody → amarchesini
The current design will mute all of content channels if there are more then two apps with content channels are playing in the background. If the behavior is want to kept last one (fall to background) be continue to play, then we need to modify AudioChannelService.
If this is WAD then closing as wontfix seems fine.  This bug was simply raised because the observed behavour was unexpected and seems buggy
Hi Casey,

Could you help to clarify the UX behavior in this case?

Thanks.
Flags: needinfo?(kyee)
(Assignee)

Comment 5

6 years ago
(In reply to Michael Vines [:m1] from comment #3)
> If this is WAD then closing as wontfix seems fine.  This bug was simply
> raised because the observed behavour was unexpected and seems buggy

That Marco said is right. The current implementation mutes all the content channels when in background if there are more then two apps playing.

I would suggest this new behaviour:

1. Play Music. 
2. Press Home Key. Music plays on the homescreen.
3. Open FM. FM music plays.
4. Press home key. FM plays on the homescreen.
5. I stop/close/kill FM
6. No music is played
7. Open Music. Music resumes playing.

So, the idea is: a content channel plays in background but it's muted if some other app using a content channel starts playing and it doesn't resume, until it becomes visible again.
Flags: needinfo?(kyee)

Comment 6

6 years ago
(In reply to Andrea Marchesini (:baku) from comment #5)
> So, the idea is: a content channel plays in background but it's muted if
> some other app using a content channel starts playing and it doesn't resume,
> until it becomes visible again.

This sounds like a good solution and exactly what I would recommend :)


In this scenario:
> 1. Play Music. 
> 2. Press Home Key. Music plays on the homescreen.
> 3. Open FM. FM music plays.
> 4. Press home key. FM plays on the homescreen.
> 5. I stop/close/kill FM
> 6. No music is played
> 7. Open Music. Music resumes playing.

I would make it so that the user in Step 7 have to manually resume music so that it doesn't surprise the user when arriving back with music.
Flags: needinfo?(kyee)
blocking-b2g: tef? → tef+
(Assignee)

Comment 7

6 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #704962 - Flags: review?(mchen)
(Assignee)

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 704962 [details] [diff] [review]
patch

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

Thanks for taking the consideration in advance.

::: dom/audiochannel/AudioChannelService.cpp
@@ +174,5 @@
> +  // If the audio content channel is visible, let's remember this ChildID.
> +  if (newType == AUDIO_CHANNEL_INT_CONTENT &&
> +      oldType == AUDIO_CHANNEL_INT_CONTENT_HIDDEN &&
> +      !mActiveContentChildIDs.Contains(aChildID)) {
> +

As I knew that there will be only one app (also means one childID) in the foreground but it can have more then one media elements (agents). Since child id will be checked here then there is only one will be added into mActiveContentChildIDs. If this assumption is correct, do we need to use nsTArray or just an uint64_t mActiveContentChildID?

@@ +418,5 @@
>        }
> +
> +      while ((index = mActiveContentChildIDs.IndexOf(childID)) != -1) {
> +        mActiveContentChildIDs.RemoveElementAt(index);
> +      }

Even there are more then one child id in the foreground, line 205 still limits only one will be in mActiveContentChildIDs for each child id. So do we need to use while loop here?
(Assignee)

Comment 9

6 years ago
> As I knew that there will be only one app (also means one childID) in the
> foreground but it can have more then one media elements (agents). Since
> child id will be checked here then there is only one will be added into
> mActiveContentChildIDs. If this assumption is correct, do we need to use
> nsTArray or just an uint64_t mActiveContentChildID?

I think more than 1 app can be visible. The keyboard is an example.

> Even there are more then one child id in the foreground, line 205 still
> limits only one will be in mActiveContentChildIDs for each child id. So do
> we need to use while loop here?

Yes, because we add the childID just once.

BTW: I would like to use the appID instead childID, but this is a separated bug.
(Assignee)

Updated

6 years ago
Blocks: 826048
(Assignee)

Updated

6 years ago
Duplicate of this bug: 832210
(In reply to Andrea Marchesini (:baku) from comment #9)
> 
> I think more than 1 app can be visible. The keyboard is an example.
> 

Oh, right. Thanks for the reminding.

> > Even there are more then one child id in the foreground, line 205 still
> > limits only one will be in mActiveContentChildIDs for each child id. So do
> > we need to use while loop here?
> 
> Yes, because we add the childID just once.

Since childID is added just once, could we remove the while loop?
(Assignee)

Comment 12

6 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #704962 - Attachment is obsolete: true
Attachment #704962 - Flags: review?(mchen)
Attachment #705802 - Flags: review?(mchen)
Comment on attachment 705802 [details] [diff] [review]
patch

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

Looks great.
Attachment #705802 - Flags: review?(mchen) → review+
I tried landing this, but stuff has changed enough for this not to apply cleanly any more :(

Andrea, can you merge this and land on inbound and b2g18 asap?
(Assignee)

Comment 15

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c4559f5d2e

For inbound I need to have 830648 829561 830672 landed.
(Assignee)

Comment 17

6 years ago
Posted patch patchSplinter Review
The tests were not updated.
Attachment #705802 - Attachment is obsolete: true
Attachment #706381 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/5197fd796303
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → B2G C4 (2jan on)

Updated

6 years ago
Status: RESOLVED → VERIFIED

Comment 21

6 years ago
Issue no longer reproduces on the Unagi device

Build ID: 20130214070203
December 5th Kernel
Gaia: 6544fdb8dddc56f1aefe94482402488c89eeec49
Gecko http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/d1288313218e

Switching between the music app and the FM radio app is flawless
You need to log in before you can comment on or make changes to this bug.