983541 - [tarako] music app was killed after keep MT call for over 30s with BT headset

Status: RESOLVED DUPLICATE of bug 987022

(Core :: IPC, defect)

Description

[Peipei Cheng (needinfo if you need my action)]

Attachment: adb logcat

Tarako v1.3t build
----------------------------------------------------------------------
Build ID: 20140313135844
Gecko: d400d3a715c7c90291596b32412be56f46f2293e
Gaia: 7329446974f0ea3b14d61bd38f205eb17a5ce62d


Reproduce steps
-----------------------------------------------------------------------
1. Pair a BT headset to tarako device
2. Launch Music and choose a song to play
3. Make a MT call to the test device, Answer the MT call
4. Keep the call for over 30s, and then end the call
   --> Music App was killed.


ADB logcat
-----------------------------------------------------------------------
Attached is the adb log.
Test time starts at: 01-01 10:01:12.610

Comment 1

[Ting-Yu Chou [:ting] (away)]

Seems Music has been reset to BACKGROUND priority:

  01-01 10:02:00.780 W/audio_hw_primary(   86): do_output_standby.mode:2 
  01-01 10:02:00.900 I/Gecko:ProcessPriorityManager(   81): [Music, child-id=16, pid=2738] Scheduling reset timer to fire in 5000ms.
  01-01 10:02:05.900 I/Gecko:ProcessPriorityManager(   81): [Music, child-id=16, pid=2738] Reset priority timer callback; about to ResetPriorityNow.
  01-01 10:02:05.900 I/Gecko:ProcessPriorityManager(   81): Add ChildID(16) into LRU pool
  01-01 10:02:05.900 I/Gecko:ProcessPriorityManager(   81): [Music, child-id=16, pid=2738] Changing priority from BACKGROUND_PERCEIVABLE:CPU_LOW to BACKGROUND:CPU_LOW.

and then get killed when there's a priority change:

  01-01 10:02:33.040 I/Gecko:ProcessPriorityManager(   81): KillBackgroundApps
  01-01 10:02:33.040 I/Gecko:ProcessPriorityManager(   81): About to force kill pid: 2738
  01-01 10:02:33.040 I/Gecko:ProcessPriorityManager(   81): Remove ChildID(16) from LRU pool
  01-01 10:02:33.050 I/Gecko:ProcessPriorityManager(   81): [Music, child-id=16, pid=2738] Changing priority from BACKGROUND:CPU_LOW to BACKGROUND:CPU_NORMAL.

But ParticularProcessPriorityManager::ComputePriority() should prevent it from being a BACKGROUND process. Was the music still playing when end the call?

Comment 2

[Peipei Cheng (needinfo if you need my action)]

> But ParticularProcessPriorityManager::ComputePriority() should prevent it
> from being a BACKGROUND process. Was the music still playing when end the
> call?

No music app was killed and I cannot hear music. When restart Music App, no music was playing.

Comment 3

[Ting-Yu Chou [:ting] (away)]

