Closed Bug 1214208 Opened 4 years ago Closed 4 years ago

Power Consumption increased in music app while playing music

Categories

(Core :: Audio/Video: Playback, defect, P2)

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
mozilla44
blocking-b2g 2.5+
Tracking Status
firefox44 --- fixed

People

(Reporter: jhylands, Assigned: ayang)

References

Details

(Keywords: regression, Whiteboard: [Power:P1])

Attachments

(4 files)

The new music app is showing a fairly significant power regression while playing music, both with the screen on and with the screen off.

https://raptor.mozilla.org/dashboard/script/power?var-device=flame-kk&var-memory=1024&var-branch=master

Click on the top right button where it says "3 days ago..." and change that to last 30 days. As soon as the automated tests worked again (Oct 9) the power usage jumped, and has remained higher.
[Blocking Requested - why for this release]:
It's a regression, and the percentage of additional power consumption is significant
blocking-b2g: --- → 2.5?
Keywords: regression
Here's a screenshot of the pwoer harness measuring playing a song (screen off) with a PVT build from Sept 25. This shows actual power consumed, without the overhead imposed by the USB charger being turned off in the automated test system.
Here's a screenshot of the power harness measuring playing the same song (screen off) with a PVT build from Oct 13.

Power consumption is noticeably higher here.
Jon, can you narrow it down the regression range to a day?
Flags: needinfo?(jhylands)
I'm pretty sure that it was the day the new version of the app was released, but I will validate that...
So, it turns out I was wrong.

The regression first appeared in this build:

https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-central-flame-kk/2015/10/2015-10-02-15-02-29/

