1059679 - [Midori][Settings][BT][Music][Camera]BT headset can not play music when do some operates

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: AudioChannel, defect, P1)

Description

[sync-1]

Firefox OS v1.3
 
 Mozilla build ID:20140422024003
 
 DEFECT DESCRIPTION:
 BT headset can not play music when do some operates
 
  REPRODUCING PROCEDURES:
 1.play music by BT headset
 2.enter into camera,take a video,click home button
 3.click BT headset "play"button
 4.music can not play--K.O
 
  EXPECTED BEHAVIOUR:
 K.O--music paly normally when click BT headset play button
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:medium
 
  REPRODUCING RATE:5/5

Comment 1

[sync-1]

Attachment: jrdlog

Comment 2

[hanp]

[Blocking Requested - why for this release]:

Comment 3

[bhavana bajaj [:bajaj]]

(In reply to hanp from comment #2)
> [Blocking Requested - why for this release]:

Can you please try a 2.0 cset and see if you can repro this ? The build you are trying is really old.

Comment 4

[hanp]

(In reply to bhavana bajaj [:bajaj] from comment #3)
> (In reply to hanp from comment #2)
> > [Blocking Requested - why for this release]:
> 
> Can you please try a 2.0 cset and see if you can repro this ? The build you
> are trying is really old.

I used Fire S(with original fxos 2.0).
Well, the music must be played at first, and while camera is opened, the music is still keep playing
then start recording.

if you check with the music application, you'll find the "mozinterruptbegin" message received, and when you click "Home", the "mozinterruptend" message is never received until another audiochannel change happens.

Comment 5

[hanp]

(In reply to bhavana bajaj [:bajaj] from comment #3)
> (In reply to hanp from comment #2)
> > [Blocking Requested - why for this release]:
> 
> Can you please try a 2.0 cset and see if you can repro this ? The build you
> are trying is really old.

To make it more clear, this bug is found on fxos 1.3 at first, but I've already reproduced it on fxos 2.0, and then cloned this bug here. Sorry to make you confused.

Comment 6

[Jack Liu]

[Blocking Requested - why for this release]: Set blocking flag again because this reproduced on both 1.3 & 2.0 with 100% reproduce rate.

2.0 moz build ID: 20140810000201

Comment 7

[bhavana bajaj [:bajaj]]

THanks, adding qawanted for branch checks.

Comment 8

[Adam Alldredge [:AdamA][Inactive]]

This issue is occurring on 2.2 Flame, 2.2 Open_C, 2.1 Flame(see notes), 2.0 Flame, and 1.4 Flame.

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20140903115424
Gaia: af04d8bc2111d4ea146239a89ff602206b85eaf5
Gecko: 117271830c4d
Version: 35.0a1 (2.2 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Device: Open_C 2.2 Master
BuildID: 20140903115424
Gaia: af04d8bc2111d4ea146239a89ff602206b85eaf5
Gecko: 117271830c4d
Version: 35.0a1 (2.2 Master)
Firmware: P821A10v1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Device: Flame 2.1
BuildID: 20140903085050
Gaia: fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko: 31dad821234e
Version: 34.0a2 (2.1)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Device: Flame 2.0
BuildID: 20140903075252
Gaia: d056733f8a7a1a152f5458b323f092c47dbffa48
Gecko: 742cb642750f
Version: 32.0 (2.0)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 1.4
BuildID: 20140903033752
Gaia: 2ee5b00bfbb8a67a967094804390b4afce8ecf54
Gecko: f5a75c0dd74e
Version: 30.0 (1.4) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Result:
Music does not resume after taking a video then pressing the headset "play" button.

Notes:
On 2.1 the music will resume if the play button is pressed a second time. On the first press it does not play.

Comment 9

[Jack Liu]

Dears,

It blocks both 1.3 & 2.0, especially maintenance release for shipped 1.3 product, please help to fix it urgently.

Thanks a lot.

Comment 10

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Also agree to nominate this one for 2.0+, We have waived similar issue in 1.3 branch, fixing this one in 2.0 at least give partner a chance to cherry-pick the patch into their own 1.3 branch for maintenance release

Thanks

Vance

Comment 11

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

ni Shawn Huang for BT relevant issue

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

This does not look like Bluetooth problem to me. Ni? Audio owner to comment.

Comment 13

[Star Cheng [:scheng]]

The behavior should be the comment 4 of bug 1009286 (please see the following discretion)[1]. To flash v2.0 SW, the music still play in background, so STR of comment 0 can *not* duplicate it.

1) Launch the Music App and select a music to playback
2) Minimize the Music App and launch the Camera
3) Take a picture and observe the volume of music app
4) The Music app is decreasing 75% volume after the shutter sound, and then resume.

It should not be stopped the background music while entering camera app. Set regressionwindow-wanted mark to find the correct version. 


[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1009286#c4

Comment 14

[Jason Smith [:jsmith]]

This is present on 1.3, which means this isn't a regression between the last production release certified. We don't do regression windows farther back than the last certified production release, so striking the keyword.

Comment 15

[Star Cheng [:scheng]]

Hi, Sotaro

I found the patch of bug 1058452 cause to stop background music while launching camera preview. I am working on finding the root cause. Do you have any suggestion about this issue?

Comment 16

[Sotaro Ikeda [:sotaro]]

(In reply to Star Cheng [:scheng] from comment #15)
> Hi, Sotaro
> 
> I found the patch of bug 1058452 cause to stop background music while
> launching camera preview. I am working on finding the root cause. Do you
> have any suggestion about this issue?

The regression caused by bug 1058452 is different from this bug. You might want to create a new bug to handle the regression.

bug 1058452 connect the event delivery from CameraPreviewMediaStream to HTMLMediaElement and update the HTMLMediaElement's state. It end up to call "mAudioChannelAgent->StartPlaying(&canPlay)" in HTMLMediaElement::UpdateAudioChannelPlayingState(). Then audio channel is occupied by the HTMLMediaElement.

Comment 17

[Star Cheng [:scheng]]

(In reply to Sotaro Ikeda [:sotaro] from comment #16)
> (In reply to Star Cheng [:scheng] from comment #15)
> > Hi, Sotaro
> > 
> > I found the patch of bug 1058452 cause to stop background music while
> > launching camera preview. I am working on finding the root cause. Do you
> > have any suggestion about this issue?
> 
> The regression caused by bug 1058452 is different from this bug. You might
> want to create a new bug to handle the regression.
> 

Hi, Sotaro

Ok! I have filed a bug to handle the regression. Please see the Bug 1069109.

> bug 1058452 connect the event delivery from CameraPreviewMediaStream to
> HTMLMediaElement and update the HTMLMediaElement's state. It end up to call
> "mAudioChannelAgent->StartPlaying(&canPlay)" in
> HTMLMediaElement::UpdateAudioChannelPlayingState(). Then audio channel is
> occupied by the HTMLMediaElement.

Thanks for your explain.

Comment 18

[Star Cheng [:scheng]]

This's a regression issue. The behavior is as followings instead of STR of comment 0: 

REPRODUCING PROCEDURES:
 1.play music and press home key (maybe the device is connected to BT headset)
 2.enter into camera, the music still play in background

Comment 19

[Sri Kasetti]

Its unclear to me what needs to be fixed here?
Comment 18 and comment 0 seem to be indicating two different behaviors.

Comment 20

[hanp]

Well, Kasetti, I think comment 18 is an alternative way to solve problem described in comment 0,
But if possible, we hope comment 0 to be solved directly. Thanks.

Comment 21

[Star Cheng [:scheng]]

The behavior of comment 0 is not correct. It is not the expected behavior. It's a regression bug.
If we fix the regression, the behavior will be normal and there is no STR of comment 0.

Comment 22

[Jack Liu]

Dears,

Thanks to provide a patch and TCL QA team will do regression test on relevant bugs.

In best case all bugs will be fixed, but there are some complicated STR has BT and voice call involved.

Any way, it's appreciated that this blocking issue be fixed.

Comment 23

[Jim Porter (:squib)]

(In reply to Star Cheng [:scheng] from comment #18)
> 
> This's a regression issue. The behavior is as followings instead of STR of
> comment 0: 
> 
> REPRODUCING PROCEDURES:
>  1.play music and press home key (maybe the device is connected to BT
> headset)
>  2.enter into camera, the music still play in background

I'm not sure why we'd expect that behavior. It seems to me that the music *should* still play in the background.

Comment 24

[Hema Koka [:hema]]

There is confusion on this bug with different STR in comment 0 and comment 18. 

But looking at the Adam's test results in comment 8, this seems to be an issue with audio competing code. Moving it to AudioChannel so we can get help from the right folks to investigate what is going on. 

Thanks
Hema

Comment 25

[Star Cheng [:scheng]]

The expected behavior is described in bug 1009286#c4. I suggest we can focus on bug 1009286 instead this bug. The STR of comment 0 is caused by the regression bug 1058452 and the STR in *not* expected behavior. 

At first, we have to resolve the regression bug 1058452 and I have created a bug 1069109 to track it. 
After resolving bug 1069109, we can check whether "click BT headset play button" works or not. If it doesn't work, it is another bug.

Comment 26

[hanp]

Sirs, in #comment 24, Hema is right about the root of this PR, 
we expect the audio cometing code to do things right.
But as we tested, if you operate exactly as the #comment 0 says,
the "mozinterruptend" message will not be dispatched from gecko side when camera quited. and that's the problem

Comment 27

[Wayne Chang [:wchang]]

Blocking base on comment 0 description - high user impact

Comment 28

[Eric Chou [:ericchou] [:echou]]

Star, please comment about the next steps since bug 1058452 and bug 1069109 have both been fixed.

Comment 29

[Star Cheng [:scheng]]

I flashed the latest SW on Flame device. I can *NOT* duplicate this issue because the bug 1069109 is resolved. The STR of comment 0 is wrong behavior and bug 1069109 can resolve this wrong behavior. 


Hi, Sync-1

Could you duplicate this issue with latest SW?

Comment 30

[Eric Chou [:ericchou] [:echou]]

(In reply to Wayne Chang [:wchang] from comment #27)
> Blocking base on comment 0 description - high user impact

Wayne,

According to comment 29, we think that the symptom described in comment 0 doesn't exist any longer. Please assist with our partner to see if they can still reproduce.

Thanks.

Comment 31

[Wayne Chang [:wchang]]

Wesly,

Can you confirm this with partner when you have a chance? Once confirmed that bug 1069109 fixes this, I guess we can dupe and close this bug.

Thanks

Comment 32

[Wesly Huang (EPM)]

got it Wayne, I've added this item into the agenda of this week's TCL/T2M weekly meeting.

@Star, Eric: just double confirm my understanding, the expected right behavior is (per [1])

(correct) music volume decreased "after" shutter sound, then resume

instead of

(wrong) music volume decreased "before" shutter sound, then resume

right?

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1009286#c4

Comment 33

[Star Cheng [:scheng]]

(In reply to Wesly Huang from comment #32)
> got it Wayne, I've added this item into the agenda of this week's TCL/T2M
> weekly meeting.
> 
> @Star, Eric: just double confirm my understanding, the expected right
> behavior is (per [1])
> 
> (correct) music volume decreased "after" shutter sound, then resume
> 
> instead of
> 
> (wrong) music volume decreased "before" shutter sound, then resume
> 
> right?
> 
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1009286#c4

Yes, you are right!

Comment 34

[Wesly Huang (EPM)]

Thanks Star, let me follow with T2M later this week when they are back from holiday.

Comment 35

[Wesly Huang (EPM)]

Hi Jack, as mentioned in today's meeting, would you help have your reporter to verify if this is still seen in your side? (we believe it's fixed already) 

then close it if verify pass.

Thank you.

Comment 36

[hanp]

Hi, Wesly
We've checked bug 1069109, and found the patch is made for fxos 2.2, 
and cannot be applied to fxos 2.0
please support, thanks

Comment 37

[Wesly Huang (EPM)]

Hi Wayne, can't TCL just applied the patch? Or can we give them some assistance here?

Comment 38

[Jack Liu]

Hi Wesly,

I think the patch for bug#1069109 should be landed to ffos2.0, then we can take and verify it.

Thanks.

Comment 39

[Eric Chou [:ericchou] [:echou]]

(In reply to Jack Liu from comment #38)
> Hi Wesly,
> 
> I think the patch for bug#1069109 should be landed to ffos2.0, then we can
> take and verify it.
> 
> Thanks.

Hi Jack,

Just a friendly reminder that we're still waiting for your feedback to close this 2.0+ issue. Would you mind giving it a quick try?

Thanks.

Comment 40

[Jack Liu]

(In reply to Eric Chou [:ericchou] [:echou] from comment #39)
> (In reply to Jack Liu from comment #38)
> > Hi Wesly,
> > 
> > I think the patch for bug#1069109 should be landed to ffos2.0, then we can
> > take and verify it.
> > 
> > Thanks.
> 
> Hi Jack,
> 
> Just a friendly reminder that we're still waiting for your feedback to close
> this 2.0+ issue. Would you mind giving it a quick try?
> 
> Thanks.

Hi Eric,

We are intend to do so, but just can't take the patch for 1069109 because it's base on 2.2, not 1.3.

Am I misunderstand or missing something important? Would you please point out how to take the patch and verify it on 1.3?

Thanks a lot.

Comment 41

[Star Cheng [:scheng]]

I am sorry that I confuse everyone. The comment 13, 15, 17 and 18 are discussing about "take a picture" instead of "take a video". So please ignore these discussions and come back to discuss about this bug. 

I can duplicate the STR of comment 0 in latest SW. And the STR is following:

STR:
 1.play music (foreground or background) by pressing "play" button of BT headset 
 2.launch the camera app, to take a video, and then click home button to back to home.
 3.pressing "play" button of BT headset in order to playing music in background
 4.music can not play in background


We can *not* resume the music because of the audio channel policy. If there is a "content" audio channel interrupts another, the one which be interrupted will be resumed to play when transit to foreground[2]. You can visit the website[1] to get more information about audio competing policy. 


[1] https://wiki.mozilla.org/WebAPI/AudioChannels - Audio Competing Policy
[2] Timing of Resuming - item [2]

Comment 42

[Ken Chang[:kenkai]]

Star, according to comment41, are you saying this is a limitation now?
Steven, can someone from your team take this bug?

Comment 43

[Star Cheng [:scheng]]

(In reply to Ken Chang[:ken] from comment #42)
> Star, according to comment41, are you saying this is a limitation now?

Yes, it is a limitation for current design.

Comment 44

[StevenLee[:slee]]

Hi Randy,
Please help this bug.
Thanks.

Comment 45

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Compare to android 4.4 phone, 
a. Play music
b. Open camera, recording a video and music is stop. 
c. Switch to music and found it was paused.
d. Press BT play key, music continues, even music is in the back-ground. <===but music still can't resume on flame-kk 2.2.

Comment 46

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

When music application is paused on the background as comment 45(d), I dump the log on music and found the app has called the audio.play() function.
  // If we reach here, the player is paused so resume it
   this.audio.play();  ~L 572

Comment 47

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

"Depends on: 1089539" is a record for checklist for future change.

The media element used by music would enter pause status after camcorder and that would block the play() action from bt headset.
Let audiochannelService to notify the mediaElemet to have the right resume manually is a possible solution.

Comment 48

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Attachment: patch v1

Only audiochannelService has the information for all clients. 
So I create new channel status, add a rule for that and let media element can resume on this case....

Comment 49

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Hi baku, 
Do you have time to review this?

Comment 50

[Andrea Marchesini [:baku]]

Comment on attachment 8514123 [details] [diff] [review]
patch v1

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

So the problem here is that, internally AudioChannelService is still muting this particular childID. Correct?
What we really want is to reset mPlayableHiddenContentChildID. But we cannot do it in 'play()' because otherwise an 3rd party app (previously muted) in foreground can play just doing a loop of play/pause and eventually it will make sound.

What I would like to suggest is:

When BT headset sends a 'play' event, we should detect it in the AudioChannelService and reset mPlayableHiddenContentChildID.
If BT service dispatches events or notifications, we can use them. Otherwise we can add a method in AudioChannelService to reset mPlayableHiddenContentChilID and use this method in some BT service.

I'm not familiar with BT code, and I cannot be more precise about how to implement this.

rlin, what do you think about this solution?

::: dom/audiochannel/AudioChannelService.cpp
@@ +408,5 @@
> +    for (int i = 0; i < AUDIO_CHANNEL_INT_LAST; i++) {
> +      count += mChannelCounters[i].Length();
> +    }
> +    // we can allow client to resume it.
> +    if (count == 1 && mChannelCounters[AUDIO_CHANNEL_INT_CONTENT_HIDDEN].Length() == 1

Just to optimize a bit I would like to have this in this way... or something similar:

if (mChannelCounters[AUDIO_CHANNEL_INT_CONTENT_HIDDEN].Length() == 1 &&
    mChannelCounters[AUDIO_CHANNEL_INT_CONTENT_HIDDEN][0] == aChildID) {
  int count = 0;
  for (int i = 0; i < AUDIO_CHANNEL_INT_LAST; i++) {
    count += mChannelCounters[i].Length();
  }

  // if count == 1, it means that we just have this content AudioChannel.
  if (count == 1) {
    return AUDIO_CHANNEL_STATE_CANRESUME; 
 }  
}

::: dom/audiochannel/nsIAudioChannelAgent.idl
@@ +54,5 @@
>    const long AUDIO_AGENT_CHANNEL_ERROR              = 1000;
>  
>    const long AUDIO_AGENT_STATE_NORMAL               = 0;
>    const long AUDIO_AGENT_STATE_MUTED                = 1;
>    const long AUDIO_AGENT_STATE_FADED                = 2;

Can you add a comment describing what CANRESUME means?

Comment 51

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

(In reply to Andrea Marchesini (:baku) from comment #50)
> Comment on attachment 8514123 [details] [diff] [review]
> patch v1
> 
> Review of attachment 8514123 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So the problem here is that, internally AudioChannelService is still muting
> this particular childID. Correct?
> What we really want is to reset mPlayableHiddenContentChildID. But we cannot
> do it in 'play()' because otherwise an 3rd party app (previously muted) in
> foreground can play just doing a loop of play/pause and eventually it will
> make sound.
> 
> What I would like to suggest is:
> 
> When BT headset sends a 'play' event, we should detect it in the
> AudioChannelService and reset mPlayableHiddenContentChildID.
> If BT service dispatches events or notifications, we can use them. Otherwise
> we can add a method in AudioChannelService to reset
> mPlayableHiddenContentChilID and use this method in some BT service.
> 
> I'm not familiar with BT code, and I cannot be more precise about how to
> implement this.
> 
> rlin, what do you think about this solution?
> 
> ::: dom/audiochannel/AudioChannelService.cpp
> @@ +408,5 @@
> > +    for (int i = 0; i < AUDIO_CHANNEL_INT_LAST; i++) {
> > +      count += mChannelCounters[i].Length();
> > +    }
> > +    // we can allow client to resume it.
> > +    if (count == 1 && mChannelCounters[AUDIO_CHANNEL_INT_CONTENT_HIDDEN].Length() == 1
> 
> Just to optimize a bit I would like to have this in this way... or something
> similar:
> 
> if (mChannelCounters[AUDIO_CHANNEL_INT_CONTENT_HIDDEN].Length() == 1 &&
>     mChannelCounters[AUDIO_CHANNEL_INT_CONTENT_HIDDEN][0] == aChildID) {
>   int count = 0;
>   for (int i = 0; i < AUDIO_CHANNEL_INT_LAST; i++) {
>     count += mChannelCounters[i].Length();
>   }
> 
>   // if count == 1, it means that we just have this content AudioChannel.
>   if (count == 1) {
>     return AUDIO_CHANNEL_STATE_CANRESUME; 
>  }  
> }
> 
> ::: dom/audiochannel/nsIAudioChannelAgent.idl
> @@ +54,5 @@
> >    const long AUDIO_AGENT_CHANNEL_ERROR              = 1000;
> >  
> >    const long AUDIO_AGENT_STATE_NORMAL               = 0;
> >    const long AUDIO_AGENT_STATE_MUTED                = 1;
> >    const long AUDIO_AGENT_STATE_FADED                = 2;
> 
> Can you add a comment describing what CANRESUME means?
oh, I don't test the "just doing a loop of play/pause and eventually" =.=

>When BT headset sends a 'play' event, we should detect it in the AudioChannelService and reset >mPlayableHiddenContentChildID.
>If BT service dispatches events or notifications, we can use them. Otherwise we can add a method in >AudioChannelService to reset mPlayableHiddenContentChilID and use this method in some BT service.
It's possible, I could try that.

Comment 52

[Wesly Huang (EPM)]

per partner's confirmation the "patch v1" works, so let's remove this one from 2.0 blocker and nom. to 2.2 for overall consideration.

Comment 53

[Bobby Chien]

Not a blocker for v2.2. This is depended on new AudioChannel refactoring.

Comment 54

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

Free it and I think 3.0 would fix this one.

Comment 55

[BMO Automation]

Firefox OS is not being worked on