[tarako] Background Music playing was killed when memory pressure is high and could not resume state later

RESOLVED INVALID

Status

defect
P2
normal
RESOLVED INVALID
5 years ago
4 years ago

People

(Reporter: angelc04, Unassigned)

Tracking

({memory-footprint, perf})

Firefox Tracking Flags

(tracking-b2g:backlog, b2g-v1.3T affected)

Details

(Whiteboard: [c=memory p= s= u=tarako] [MemShrink:P2] OOM [priority][sprd287908])

Attachments

(2 attachments)

Posted file adb logcat
Steps to reproduce
-------------------------------------------------------------------------
1. Launch Music and play some music
2. Tap on Home button to send Music to background
3. Try to do sth which can consume a lot of memory, such as
   Launch Messages -> Attach a large pic to MMS
   --> Music app stop playing when memory pressure is high.
4. Launch Music again
   --> Previous music play state was not resumed.

[expected] Music should not be killed or it can be killed and then resume previous state.

Attached adb logcat. Test starts: 04-16 19:56:43.300


Test build
------------------------------------------------------------------
Gaia      c62bff0bfb8b069c962dfae2640e931cc9ad1bdf
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/7e764399b4fc
BuildID   20140415164003
Version   28.1
ro.build.version.incremental=eng.cltbld.20140415.201139
ro.build.date=Tue Apr 15 20:11:46 EDT 2014
Hi! Dominic,

Could you help to take a look? Thanks

--
Keven
blocking-b2g: --- → 1.3T?
Flags: needinfo?(dkuo)
Keywords: footprint, perf
Whiteboard: OOM, [c=memory p= s= u=] [MemShrink]
Is this something we actually want to support for now? While it'd be nice to have, my understanding was that our goal for 1.3T was just to solve the dialer/music case.
After reading this, I had a thought for a new feature that we might want to consider.

Basically, split the Music Player into 2 pieces:
- The UI
- An enginge which can play a list of songs

The UI seems to be the thing which consumes the most memory. The enginge for just playing a sing should be quite small.

