1077768 - on connecting Bluetooth, affer playing video,music resume automatically

Status: RESOLVED WONTFIX

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

Description

[soyeon]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140923175406

Steps to reproduce:

Dear Sireesha and Ranjith
 I have an another question.

on connecting BT(bluetooth)
1. play the music.
2. and play the video.
3. In Actual results, music pause and video play.
4. Receive the call
5. after calling, video player is stopped and music resume.





Actual results:

is it a correct scenario??

I think the music don't need to play. But in Actual results, music resume automatically.

on the contary, wired earjack, music is not resume.



Expected results:

I analyze the situation and saw the log of  call stack.

On connecting Bluetooth, 
atfer calling, Bluetooth path is updated in Audio Framework.
and then, call the Play() in HTMLMediaElement.cpp I think maybe.


10-04 10:44:48.404 I 231      231      GeckoBluetooth:                      NotifyConnectionStateChanged: bluetooth-sco-status-changed 

10-04 10:44:48.404 E 231      1120     bt-btif                              : btif_media_aa_prep_2_send : 5 frames (queue 0) 
10-04 10:44:48.404 E 231      1120     bt-btif                              : TX QUEUE NOW 0 
10-04 10:44:48.404 E 231      1120     bt-btif                              : btif_media_aa_prep_2_send : 5 frames (queue 1) 
10-04 10:44:48.404 D 1665     1665     HTMLMediaElement:                    0xb1e888e0 Play 
10-04 10:44:48.404 D 1665     1665     HTMLMediaElement:                    0xb1e888e0 Queuing event play 
10-04 10:44:48.404 E 231      1120     bt-btif                              : TX QUEUE NOW 1 
10-04 10:44:48.404 E 231      1120     bt-btif                              : btif_media_send_aa_frame : send 5 frames 
10-04 10:44:48.404 D 1665     1665     HTMLMediaElement:                    0xb1e888e0 Queuing event playing 

----------------------------------------------------------------------

it seems like Bug 1048109.
The issue is not represent 100%  but I  think that has the problem.

Dear. Dear Sireesha and Ranjith

I'm sorry to say, Let us know why HTMLMediaElement::Play(ErrorResult& aRv)  is called.
if it possible to modify that, I'll ask for your help.

Comment 1

[soyeon]

I don't know well the person in charge music app
so I ask star cheng for help at first.

Comment 2

[Star Cheng [:scheng]]