(In reply to pcheng from comment #2)
> > But ParticularProcessPriorityManager::ComputePriority() should prevent it
> > from being a BACKGROUND process. Was the music still playing when end the
> > call?
> 
> No music app was killed and I cannot hear music. When restart Music App, no
> music was playing.

I didn't make myself clear, I meant was the music still playing right before Music app get killed?

Comment 4

[Joe Cheng [:jcheng] (please needinfo)]

Ting, can you take this bug? it seems like you are working on this. thanks

Comment 5

[Ting-Yu Chou [:ting] (away)]

Ok, will let you know if I don't have enough time to work on it.

Comment 6

[Joe Cheng [:jcheng] (please needinfo)]

thanks Ting, assign to you for now. thanks

Comment 7

[yang.zhao]

(In reply to Ting-Yu Chou [:ting] from comment #5)
> Ok, will let you know if I don't have enough time to work on it.
 hi,Ting
   Could you please help me to answer some questions?
   When the music is playing ,the lockscreen and the notification both have the music controller,and when press "HOME" to the launcher,the music will be playing in background,and the music app is also running.
   I'd like to know is it possible to play music in background meanwhile the music app is closed?For the music takes about 8M memory,I only want the music go on playing in background whithout music app runs. Thank you.

Comment 8

[James Zhang (Spreadtrum)]

(In reply to yang.zhao from comment #7)
> (In reply to Ting-Yu Chou [:ting] from comment #5)
> > Ok, will let you know if I don't have enough time to work on it.
>  hi,Ting
>    Could you please help me to answer some questions?
>    When the music is playing ,the lockscreen and the notification both have
> the music controller,and when press "HOME" to the launcher,the music will be
> playing in background,and the music app is also running.
>    I'd like to know is it possible to play music in background meanwhile the
> music app is closed?For the music takes about 8M memory,I only want the
> music go on playing in background whithout music app runs. Thank you.

Hi Fabrice,

   I don't think it common issue, we can fix it with system and music app.
   Can you ask music owner to try this method to shrink about 8MB USS memory? 
   If music app exit or be killed by LMK, don't stop music playing, we can still control music and reenter music app by status bar. Then we can pass many system test case when music playing background.

Comment 9

[Ting-Yu Chou [:ting] (away)]

(In reply to yang.zhao from comment #7)
>    I'd like to know is it possible to play music in background meanwhile the
> music app is closed?

Yang, I am sorry, but I am not the right person to answer your question. James has already needinfo Steven and Fabrice, please wait for their response.

Comment 10

[Steven Yang [:styang]]

(In reply to James Zhang from comment #8)
> (In reply to yang.zhao from comment #7)
> > (In reply to Ting-Yu Chou [:ting] from comment #5)
> > > Ok, will let you know if I don't have enough time to work on it.
> >  hi,Ting
> >    Could you please help me to answer some questions?
> >    When the music is playing ,the lockscreen and the notification both have
> > the music controller,and when press "HOME" to the launcher,the music will be
> > playing in background,and the music app is also running.
> >    I'd like to know is it possible to play music in background meanwhile the
> > music app is closed?For the music takes about 8M memory,I only want the
> > music go on playing in background whithout music app runs. Thank you.
> 
> Hi Fabrice,
> 
>    I don't think it common issue, we can fix it with system and music app.
>    Can you ask music owner to try this method to shrink about 8MB USS
> memory? 
>    If music app exit or be killed by LMK, don't stop music playing, we can
> still control music and reenter music app by status bar. Then we can pass
> many system test case when music playing background.

Sound a good catch.
Hi Dominic, could you help to comment on this? Thanks.

Comment 11

[[:fabrice] Fabrice Desré]

(In reply to James Zhang from comment #8)
 
>    I don't think it common issue, we can fix it with system and music app.
>    Can you ask music owner to try this method to shrink about 8MB USS
> memory? 
>    If music app exit or be killed by LMK, don't stop music playing, we can
> still control music and reenter music app by status bar. Then we can pass
> many system test case when music playing background.

That's a non trivial amount of work to do that - and running more applicative code in the parent process is in general not what we want to do. So I'm really on the fence on this one. if someone has spare cycles to try, why not...

Comment 12

[Dominic Kuo [:dkuo]]

(In reply to Steven Yang [:styang] from comment #10)
> Sound a good catch.
> Hi Dominic, could you help to comment on this? Thanks.

Currently I don't think we are able to play music in the background if the music app is not opened or killed by LMK, the logic of how we retrieve the audio files from storages and load them into the audio element then play is in music app, not system app. The playback controls on lockscreen and utility tray only send the commands of "playpause", "previous" and "next" to control the music app, so actually system app does not involved "playing the music in the background".

I don't know all the test cases, but if the requirement for this issue is to restore the music app after the call ends, we can save the states before LMK kills the music app, like album, artist or position... something that help the music app to restore. More, we can probably resume the interrupted song after the music app is restored, but I am afraid of the latency might be noticeable because what we need to do for restoring the music app should be:

1. Re-launch music app and get the last stored information from the database, like asyncStorage.
2. Use the stored information to retrieve the last song records and render them on ui.
3. Play the last song and seek to the recorded position, then pause the player.
4. Resume the player. (This might be optional because the latency of the above steps might cause some ux issues.)

This will be a feature for sure and if we want it, we should think if there are some use cases here and implement them together.

Comment 13

[Ting-Yu Chou [:ting] (away)]

I couldn't reproduce the issue, I skipped step1 since I don't have a BT headset, is it necessary?

Comment 14

[Peipei Cheng (needinfo if you need my action)]

(In reply to Ting-Yu Chou [:ting] from comment #13)
> I couldn't reproduce the issue, I skipped step1 since I don't have a BT
> headset, is it necessary?

Yes. BT headset is neccessary to reproduce this issue. Thanks!

Comment 15

[Dominic Kuo [:dkuo]]

As we discussed in the offline meeting, I have wrote a quick patch in https://github.com/dominickuo/gaia/commit/3e7eeb0f59e460f283a537c87e1f42b16176cb66.patch for gecko devs to test if we can ease the memory usage while music app is in the background(visibility is hidden).

Comment 16

[thomas tsai]

I tried today's code without WIP and followed #1 step. The MT call has connected for more than 60s and then hang up call by bt headset (Moto S10-HD). Total 3 tries are performed and only the first try has music app is killed in the moment when connection hangs up. The reset 2 tries can't repro this issue.
After the call hangs up, the music app goes to foreground, but the music won't continue to play.

gecko commit 046803073a9c5a36325c1d26d3e75e2b56b7f4b5
gaia commit f21004ba320021d916c81edc8379a4d2b5770041

free+cache in the call is about 24-26MB.

Comment 17

[Joe Cheng [:jcheng] (please needinfo)]

ni? Dkuo for comment 16

Comment 18

[Dominic Kuo [:dkuo]]

(In reply to thomas tsai from comment #16)
> I tried today's code without WIP and followed #1 step. The MT call has
> connected for more than 60s and then hang up call by bt headset (Moto
> S10-HD). Total 3 tries are performed and only the first try has music app is
> killed in the moment when connection hangs up. The reset 2 tries can't repro
> this issue.
> After the call hangs up, the music app goes to foreground, but the music
> won't continue to play.

This result looks different and more like bug 982540, so maybe we have two separated issues here, and to clarify it, I have needinfoed Mike to confirm bug 982540. And if music app was not killed and won't continue to play, I am pretty sure that's a gecko regression on audio channel.

Comment 19

[Ting-Yu Chou [:ting] (away)]

Attachment: AudioChannelAgent::StopPlaying.bt

Captured the backtrace of Music calling AudioChannelAgent::StopPlaying(), note it is not called when BT headset is not used.

The StopPlaying() removes Music from AudioChannel, triggers ProcessPriorityManager to reset its priority from BACKGROUND_PERCEIVABLE to BACKGROUND.

Comment 20

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #19)
> The StopPlaying() removes Music from AudioChannel, triggers
> ProcessPriorityManager to reset its priority from BACKGROUND_PERCEIVABLE to
> BACKGROUND.

After answer the call, the scoConnectionHandler() of remote_control.js is called and initiates the pause command handler of music/js/communications.js to stop playing, this should be expected behavior.

Fabrice, seems this is a side effect from attachment 8380962 [details] [diff] [review], the Music gets killed before it is resumed. But indeed, if background application tends to be killed on Tarako, should we still reset it to BACKGROUND in this use case?

Note since WIPs are no longer included from daily build 20140320, I expect the issue can't be reproduced easily.

Comment 21

[James Zhang (Spreadtrum)]

If we want to pass Spreadtrum system test case, there are two solutions.
1. Clear app's UI memory(include HTML/CSS content) when app switch to background.
2. Keep background app status if be killed by LMK, and restore when app switch to foreground.

Comment 22

[[:fabrice] Fabrice Desré]

(In reply to Ting-Yu Chou [:ting] from comment #20)
> (In reply to Ting-Yu Chou [:ting] from comment #19)
> > The StopPlaying() removes Music from AudioChannel, triggers
> > ProcessPriorityManager to reset its priority from BACKGROUND_PERCEIVABLE to
> > BACKGROUND.
> 
> After answer the call, the scoConnectionHandler() of remote_control.js is
> called and initiates the pause command handler of music/js/communications.js
> to stop playing, this should be expected behavior.
> 
> Fabrice, seems this is a side effect from attachment 8380962 [details] [diff] [review]
> [diff] [review], the Music gets killed before it is resumed. But indeed, if
> background application tends to be killed on Tarako, should we still reset
> it to BACKGROUND in this use case?

This patch should not be in your build, it never landed. Please test with a proper build.

> Note since WIPs are no longer included from daily build 20140320, I expect
> the issue can't be reproduced easily.

It seems you still have unlanded patches...

Comment 23

[[:fabrice] Fabrice Desré]

(In reply to James Zhang from comment #21)
> If we want to pass Spreadtrum system test case, there are two solutions.
> 1. Clear app's UI memory(include HTML/CSS content) when app switch to
> background.

Do we know how much we would clear?

> 2. Keep background app status if be killed by LMK, and restore when app
> switch to foreground.

For that we will need a proper session restore mechanism - that would be a stretch for 1.3 unless the work done for the e10s version of firefox is reusable.

Comment 24

[James Zhang (Spreadtrum)]

Add Jesse here for discussing solution 1.

Comment 25

[Jesse]

(In reply to Dominic Kuo [:dkuo] from comment #15)
> As we discussed in the offline meeting, I have wrote a quick patch in
> https://github.com/dominickuo/gaia/commit/
> 3e7eeb0f59e460f283a537c87e1f42b16176cb66.patch for gecko devs to test if we
> can ease the memory usage while music app is in the background(visibility is
> hidden).

The patch can really save some memory, but not enough. Maybe we need to do more work to ease memory usage of music app which runs in background.

Comment 26

[Jesse]

The solution which "kill music app when a call is coming, and restart it when call ends." is not adoptable for its high latency in Tarako. What we could do is simply moving apps to background and cleaning up memory usage of these apps.

Comment 27

[Jesse]

Fabrice, we could do some work on gecko to remove css layout objects and image cache when app runs into background. What do you think?

Comment 28

[Joe Cheng [:jcheng] (please needinfo)]

Triage: team's working on bug 987022 to speed up incoming call rings. let's track bug 987022

Comment 29

[[:fabrice] Fabrice Desré]

Clearing ni since we're now working on bug 987022