Closed Bug 860760 Opened 7 years ago Closed 7 years ago

[Music] Power consumption is always the same ,irrespective of music paused or played out

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

RESOLVED FIXED
1.1 QE1 (5may)
blocking-b2g leo+
Tracking Status
firefox21 --- wontfix
firefox22 --- wontfix
firefox23 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix

People

(Reporter: diego, Assigned: diego)

References

Details

(Whiteboard: [fixed-in-birch])

Attachments

(2 files)

1. Title :  When Music is paused, power consumed is always more or less the same
2. Precondition : have a Couple of music files 
3. Tester's Action : play MP3 -> stop MP3-> power key press (lcd, backlight turn off) -> measure sleep current
4. Detailed Symptom (ENG.) : When music is paused and LCD screen set to OFF , Sleep current almost remains the same.
5. Gaia Revision #:   " a78ebf426840b5ef08c0cc3e437ad30aba3e2528 "
6. Expected : When music is not played out, power consumption should be comparatively less
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
	1)Model Comparing : 
9. Attached files: 
	1)Log : 
	2)Test Contents : 
	3)Video file :
I am thinking that how about the CPU usage when the music is paused? My device is out of battery now. Will try it later.
OMXCodec needs to now when the stream is paused. Otherwise it will prevent the device from lowering its power state when idle.
Attachment #736403 - Flags: feedback?(sotaro.ikeda.g)
Attachment #736403 - Flags: feedback?(chris.double)
For landing on mozilla-b2g18 we need to pull in OnDecodeThreadFinish() and OnDecodeThreadStart() to nsBuiltinDecoderStateMachine from the trunk
Attachment #736406 - Flags: feedback?(sotaro)
Attachment #736406 - Flags: feedback?(chris.double)
Attachment #736406 - Flags: feedback?(sotaro) → feedback?(sotaro.ikeda.g)
blocking-b2g: --- → leo?
Status: NEW → ASSIGNED
Comment on attachment 736403 [details] [diff] [review]
Pause OMX media sources when playback is paused - v2

looks good.
Attachment #736403 - Flags: feedback?(sotaro.ikeda.g) → feedback+
Comment on attachment 736406 [details] [diff] [review]
Pause OMX media sources when playback is paused - mozilla-b2g18 - v2