(In reply to soyeon from comment #1)
> I don't know well the person in charge music app
> so I ask star cheng for help at first.

I can *not* duplicate it with latest SW on flame device. If the symptom described in comment 0 occurs, it is a bug.

Comment 3

[ranjith253]

Hi Dominic,

Looks like music app gets 'mozinterruptbegin' event when video starts playing.
when call ends based on SCO status music is resumed when player is in INTERRUPT_BEGIN state.
I think bug 1048109 could be due to similar reason. 

Need your help in fixing this issue.

Thanks.

Comment 4

[Dominic Kuo [:dkuo]]

I have discussed with ranjith on IRC, and he mentioned to use IAC to notify music app which channel type has interrupted music app could possibly solve this issue.

When the device is using the BT headset and in a call(SCO is connected):
1. If music is interrupted by ringer channel, then it should resume after the interrupt ends.
2. If music is interrupted by content channel, then it shouldn't resume after the interrupt ends.

Though I think it's a hacky way but for now, but it might be a workable fix.

Comment 5

[soyeon]

Hi, star cheng.


as you say,you could not reproduce the issue of lastet sw. what was the version?
I checked that the same issue was reproduced on flame device on a date 10/6.

So, this issue have to be modified.

and then... 

I know that the mozila's opinion doesnt agree with Ranjith. is it right?

But I want to be fixed about this issue.
I ask your reply as soon as possible.

Comment 6

[Dominic Kuo [:dkuo]]

Please try to nominate this bug to be a blocker or higher priority, usually we don't allow new workaround to be landed on master branch, unless some issue has blocked the releases.

The work order is in [1], and the guidelines for nominating blockers is in [2], after the flags are set, it should get the TAM's attention then we can put this in progress to fix, thanks.

[1] https://wiki.mozilla.org/Release_Management/B2G_Landing
[2] https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines

Also flagging Vance to help on this, Vance, would you please help our partner to understand how [1] and [2] works? thanks.

Comment 7

[soyeon]

Sorry, I missed on the comment.
I checked v2.0 on flame device. 
But issue is reproduced.

Comment 8

[soyeon]

+ to add

I'm sorry to troble with you...

does not the issue reproduce on V2.2?
if it is true, I'll block this issue.

Comment 9

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

Just run a quick test on master and still can reproduce this issue

Hi Dominic, Star, for 2.0, is it possible we use the approach mentioned in Comment#4 to fix this issue?

Thanks

Comment 10

[Dominic Kuo [:dkuo]]

(In reply to Vance Chen [:vchen][[e-mail]] from comment #9)
> Just run a quick test on master and still can reproduce this issue
> 
> Hi Dominic, Star, for 2.0, is it possible we use the approach mentioned in
> Comment#4 to fix this issue?
> 
> Thanks

It should work though it means the system app needs to expose some information which the music app shouldn't know about it, but as I mentioned in comment 6, let's triage this bug then decide the work priority, I can do both patch or review if needed, thanks.

Comment 11

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

Per discussion with partner, this one is a blocker for their device. Nominate to 2.0? first. 

Hi Ranjith - Seems like you already discussed this issue with Dominic, is it possible you can help to write a patch for this bug? We can help to review then.

Thanks

Comment 12

[ranjith253]

Hi vance ,

I can  take this issue , but I have a query for Dominic.

Dominic: As you know IAC communication & "mozinterruptbegin" are not synchronous.

First IAC will send the channel info which I will be storing.
Then "mozinterruptbegin" will be triggered , there is delay between these two steps.
Can this cause any regression issues?

Comment 13

[ranjith253]

Attachment: WIP

Hi Dominic,

This patch is for sending audio channel information to music application via IAC.
And based on the audio channel music app  will decide on resuming the audio.

I have following observation for this patch.

As you know in BT call state music app will not get "mozinterruptend" event after call end. So audio will not be resumed automatically when user get the music app to foreground . Player will be in paused state & user has to press the play button for resuming the audio for this particular issue scenario. 

This behavior can be treated as expected?

Need your review comments.

Thanks.

Comment 14

[Dominic Kuo [:dkuo]]

I have noticed this blocker and will review it today or tomorrow, thanks.

Comment 15

[Dominic Kuo [:dkuo]]

Comment on attachment 8507677 [details]
WIP

Ranjith, thanks for giving this patch, the idea looks fine but I think there are two major issues I have found:

1. The patch notifies the music app every time when the |audio-channel-changed| event fires, but this bug is only part of the cases(content <-> ringer), so we better not to send the other unnecessary audio channel information to music app.
2. The patch actively send the audio channel information from system to music, it might works but probably not a best timing to get the correct audio channel info(though I am not sure either).

Have you consider to get the audio channel info when |mozinterruptbegin| fires in music app? it's possibly a suitable timing to get the audio channel info from system app, though I don't know which event will fire first, |mozinterruptbegin| or |audio-channel-changed|, you should know this better than me since you have tested it.

Comment 16

[ranjith253]

(In reply to Dominic Kuo [:dkuo] from comment #15)
> Comment on attachment 8507677 [details]
> WIP
> 
> Ranjith, thanks for giving this patch, the idea looks fine but I think there
> are two major issues I have found:
> 
> 1. The patch notifies the music app every time when the
> |audio-channel-changed| event fires, but this bug is only part of the
> cases(content <-> ringer), so we better not to send the other unnecessary
> audio channel information to music app.
  
I think you are right I need to send only ringer audio channel information.

> 2. The patch actively send the audio channel information from system to
> music, it might works but probably not a best timing to get the correct
> audio channel info(though I am not sure either).
> 

I too thought of doing this but music app need to request system app first for channel
info & then system app will post the channel info . This may have delay then using single
post message from system app for updating channel info. I have noticed during  testing that 
|audio-channel-changed| event is fired fist,  so I store the channel info & update it when
 |mozinterruptbegin| is fired. I am also not sure which approach may give best timing for updating the
channle info, if you have any opinion about its let me know.

> Have you consider to get the audio channel info when |mozinterruptbegin|
> fires in music app? it's possibly a suitable timing to get the audio channel
> info from system app, though I don't know which event will fire first,
> |mozinterruptbegin| or |audio-channel-changed|, you should know this better
> than me since you have tested it.

Comment 17

[Dominic Kuo [:dkuo]]

If the |audio-channel-changed| event in system always fire first than the |mozinterruptbegin| in music app, what if IAC delivered the audio channel info after the |mozinterruptbegin| event? I think you are assuming the message flow will like:

(A): |audio-channel-changed| -> IAC send channel info -> got channel info -> |mozinterruptbegin| -> check channel

but could it be possibly like:

(B). |audio-channel-changed| -> |mozinterruptbegin| -> check channel -> IAC send channel info -> got channel info ?

and what I am suggesting is:

(C). |audio-channel-changed| -> |mozinterruptbegin| -> IAC to request channel info -> got channel info -> check channel

(A) and (B) are the possibilities your approach could happen, but (B) might gets wrong channel info because the channel info is out-of-date, the channel info is checked before the latest one sent. And although (C) seems the slowest approach, but it should get the correct channel info base on your assumption - |audio-channel-changed| fire first and system has the latest channel info, also the delay should be acceptable because when the time music app needs to resume the player, the channel info should be update-to-date and we are not resuming right after |mozinterruptbegin|, it's the time when a call ends.

I didn't actually test how those two events fire, just judging by what the information you shared with us, I might be wrong so please do share your opinions on what you have got from the real device, thanks.

Comment 18

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

ni Bobby to look into this

Comment 19

[Ken Chang[:kenkai]]

Randy, this bug can be fixed after finishing bug 1089539. Right?

Comment 20

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

Hi Ken, 
Gecko won't solve this problem even phase in the bug 1089539.
As discussed with Dominic, he suggests this one could be solved by the attachment provided by  [e-mail].
He will make comment later.

Comment 21

[Dominic Kuo [:dkuo]]

Ranjith, would you please see comment 17 and see if it helps or I am wrong about it? thanks.

Comment 22

[bhavana bajaj [:bajaj]]

Passing this onto ranjith given he is having a WIP patch.

Comment 23

[ranjith253]

Hi Dominic,

During testing I always got the following sequence.

(A): |audio-channel-changed| -> IAC send channel info -> got channel info -> |mozinterruptbegin| -> check channel.

But anyway you approach (C) looks good for me because its more reliable. 
I will try to upload new WIP in 1 or 2 days .

Thanks.

Comment 24

[Rachelle Yang [:ryang][[e-mail]]]

Partner working on this to provide WIP. 
denominate for now.

Comment 25

[ranjith253]

I am updating fail scenarios which are identified on flame device with latest v2.2 version.
All these cases should be addressed while resolving this issue.

Case 1:
Music play -> Video play(Music is stopped) -> Incoming call -> Accept call -> End
Expected: Music should not be played automatically after call end.
Actual: Music  is played automatically after call end.

Case 2:
Music play -> Video play(Music is stopped) -> Outgoing call -> Reject(Do not accept)
Expected: Music should not be played automatically after call end.
Actual: Music  is played automatically after call end.

Case 3:
Music play -> Video play(Music is stopped) -> Outgoing call -> Accept call -> End
Expected: Music should not be played automatically after call end.
Actual: Music  is played automatically after call end.

Case 4:
Music play -> Video play(Music is stopped) -> Outgoing call -> Reject by opponent(Do not accept)
Expected: Music should not be played automatically after call end.
Actual: Music  is played automatically after call end.

Comment 26

[ranjith253]

Attachment: Pointer to Pull Request

Hi Dominic,

I am uploading a new pull request as per the suggestion in comment 17.
Let me know your review comments.

Comment 27

[Dominic Kuo [:dkuo]]

Comment on attachment 8627629 [details]
Pointer to Pull Request

Ranjith,

I took a quick look on the new patch, but found you are using iac_handler.js, actually music already handles the iac messages in remote_controls.js, apparently it's unnecessary. Also, the New audio channel service will be landed soon, in theory it should fix this issue, let's wait and see if it works, thanks.

Comment 28

[ranjith253]

Hi Dominic, 

If you mind sharing information about new audio channel service like
1. Bugzilla ID
2. Probable target date of landing this patch.
3. Will this patch will be uplifted to previous versions like V2.2 ?

Above information will be helpful form me in tracking this bug.

Comment 29

[Alastor Wu [:alwu]]

Hi, Ranjith,
You can see the Gecko part in bug1113086, and Gaia part in bug1100822.
It will be landed recently.
Now we don't have plan to uplift it to previous version, because it might be little risky (we don't know how stable it is)

Comment 30

[Dominic Kuo [:dkuo]]

Cancelling the needinfo, Alastor answered in comment 29 :)

Comment 31

[Bobby Chien]

We don't have plan to land NewAudioChannel in v2.2. (Same comment 29).

Comment 32

[Jinho Lee]

Dear Bobby Chien,

Could you explain more to me?
Why these changes can not be applied on v2.2?

Thanks.

Comment 33

[Bobby Chien]

NewAudioChannel is big re-factoring from origin audio channel system. It moves audio policy system from Gecko to Gaia layer. As architecture design, it's huge effort to move these change back to v2.2. So it is only applied since v2.5. 

NewAudioChannel also changes audio competing policy[1] based origin behavior in v2.2. There are some changes (please refer [1] for detail). However, as regression perspective, NewAudioChannel would keep same behavior with v2.2 as possible. 

[1] https://bug1068219.bmoattachments.org/attachment.cgi?id=8579177#12

Hope this answer your concern. Let me know. Thanks.

Comment 34

[Jinho Lee]

It's very helpful. Thanks!