Closed Bug 1131479 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.0 wontfix, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.1S verified, b2g-v2.2 unaffected, b2g-master unaffected)

VERIFIED FIXED
2.2 S8 (20mar)
Tracking Status
b2g-v2.0 --- wontfix
b2g-v2.0M --- verified
b2g-v2.1 --- verified
b2g-v2.1S --- verified
b2g-v2.2 --- unaffected
b2g-master --- unaffected

People

(Reporter: jingmei.zhang, Assigned: alwu)

References

Details

Attachments

(3 files, 2 obsolete files)

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.
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?
Flags: needinfo?(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
Flags: needinfo?(alwu)
(In reply to Alastor Wu [:alwu] from comment #2)

Yes,it could works.Thanks. ^_^

This should be a workaround one in 2.1.
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?
Flags: needinfo?(alwu)
OK, I would find another way to fix it.
Keep ni for tracking this.
Attached patch Check stream type (obsolete) — Splinter Review
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
Flags: needinfo?(alwu)
(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~~~~~~~~~~~~
Thanks :)
I think this should be merged into V2.0, I would find a reviewer to review it.
Assignee: nobody → alwu
Attached patch Check stream type (obsolete) — Splinter Review
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 :)
Attachment #8564024 - Attachment is obsolete: true
Attachment #8568417 - Flags: review?(sotaro.ikeda.g)
(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?
Flags: needinfo?(alwu)
Comment on attachment 8568417 [details] [diff] [review]
Check stream type

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

Forward a review to cajbir.
Attachment #8568417 - Flags: review?(sotaro.ikeda.g) → review?(cajbir.bugzilla)
Hi, Sotaro,
The changing of |mPaused| would be applied in the Bug1119936 :)

Hi, Cajbir,
Could you help me review this patch?
Thanks a lots!
Flags: needinfo?(alwu)
Attachment #8568417 - Flags: review?(cajbir.bugzilla) → review+
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
Attachment #8568417 - Flags: approval-mozilla-b2g34?
Hi Alastor,

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

>+        !(mSrcStream && !(mSrcStream->GetTrackTypesAvaliable() &
Modify spelling mistake.

[Approval Request Comment]
See Comment13.
Attachment #8568417 - Attachment is obsolete: true
Attachment #8568417 - Flags: approval-mozilla-b2g34?
Attachment #8572963 - Flags: approval-mozilla-b2g34?
Attachment #8572963 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
:alwu, do we need this on b2g37 as well ? Does the problem happen there?
Flags: needinfo?(alwu)
Hi, Bhavana,
This issue doesn't be found on b2g37, so I think we only need this on b2g34 :)
Flags: needinfo?(alwu)
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8e3bf21a8879
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
Hi Kai-Zhen,
Could you help to land this on 2.0M? Thanks!
Blocks: Woodduck
Flags: needinfo?(kli)
Duplicate of this bug: 1142397
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!
Flags: needinfo?(kli)
Flags: needinfo?(alwu)
Rebase for v2.0m.
Thanks!
Flags: needinfo?(alwu) → needinfo?(kli)
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.
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.