1131479 - [FFOS7715 v2.1][AudioChannel]Music will pause when we open the camera

Status: VERIFIED FIXED

(Firefox OS Graveyard :: AudioChannel, defect)

Description

[jingmei.zhang]

STR:
1.Play music
2.open camera

result:

music stop when camera is opened.

expect result:
music will resume when we open the camera without play something.

Comment 1

[jingmei.zhang]

Hi Alastor,

Need your help :(
Play music:
I/* JINGMEI * ==>( 6522): AudioChannelAgent::StartPlaying mAudioChannelType 0 service 0 mIsRegToService 0
I/* JINGMEI * ==>( 6522): AudioChannelService::RegisterAudioChannelAgent
I/* JINGMEI * ==>( 6522): AudioChannelService::RegisterType 1
I/* JINGMEI * ==>( 6184): AudioChannelService::RegisterType 1

go to camera:
I/* JINGMEI * ==>( 6463): AudioChannelAgent::StartPlaying  0
I/* JINGMEI * ==>( 6463): AudioChannelService::RegisterAudioChannelAgent
I/* JINGMEI * ==>( 6463): AudioChannelService::RegisterType 0
I/* JINGMEI * ==>( 6184): AudioChannelService::RegisterType 0

Music is stopped because Type 0 is called to play when go to camera.

It might be more reasonable to resume play music when we go to the camera,right?

Comment 2

[Alastor Wu [:alwu]]

Hi, JingMei,
Try about this patch, there is also similar problem.
Maybe the camera app active some audio channels at wrong time.
https://bugzilla.mozilla.org/show_bug.cgi?id=1126592#c9

Comment 3

[jingmei.zhang]

(In reply to Alastor Wu [:alwu] from comment #2)

Yes,it could works.Thanks. ^_^

This should be a workaround one in 2.1.

Comment 4

[jingmei.zhang]

Hi Alastor,

I check the patch in another case:
1.play music
2.open fm
3.put both fm & music in background.

There comes the issue:fm & music play in background at the same time.

Fm&music share the same Audiochannel:content,So the patch need to improve,can you help to check?

Comment 5

[Alastor Wu [:alwu]]

OK, I would find another way to fix it.
Keep ni for tracking this.

Comment 6

[Alastor Wu [:alwu]]

Attachment: Check stream type

Hi, JingMei,
You can try this patch.

The root cause is the same as this bug. You can see it for more details.
https://bugzilla.mozilla.org/show_bug.cgi?id=1069109#c8

Comment 7

[jingmei.zhang]

(In reply to Alastor Wu [:alwu] from comment #6)

Thank you !

I have checked the patch & land it in our locale for 2.1 :)

What's more,Happy for the incoming new year~~~~~~~~~~~~

Comment 8

[Alastor Wu [:alwu]]

Thanks :)
I think this should be merged into V2.0, I would find a reviewer to review it.

Comment 9

[Alastor Wu [:alwu]]

Attachment: Check stream type

Hi, Sotaro,
Sorry to bother you,
Could you help me to review this patch?
This issue is the same as the Bug1069109 which you have solved before.

I change the |mPaused| to the front of the ChangeReadyState(), because this variable would be referenced in the ChangeReadyState().

Thanks a lots :)

Comment 10

[Sotaro Ikeda [:sotaro]]

(In reply to Alastor Wu [:alwu] from comment #9)
> Created attachment 8568417 [details] [diff] [review]
> Check stream type
> 
> I change the |mPaused| to the front of the ChangeReadyState(), because this
> variable would be referenced in the ChangeReadyState().

If the change to |mPaused| is necessay. Can you also apply the change to master?

Comment 11

[Sotaro Ikeda [:sotaro]]

Comment on attachment 8568417 [details] [diff] [review]
Check stream type

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

Forward a review to cajbir.

Comment 12

[Alastor Wu [:alwu]]

Hi, Sotaro,
The changing of |mPaused| would be applied in the Bug1119936 :)

Hi, Cajbir,
Could you help me review this patch?
Thanks a lots!

Comment 13

[Alastor Wu [:alwu]]

Comment on attachment 8568417 [details] [diff] [review]
Check stream type

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Lack of the stream type checking

User impact if declined: Because this problem have already been fixed in m-c, but the v2.1 version doesn't have the changeset. Therefore, I hope we can merge this to the V2.1 to prevent similar issues.

Testing completed: 
You can compare these patches, one is with try message, another is with the changeset.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=23b7bc5bcd2c
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ec53ff661ef

Risk to taking this patch (and alternatives if risky): 
Seems no risk, because the changeset have already been merged in m-c.
See https://bugzilla.mozilla.org/show_bug.cgi?id=1069109#c8

String or UUID changes made by this patch: None

Comment 14

[jingmei.zhang]

Hi Alastor,

It seems there is a Spelling mistakes in function name 'GetTrackTypesAvailable'

>+        !(mSrcStream && !(mSrcStream->GetTrackTypesAvaliable() &

Comment 15

[Alastor Wu [:alwu]]

Attachment: Check stream type. r=cajbir

Modify spelling mistake.

[Approval Request Comment]
See Comment13.

Comment 16

[bhavana bajaj [:bajaj]]

:alwu, do we need this on b2g37 as well ? Does the problem happen there?

Comment 17

[Alastor Wu [:alwu]]

Hi, Bhavana,
This issue doesn't be found on b2g37, so I think we only need this on b2g34 :)

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8e3bf21a8879

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/8e3bf21a8879

Comment 20

[Josh Cheng [:josh]]

Hi Kai-Zhen,
Could you help to land this on 2.0M? Thanks!

Comment 22

[Kai-Zhen Li [:seinlin][:kli]]

Alastor,
Uplift into 2.0m branch get conflicts. Could you rebase a patch for v2.0m?
Please ni me to merge when you finish it.
Thanks!

Comment 23

[Alastor Wu [:alwu]]

Attachment: [v2.0m] Check stream type. r=cajbir

Rebase for v2.0m.
Thanks!

Comment 24

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/a2632d2457db

Comment 25

[Shally Li（leave from Mozilla）]

This bug has been verified as pass on latest build of Woodduck v2.0, Flame v2.1 and v2.1S_512mb by the STR in Comment 0.

Actual results: Music continues to play when you enter Camera app.
See attachment: verified_v2.1.mp4
Reproduce rate: 0/5


Device: Woodduck v2.0 build(Pass)
Build ID               20150601050314
Gaia Revision          f1e51289f830a759c433c238521dd65eedbee157
Gaia Date              2015-05-27 19:33:31
Gecko Revision         6e1bada60fa18f3dd552db7618582879964747d7
Gecko Version          32.0
Device Name            jrdhz72_w_ff
Firmware(Release)      4.4.2
Firmware(Incremental)  1433106288
Firmware Date          Mon Jun  1 05:05:07 CST 2015

Device: Flame v2.1 build(Pass)
Build ID               20150601001204
Gaia Revision          2304a1f6327c2ccf35d6995ee16f2231ed1f22a3
Gaia Date              2015-05-26 13:30:41
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/e52807dee101
Gecko Version          34.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150601.035649
Firmware Date          Mon Jun  1 03:57:00 EDT 2015
Bootloader             L1TC000118D0

Device: v2.1S_512mb (Pass)
Build ID               20150601001204
Gaia Revision          0493b4dbb59f9617c1c1a45f422303c2aff32a9a
Gaia Date              2015-05-27 19:33:12
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/1aae9fc735b5
Gecko Version          34.0
Device Name            scx15_sp7715ea
Firmware(Release)      4.4.2
Firmware(Incremental)  122
Firmware Date          Thu Feb  5 12:42:58 CST 2015

Note:
By the STR in Comment 4, when both FM radio and Music are opened in background, only one of them is played.

Comment 26

[Shally Li（leave from Mozilla）]

Attachment: verified_v2.1.mp4