Then it would allow playing music in the background to not consume as much memory as it does today.
blocking-b2g: 1.3T? → backlog
Whiteboard: OOM, [c=memory p= s= u=] [MemShrink] → OOM, [c=memory p= s= u=] [MemShrink][priority]
(In reply to Keven Kuo [:kkuo] from comment #1)
> Hi! Dominic,
> 
> Could you help to take a look? Thanks
> 
> --
> Keven

I have a WIP in the previous mail thread might help here for 1.3t, possibly could also be a nice-to-have feature for master, I will attach it later in case this become 1.3T+ again.
Flags: needinfo?(dkuo)
(In reply to Dave Hylands [:dhylands] (away - back Tue April 15) from comment #3)
> After reading this, I had a thought for a new feature that we might want to
> consider.
> 
> Basically, split the Music Player into 2 pieces:
> - The UI
> - An enginge which can play a list of songs
> 
> The UI seems to be the thing which consumes the most memory. The enginge for
> just playing a sing should be quite small.
> 
> Then it would allow playing music in the background to not consume as much
> memory as it does today.

I agree with Dave, we should split the music app into two parts if music app is possible to be killed while playing, but currently we don't have a specific api to support capability like this, to have a separated audio engine/service outside the music app.

I think if we really need this for short-term, to let this feature live on master and to deal with the playing music killed because of low memory, we can try: add a mini player in system app, and the music app will communicate with it via IAC. If music app is killed, the mini player is still playing in system, once the user try to launch or switch back to it from homescreen or card view, the music re-launches then try to sync its state with the mini player in system, this sounds achievable to me but there are some known issues:

1. It takes time to sync the state between music app and system mini player, because the operations include: Basic app launch + MediaDB initialization + Render ui
2. The bluetooth remote controls(AVRCP) are handled in music app, these logic must move to the system mini player, or once music app is crashed, AVRCP will not function, and this means the system app needs to handle the extra AVRCP system messages, which I am not sure if it's a right design for the whole gaia architecture.
3. The current audio competing behaviours will be broken because the audio element doesn't live in the music app any more, I am sure we need to write some "extra" logic for audio competing.
4. The system mini player will be a super power for gaia music app, the 3rd party media players will still be killed due to low memory, unless we standardize it or have new api to support usages like this.
(In reply to Dominic Kuo [:dkuo] from comment #6)
> Created attachment 8408033 [details]
> WIP - if we need it

Of course we need it.
(In reply to Dominic Kuo [:dkuo] from comment #5)
> I think if we really need this for short-term, to let this feature live on
> master and to deal with the playing music killed because of low memory, we
> can try: add a mini player in system app, and the music app will communicate
> with it via IAC.

Would it be possible to do this in a single app, e.g. with multiple frames? Alternately, we could make a separate "music-daemon" app that handles the playback. I don't think IAC requires us to use the system app here.
(In reply to Dominic Kuo [:dkuo] from comment #6)
> Created attachment 8408033 [details]
> WIP - if we need it

Looking at this, I don't think we should do this in general. If we have enough memory to hold both the music UI and whatever else the user is doing at the time, we shouldn't clean up the UI. That should make returning to the music app more responsive. We should only clean up the UI if we're running low on memory. (That said, we should also be conscious of minimizing CPU usage when the UI isn't visible.)

Basically, if we have free memory on the device, we should be using it to help make the experience better.
blocking-b2g: backlog → 1.3T?
Whiteboard: OOM, [c=memory p= s= u=] [MemShrink][priority] → OOM, [c=memory p= s= u=] [MemShrink][priority][sprd287908]
Whiteboard: OOM, [c=memory p= s= u=] [MemShrink][priority][sprd287908] → OOM, [c=memory p= s= u=] [MemShrink][priority][sprd287908] [partner-blocker]
ni? styang to discuss with partner
Flags: needinfo?(styang)
Flags: needinfo?(styang)
Whiteboard: OOM, [c=memory p= s= u=] [MemShrink][priority][sprd287908] [partner-blocker] → OOM [c=memory p= s= u=] [MemShrink][priority][sprd287908] [partner-blocker]
(In reply to Jim Porter (:squib) from comment #8)
> (In reply to Dominic Kuo [:dkuo] from comment #5)
> > I think if we really need this for short-term, to let this feature live on
> > master and to deal with the playing music killed because of low memory, we
> > can try: add a mini player in system app, and the music app will communicate
> > with it via IAC.
> 
> Would it be possible to do this in a single app, e.g. with multiple frames?
> Alternately, we could make a separate "music-daemon" app that handles the
> playback. I don't think IAC requires us to use the system app here.

Do you mean to have a music app that embedded an audio iframe/app from outside the music app? I am not sure if music app is able to embed an external iframe/app, I guess the answer is no because that's probably only the system app can do. And to make a separate "music-daemon" app is achievable, but we will see the music-daemon app in the card view because the window management doesn't know it should be hidden, and plus on tarako the audio engine/service will not be killed only if it's part of system, so that's why I think to put a mini player in system app and communicate with music app via IAC is more realistic.
(In reply to Jim Porter (:squib) from comment #9)
> (In reply to Dominic Kuo [:dkuo] from comment #6)
> > Created attachment 8408033 [details]
> > WIP - if we need it
> 
> Looking at this, I don't think we should do this in general. If we have
> enough memory to hold both the music UI and whatever else the user is doing
> at the time, we shouldn't clean up the UI. That should make returning to the
> music app more responsive. We should only clean up the UI if we're running
> low on memory. (That said, we should also be conscious of minimizing CPU
> usage when the UI isn't visible.)
> 
> Basically, if we have free memory on the device, we should be using it to
> help make the experience better.

I agree with you Jim, we shouldn't clean up the ui unless the memory is low, but currently gecko won't expose the memory pressure information to apps, it shouldn't, too, that's the system level thing which only system app should care about, and I was told the direct way to release the memory is to remove the dom elements from dom tree, so that's why the WIP look like.

I really hope apps won't be killed unexpectedly because of low memory, but looks like it's the current strategy for saving the memory on tarako, and if cleaning up the ui in background is not suggested by you, then we should consider splitting up the music app seriously to make everyone happy.
triage: this will not block tarako release
blocking-b2g: 1.3T? → backlog
Whiteboard: OOM [c=memory p= s= u=] [MemShrink][priority][sprd287908] [partner-blocker] → OOM [c=memory p= s= u=] [MemShrink][priority][sprd287908]
Whiteboard: OOM [c=memory p= s= u=] [MemShrink][priority][sprd287908] → OOM [c=memory p= s= u=] [MemShrink:P2][priority][sprd287908]
Let's try this out with a recent version.
Flags: needinfo?(pbylenga)
I was unable to reproduce this using an updated Tarako build

1.3t Environmental Variables:
Device: Tarako 1.3t
BuildID: 20140505173556
Gaia: 48372ff88c712468a77a6f14a97f9978ee56eade
Gecko: 78015cc18d1f
Version: 28.1
Firmware Version: sp6821
Flags: needinfo?(pbylenga) → needinfo?
verify with today's daily build, below are build information and results
Gaia      746934a1e331b9ce578bd9fbdb4614d950baf765
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/771fedd3e904
BuildID   20140506164003
Version   28.1

Results: Music is killed by several situations
1. music in background -> add contact picture from gallery
2. music in background -> import contacts from FB
3. music in background -> mms attach image from gallery
4. music in background -> compose an email and attach image from gallery
5. music in background -> change wallpaper from gallery (either from homescreen or settings app)
Flags: needinfo?
(In reply to Mike Lien[:mlien] from comment #16)
> verify with today's daily build, below are build information and results
> Gaia      746934a1e331b9ce578bd9fbdb4614d950baf765
> Gecko    
> https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/771fedd3e904
> BuildID   20140506164003
> Version   28.1
> 
> Results: Music is killed by several situations
> 1. music in background -> add contact picture from gallery
> 2. music in background -> import contacts from FB
> 3. music in background -> mms attach image from gallery
> 4. music in background -> compose an email and attach image from gallery
> 5. music in background -> change wallpaper from gallery (either from
> homescreen or settings app)

update additional Music is killed result
music in background -> use Browser
Whiteboard: OOM [c=memory p= s= u=] [MemShrink:P2][priority][sprd287908] → [c=memory p= s= u=tarako] [MemShrink:P2] OOM [priority][sprd287908]
remove dependency here as P2
No longer blocks: 995122
blocking-b2g: backlog → ---
Closing this, since it's Tarako-only. Feel free to reopen if you think it's still important.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.