1018825 - [System][Music]Status bar play icon is still visible when music is no longer playing.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[ranjith253]

Attachment: Issue_image.jpg

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140410030200

Steps to reproduce:

1.Go to music app -> play a song ->press home key
2.Open video app -> play a video -> press home key.

After above two steps both Music & video are paused.


Actual results:

Check on the status bar play icon is still visible even if the music play-back is in pause state.


Expected results:

When music is in pause state , play icon should disappear from status bar.

Comment 1

[Sharaf]

Dear Wayne,

Can you please help us in identifying the correct person for looking into this issue?

Thanks
Sharaf

Comment 2

[Seil Park]

I think this issue is related with audio channel. Music is automatically resumed after click music icon.

Comment 3

[Wayne Chang [:wchang]]

Dominic, can you check this behaviour first?
By design do we show the icon if music is paused?

Comment 4

[Seil Park]

Can anybody check this behaviour is happend in ver 2.0? 
We are preparing for v2.0. If this case is fixed in v2.0, That's no problem.

Comment 5

[Dominic Kuo [:dkuo]]

I think this should be an issue because apparently music and video are both paused/interrupted, the play icon should disappear if there is no active sounds playing.

How we show/hide the play icon in status bar is let system app listen to the |audio-channel-changed| mozChromeEvent then decide whether to show it or not when it's |content| channel, and because both music and video use content type, so there is no audio-channel-changed mozChromeEvent(or it did fired but still content channel).

This might be an issue under audio channel cause when the active content channel is paused(video), although there is an interrupted content channel is the background(music), it should still fire |audio-channel-changed|, like |none| which make more sense and could probably solve this problem.

Marco, do you know anyone can help on this since Start is PTOing?

Comment 6

[Marco Chen [:mchen]]

I found it is a gecko bug and related to [1].

There is a speical check for content channel at [1] so we should skip the normal check below.
Will find some time to submit the patch.

[1] http://dxr.mozilla.org/mozilla-central/source/dom/audiochannel/AudioChannelService.cpp?from=audiochannelservice.cpp&case=true#549

Comment 7

[Preeti Raghunath(:Preeti)]

UX,

Please check if this behavior is per design or needs a change.

Comment 8

[Stephany Wilkes]

Flagging Jacqueline on expected music behavior.

Comment 9

[Jacqueline Savory [:jsavory] UX]

Dominic is correct - if there is no active sounds playing the pause icon should not appear. 

The expected result of this bug is the intended behaviour in this case, the play icon should disappear after pausing both music and video.

Comment 10

[Sam Lin[:neopleo]]

Hi,

Is this bug currently taken by anyone ?

Comment 11

[Wayne Chang [:wchang]]

Nominating this - according to UX feedback the current behaviour is incorrect.

(In reply to jsavory from comment #9)
> Dominic is correct - if there is no active sounds playing the pause icon
> should not appear. 
> 
> The expected result of this bug is the intended behaviour in this case, the
> play icon should disappear after pausing both music and video.

Comment 12

[Dominic Kuo [:dkuo]]

Looks like Marco has found the root cause and should be able to fix this issue. Marco, please take this one or assign to someone you think is available to fix this, thanks.

Comment 13

[Marco Chen [:mchen]]

Already asked Star to help this issue and thanks for Eric's team.

Comment 14

[bhavana bajaj [:bajaj]]

Adding qawanted to check on 1.3 to see if this is a regression.

Comment 15

[Pi Wei Cheng [:piwei] (inactive)]

(In reply to bhavana bajaj [:bajaj] [NOT reading Bugmail, needInfo please] from comment #14)
> Adding qawanted to check on 1.3 to see if this is a regression.

This issue occurs on Flame 1.3 base image v122. It's NOT a regression.
Behavior: If music is interrupted and paused by Video app, it still displays a 'play' icon on status bar even though music is no longer playing.

Device: Flame
Build ID: 20140616171114
Gaia: e1b7152715072d27e0880cdc6b637f82fa42bf4e
Gecko: e181a36ebafaa24e5390db9f597313406edfc794
Version: 28.0 (1.3)
Firmware Version: B1TC00011220
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0


I also did a quick branch check so I could set tracking flags accordingly.
Issue reproduces on Buri 2.0, Flame 2.1 master, and Flame 1.4.

Comment 16

[Star Cheng [:scheng]]

(In reply to Marco Chen [:mchen] (Workshop with Partner 6/16 ~ 6/20) from comment #13)
> Already asked Star to help this issue and thanks for Eric's team.

I will deal with this issue. Thanks for Marco's analysis.

Comment 17

[bhavana bajaj [:bajaj]]

(In reply to Wayne Chang [:wchang] from comment #11)
> Nominating this - according to UX feedback the current behaviour is
> incorrect.
> 
> (In reply to jsavory from comment #9)
> > Dominic is correct - if there is no active sounds playing the pause icon
> > should not appear. 
> > 
> > The expected result of this bug is the intended behaviour in this case, the
> > play icon should disappear after pausing both music and video.

Check QA results below, we've not blocked a release on this issue and have shipped them..So this does not qualify as a blocker, if folks have a low risk fix to uplift before July 21 we can take it.

Comment 18

[Star Cheng [:scheng]]

Attachment: Bug-1018825-fix.patch

To fix the special check for the content channel.

Comment 20

[Marco Chen [:mchen]]

Comment on attachment 8462309 [details] [diff] [review]
Bug-1018825-fix.patch

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

It looks good and only one suggeston as below.

::: dom/audiochannel/AudioChannelService.cpp
@@ +596,5 @@
> +        }
> +      } else {
> +        higher = kMozAudioChannelAttributeTable[index].value;
> +        break;
> +      }

We can "continue" the case of empty earlier then saving one if-else level.

if (mChannelCounters[index * 2 + 1].IsEmpty()) {
  continue;
}

if (kMozAudioChannelAttributeTable[index].value == (int16_t)AudioChannel::Content) {
  if (mPlayableHiddenContentChildID != CONTENT_PROCESS_ID_UNKNOWN) {
    higher = kMozAudioChannelAttributeTable[index].value;
    break;
  }
} else {
  higher = kMozAudioChannelAttributeTable[index].value;
  break;
}

Comment 21

[Star Cheng [:scheng]]

Attachment: Bug-1018825-fix_v2.patch

1.To modify the code according to Marco's suggestation of comment 20.
2.TPBL result: https://tbpl.mozilla.org/?tree=Try&rev=ac440b3d06ba
3.add reviewer

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/6ee4133fd646

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6ee4133fd646

Comment 25

[jingmei.zhang]

hi Star,could the patch be applied in 1.4?

Comment 26

[jingmei.zhang]

Attachment: patch_for_1.4.patch

Hi Star,
As the patch have not applied in 1.4,I have filed a patch for 1.4 according to the patch you provided in 2.1.
I am a newer for gecko audio,could you please help to review the patch??

Thank you so much!!