looks good.
Attachment #736406 - Flags: feedback?(sotaro.ikeda.g) → feedback+
(leo+, blocks a blocker)
blocking-b2g: leo? → leo+
Attachment #736403 - Flags: review?(sotaro.ikeda.g)
Attachment #736403 - Flags: review?(chris.double)
Attachment #736403 - Flags: feedback?(chris.double)
Attachment #736403 - Flags: feedback-
Attachment #736403 - Flags: review?(chris.double) → review+
Attachment #736406 - Flags: feedback?(chris.double) → feedback+
Attachment #736403 - Flags: review?(sotaro.ikeda.g) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0d95450aee43
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-birch]
Target Milestone: --- → Leo QE1 (5may)
Attachment #736403 - Flags: feedback-
(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> Comment on attachment 736406 [details] [diff] [review]
> Pause OMX media sources when playback is paused - mozilla-b2g18 - v2
> 
> looks good.

There is a hidden trouble. the pause() is not a pure virtual interface like stop().
Not of all Decoders has been implement it. the Decoder would be crashed when resume, because of start() reentry.
what is better suggestion?

crash stack as follow:
 0  libc.so!__libc_android_abort [abort.c : 82 + 0x0]
 1  liblog.so!__android_log_assert [logd_write.c : 246 + 0x3]
 2  libstagefright.so!android::AACSPRDDecoder::start [AACSPRDDecoder.cpp : 140 + 0xf]
 3  libxul.so!android::OmxDecoder::Play [OmxDecoder.cpp : 693 + 0x7]
 4  libxul.so!nsMediaOmxReader::OnDecodeThreadStart [nsMediaOmxReader.cpp : 337 + 0x3]
Or
0  libc.so!__libc_android_abort [abort.c : 82 + 0x0]
 1  liblog.so!__android_log_assert [logd_write.c : 246 + 0x3]
 2  libstagefright.so!android::MP3Decoder::start [MP3SPRDDecoder.cpp : 82 + 0xf]
 3  libxul.so!android::OmxDecoder::Play [OmxDecoder.cpp : 693 + 0x7]
 4  libxul.so!nsMediaOmxReader::OnDecodeThreadStart [nsMediaOmxReader.cpp : 337 + 0x3]
Gecko need test 'pause' interface result, because not all vendors implement this interface.
Thanks for reporting. Can you file a new bug for it?
(In reply to cheng.luo from comment #10)
> crash stack as follow:
>  0  libc.so!__libc_android_abort [abort.c : 82 + 0x0]
>  1  liblog.so!__android_log_assert [logd_write.c : 246 + 0x3]
>  2  libstagefright.so!android::AACSPRDDecoder::start [AACSPRDDecoder.cpp :
> 140 + 0xf]
>  3  libxul.so!android::OmxDecoder::Play [OmxDecoder.cpp : 693 + 0x7]
>  4  libxul.so!nsMediaOmxReader::OnDecodeThreadStart [nsMediaOmxReader.cpp :
> 337 + 0x3]
> Or
> 0  libc.so!__libc_android_abort [abort.c : 82 + 0x0]
>  1  liblog.so!__android_log_assert [logd_write.c : 246 + 0x3]
>  2  libstagefright.so!android::MP3Decoder::start [MP3SPRDDecoder.cpp : 82 +
> 0xf]
>  3  libxul.so!android::OmxDecoder::Play [OmxDecoder.cpp : 693 + 0x7]
>  4  libxul.so!nsMediaOmxReader::OnDecodeThreadStart [nsMediaOmxReader.cpp :
> 337 + 0x3]

The above call stack is not what I expected. Why OmxDecoder::Play() calls MP3Decoder::start()/AACSPRDDecoder::start()? OmxDecoder::Play() should call OMXCodec::start(). It is the android ICS's default implementation. The above stacks seems like pre ICS era. Firefox OS v1.01 and v1.1 expect ICS.
(In reply to Sotaro Ikeda [:sotaro] from comment #13)
> OmxDecoder::Play() should call
> OMXCodec::start(). It is the android ICS's default implementation. The above
> stacks seems like pre ICS era. Firefox OS v1.01 and v1.1 expect ICS.

It seems you are right after discussing with our Decoder developer. the MP3Decoder
and AACSPRDDecoder inherited from pre ICS version. With OMX.google.mp3.decoder and OMX.google.aac.decoder which are google's raw Decoder, it could run OK,Although the sound quality is not very good。
We will adapt it to ICS,but have a question:Is ICS's default implementation of OMXCodec::pause() enough?Is it necessary to add OMX_StatePause for OMXCodec?
If use google native MP3/AAC decoder, play/pause/play/pause/play, music will stuck.
Pls check the OMXCodec log as below.

I/OMXCodec(  324): [OMX.google.mp3.decoder] init begin
I/OMXCodec(  324): [OMX.google.mp3.decoder] setState(LOADED_TO_IDLE) ! kRequiresLoadedToIdleAfterAllocation
I/OMXCodec(  324): [OMX.google.mp3.decoder] Now Idle.  mState(0), mState
I/OMXCodec(  324): [OMX.google.mp3.decoder] Now Executing.
I/OMXCodec(  324): [OMX.google.mp3.decoder] init done
E/OMXCodec(  324): [OMX.google.mp3.decoder] Timed out waiting for output buffers: 4/0
E/OMXCodec(  324): [OMX.google.mp3.decoder] Timed out waiting for output buffers: 4/0
E/OMXCodec(  324): [OMX.google.mp3.decoder] Timed out waiting for output buffers: 4/0
E/OMXCodec(  324): [OMX.google.mp3.decoder] Timed out waiting for output buffers: 4/0
E/OMXCodec(  324): [OMX.google.mp3.decoder] Timed out waiting for output buffers: 4/0
E/OMXCodec(  324): [OMX.google.mp3.decoder] Timed out waiting for output buffers: 4/0

I'll file a new bug.
Blocks: 872462
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap-
Looking at the implementation of OMXCodec::pause, I can't understand how it is related to power consumption for it just set mPaused to true.
Flags: needinfo?(dwilson)
Btw, this bug might have something to do with the timeouts in Bug 916135.
Please check https://bugzilla.mozilla.org/show_bug.cgi?id=916135#c19, root cause #3.

"Implementation of OMXCodec::pause is broken. Calling OMXCodec::start later will return error and can't leave the paused state."
(In reply to JW Wang[:jwwang] from comment #16)
> Looking at the implementation of OMXCodec::pause, I can't understand how it
> is related to power consumption for it just set mPaused to true.

The OMXCodec:pause() is ICS does a bit more than that [1]. Certain codecs go into a lower power mode when it's called.

[1] http://smaug.qualcomm.com:8080/source/xref/b2g_ics_1.2/frameworks/base/media/libstagefright/OMXCodec.cpp#6150
Flags: needinfo?(dwilson)
As I remember that Fugu device already help to return non-supported in OMXCodec::Pause() so Gecko wouldn't call OMXCodec::Start() later.

So we can close this discussion.
You need to log in before you can comment on or make changes to this bug.