So something that was merged on Oct 2, between the first PVT build and the second one.
Flags: needinfo?(jhylands)
(In reply to Jon Hylands [:jhylands] from comment #7)
> So, it turns out I was wrong.
> 
> The regression first appeared in this build:
> 
> https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/nightly/mozilla-
> central-flame-kk/2015/10/2015-10-02-15-02-29/
> 
> So something that was merged on Oct 2, between the first PVT build and the
> second one.

Jon: I looked at the gaia commits for the Music app around Oct. 2, and they mostly revolved around adding the `[defer]` attribute to our `<script>` tags. Nothing that deals with audio playback was changed. If you flash an ENG build from PVT, you will get both the old Music app and the new Music app installed together. I'm curious to see if there is any difference in power consumption for audio playback between the two. If both apps show the same power regression, then its likely that this is an issue with Gecko audio playback. I was going to try and run fxos-powertool yesterday, but didn't get around to it.
Justin - it looks like its probably a gecko issue - both apps display the same power profile on the Oct 2 Eng build.
Component: Gaia::Music → Audio/Video: Playback
Product: Firefox OS → Core
Summary: Power Consumption increased in new music app while playing music → Power Consumption increased in music app while playing music
Whiteboard: [Power]
Per comment 8, not sure this could be easy investigated or not. Blake, do you have any comment on this?
Flags: needinfo?(bwu)
Priority: -- → P2
First of all, Thanks Jon. This is so cool! I didn't know we have this hardness to monitor power consumption. 
After quickly checking the long list of gecko committed logs from comment 10, I cannot find a suspicious one. 
Does anyone know will there be any quick way to find the patch caused this regression?
Flags: needinfo?(bwu)
Bug 1209410 sticks out here but Jon will verify.
I've verified that Bug 1209410 does not seem to be the problem, because I checked out the commit prior to that, and the problem is still there. I'll continue bisecting today...
(In reply to Jon Hylands [:jhylands] from comment #14)
> I've verified that Bug 1209410 does not seem to be the problem, because I
> checked out the commit prior to that, and the problem is still there. I'll
> continue bisecting today...
A million thanks!
Okay, so it turns out I was looking at the list backwards, and Bug 1209410 is in fact the culprit.
Blocks: 1209410
Jean, can we do anything here?
Flags: needinfo?(jyavenard)
I can't really see why enabling the MP3Demuxer would significantly increase power usage. How much higher is it ?

Can you run a profiler to see where most of the time is spent?

Bug 1209410 may also expose an issue with the Gonk PDM (the MP3 decoder) which wasn't called before (it was only called for decoding MP3 inside MP4 containers)

There's been some changes recently on the Gonk PDM to make it faster at decoding H264; could this has an affect on power usage?

Eugene is this something you could look into?
Flags: needinfo?(jyavenard) → needinfo?(esawin)
Running manual power tests, the old music player took an average of 27 mA running with the screen off, and the new one takes 67 mA. That's a difference of 40 mA, and more than double the power used.

In real terms, its the difference between 55 hours of play time and 22 hours (assuming 1500 mAh available from the battery).
Sorry, I misspoke in the last comment. Its not an issue of the old or new player. What I meant to say was, pre-regression power usage is 27 mA, and post-regression is 67 mA.
That's a significant regression. I suspect that it's the decoder, since the new demuxer is modeled after the previous one with no significant semantic changes, but I wouldn't rule it out before seeing the profiling logs.

I don't have a FxOS setup, can you please run a profiler and report the results? I can see what profiling gives us on Android, but we don't have power usage metrics and it would be using MediaCodec's decoder.
Flags: needinfo?(esawin)
(In reply to Jean-Yves Avenard [:jya] from comment #18)
> I can't really see why enabling the MP3Demuxer would significantly increase
> power usage. How much higher is it ?
> 
> Can you run a profiler to see where most of the time is spent?
> 
> Bug 1209410 may also expose an issue with the Gonk PDM (the MP3 decoder)
> which wasn't called before (it was only called for decoding MP3 inside MP4
> containers)
> 
> There's been some changes recently on the Gonk PDM to make it faster at
> decoding H264; could this has an affect on power usage?
Probably. 
Alfredo, do you have any ideas which might affect this?
Flags: needinfo?(ayang)
Blocking for 2.5 based on how bad the battery consumption can get
blocking-b2g: 2.5? → 2.5+
Is there a possibility that is related to usage of AudioOffloadPlayer? Bug 1149984 might be related to it.
See Also: → 1149984
Alfredo, any update?
(In reply to Sotaro Ikeda [:sotaro] from comment #24)
> Is there a possibility that is related to usage of AudioOffloadPlayer? Bug
> 1149984 might be related to it.

I'm confused about it. Is AudioOffloadPlayer enable on PDM? Why do you think AudioOffloadPlayer is related to this bug?
Flags: needinfo?(ayang)
(In reply to Alfredo Yang (:alfredo) from comment #26)
> (In reply to Sotaro Ikeda [:sotaro] from comment #24)
> > Is there a possibility that is related to usage of AudioOffloadPlayer? Bug
> > 1149984 might be related to it.
> 
> I'm confused about it. Is AudioOffloadPlayer enable on PDM? Why do you think
> AudioOffloadPlayer is related to this bug?

I just said that when MediaFormatReader is used for decoding, AudioOffloadPlayer is not used. It causes the power consumption up during music playback.
Is there anything we can do for this bug?
Flags: needinfo?(bwu)
(In reply to Ken Chang[:ken] from comment #28)
> Is there anything we can do for this bug?

Bug 1209410 turns on mp3 decoding in Gonk PDM. I'm checking if the test audio matching this and any CPU spinning in test. I'll update it soon.
Flags: needinfo?(bwu)
Assign to Alfredo. 
Hi Alfredo, 
Please feel free to un-assign yourself if you are not the right person to check this bug.
Thanks!
Assignee: nobody → ayang
Can you please test patch 5 [1] from bug 1163667 to see whether decreasing the scan buffer size has an effect on power consumption?

[1] https://bug1163667.bmoattachments.org/attachment.cgi?id=8677642
Applying that patch has no apparent effect on power consumption.
Whiteboard: [Power] → [Power:P1]
Ok, I think I got the whole picture.

AudioOffloadPlayer is an audio tunneling mode player on Gonk. It uses less CPU and power comparing to normal playback engine when playing audio only, non-streaming file.
 
Bug 1209410 switches mp3 decoder to PDM. Unfortunately, current Gonk PDM doesn't support AudioOffloadPlayer. And then this regression happens.

So it needs to switch back to AudioOffloadPlayer or supports AudioOffloadPlayer on gonk PDM.
the Gonk PDM doesn't support AudioOffloadPlayer leading to increased power consumption.
Comment on attachment 8677982 [details] [diff] [review]
Do not use MP3Decoder on B2G. r=alfredo

This disable the new MP3Decoder on platform with the OMX player.

Alfredo, I'm not sure if OMX is used on Android or not, and the MP3Decoder was primarily implemented for adding MP3 support to newer Android version.
Attachment #8677982 - Flags: feedback?(ayang)
Comment on attachment 8677982 [details] [diff] [review]
Do not use MP3Decoder on B2G. r=alfredo

This patch fixes the power regression.
Attachment #8677982 - Flags: feedback?(ayang) → feedback+
(In reply to Jean-Yves Avenard [:jya] from comment #35)
> Comment on attachment 8677982 [details] [diff] [review]
> Do not use MP3Decoder on B2G. r=alfredo
> 
> This disable the new MP3Decoder on platform with the OMX player.
> 
> Alfredo, I'm not sure if OMX is used on Android or not, and the MP3Decoder
> was primarily implemented for adding MP3 support to newer Android version.

I think Fennec doesn't use OMX (actually, it is stagefright) directly like Gonk.
See Also: → 1188460
Android uses java's Mediacodec(Bug 1014614, Bug 1097126) to avoid symbol mismatch problem on recent android. Android devices have modification to their native(c++ code) and it causes symbol mismatch if gecko uses c++ MediaCodec. And android applications do not have permission to load hw codec in their processes. Using java api makes them more stable than use c++ code.
(In reply to Sotaro Ikeda [:sotaro] from comment #38)
> Android uses java's Mediacodec(Bug 1014614, Bug 1097126) to avoid symbol
> mismatch problem on recent android. Android devices have modification to
> their native(c++ code) and it causes symbol mismatch if gecko uses c++
> MediaCodec. And android applications do not have permission to load hw codec
> in their processes. Using java api makes them more stable than use c++ code.

I'm not sure the link with this bug, but what you describe maybe the reason why we have bug 1192539.

Just attempting to create the decoder causes a crash
Comment on attachment 8677982 [details] [diff] [review]
Do not use MP3Decoder on B2G. r=alfredo

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

Well, if OMX is only used by B2G, that should be all that we need then.
Attachment #8677982 - Flags: review?(ayang)
Attachment #8677982 - Flags: review?(ayang) → review+
https://hg.mozilla.org/mozilla-central/rev/34564c10054d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
I can validate that the current build (from taskcluster) shows this regression as fixed.

https://raptor.mozilla.org/dashboard/script/power?var-device=flame-kk&var-memory=1024&var-branch=master&from=1442768969400&to=1446052303116
Depends on: 1209434
You need to log in before you can comment on or make changes to this bug.