Closed
Bug 835211
Opened 11 years ago
Closed 11 years ago
AudioManager cannot set correct device connection state on some platforms
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 895665
People
(Reporter: alan.yenlin.huang, Assigned: alan.yenlin.huang)
Details
Attachments
(1 file, 2 obsolete files)
9.08 KB,
patch
|
Details | Diff | Splinter Review |
For platform with headset/head phone plug/unplug events like below, AudioManager cannot set correct device connection state when unplug. $ adb shell su -- getevent -lt /dev/input/event6 4181-378357: EV_SW SW_HEADPHONE_INSERT 00000001 4181-378357: EV_SW SW_MICROPHONE_INSERT 00000001 4181-378387: EV_SYN SYN_REPORT 00000000 4183-834026: EV_SW SW_HEADPHONE_INSERT 00000000 4183-834026: EV_SW SW_MICROPHONE_INSERT 00000000 4183-834026: EV_SYN SYN_REPORT 00000000 I think it is caused by that (in InternalSetAudioRoutesICS, AudioSystem.cpp) we OR switch state(s) when AUDIO_POLICY_DEVICE_STATE_AVAILABLE, but we didn't use AND to check switch flags when AUDIO_POLICY_DEVICE_STATE_UNAVAILABLE.
Assignee | ||
Comment 1•11 years ago
|
||
Hi Michael, We met this bug when porting b2g to QCT msm8960 platform. Can you help to review this patch? Thanks.
Attachment #706953 -
Flags: review?(mwu)
Assignee | ||
Comment 2•11 years ago
|
||
Attach full patch, including Event hub part and implementation of GeckoInputDispatcher::notifySwitch. I port EventHub part from android Eventhub for headset using input device instead of uevent.
Attachment #706953 -
Attachment is obsolete: true
Attachment #706953 -
Flags: review?(mwu)
Attachment #706996 -
Flags: review?(mwu)
Comment 3•11 years ago
|
||
Comment on attachment 706996 [details] [diff] [review] proposed fix, v2 Review of attachment 706996 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/AudioManager.cpp @@ +94,5 @@ > AudioSystem::setDeviceConnectionState(AUDIO_DEVICE_OUT_WIRED_HEADPHONE, > AUDIO_POLICY_DEVICE_STATE_AVAILABLE, ""); > sHeadsetState |= AUDIO_DEVICE_OUT_WIRED_HEADPHONE; > } else if (aState == SWITCH_STATE_OFF) { > + if (sHeadsetState && AUDIO_DEVICE_OUT_WIRED_HEADSET) { Did you really mean to use &&?
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #706996 -
Attachment is obsolete: true
Attachment #706996 -
Flags: review?(mwu)
Attachment #707388 -
Flags: review?(mwu)
Assignee | ||
Comment 5•11 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #3) > Comment on attachment 706996 [details] [diff] [review] > proposed fix, v2 > > Review of attachment 706996 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/AudioManager.cpp > @@ +94,5 @@ > > AudioSystem::setDeviceConnectionState(AUDIO_DEVICE_OUT_WIRED_HEADPHONE, > > AUDIO_POLICY_DEVICE_STATE_AVAILABLE, ""); > > sHeadsetState |= AUDIO_DEVICE_OUT_WIRED_HEADPHONE; > > } else if (aState == SWITCH_STATE_OFF) { > > + if (sHeadsetState && AUDIO_DEVICE_OUT_WIRED_HEADSET) { > > Did you really mean to use &&? No, that's a mistake. I've updated patch with correct logical AND. Please help me to review it, thanks!
Comment 6•11 years ago
|
||
This fix does not match the bug description. Why are you adding a new source of switch events? The EventHub.cpp patch you have is a codeaurora specific patch. We try to stick to AOSP upstream copies. Also, there's only one kind of logical AND. && = logical AND & = bitwise AND
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #6) > Also, there's only one kind of logical AND. > > && = logical AND > & = bitwise AND Thank you for correcting my terminology. I intend to use bitwise AND in this patch. > This fix does not match the bug description. Why are you adding a new source > of switch events? The EventHub.cpp patch you have is a codeaurora specific > patch. We try to stick to AOSP upstream copies. Unlike unagi, some partner's platform uses /dev/input for headset/microphone, so we have to add a new source of switch events. Then we found this bug: when insert headphone, both SW_HEADPHONE_INSERT and SW_MICROPHONE_INSERT events are reported. When remove headset, not all connection state are disconnected. (or, maybe it would be better that I change bug title?) I agree it would be better that we stick to AOSP copies. Do you have some good idea to make headset work in this case? Thank you.
Comment 8•11 years ago
|
||
Hey, please take look at the patch on bug 895665. Looks like we may be trying to fix the same bug.
Comment 9•11 years ago
|
||
Comment on attachment 707388 [details] [diff] [review] proposed fix, v2 Review of attachment 707388 [details] [diff] [review]: ----------------------------------------------------------------- We should be able to avoid the EventHub changes in this patch by making nsAppShell.cpp a little smarter. There are also existing devices that send buggy dev input jack events, so I really think we need this pref-ed off by default ("ro.moz.devinputjack=1").
Assignee | ||
Comment 10•11 years ago
|
||
Thanks, Michael. I believe what you do in nsAppShell.cpp on bug 895665 is better than what I attempt to do. Also, I think using read-only build property is a good idea for devices supporting this. I mark this as duplicated as 895665.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → DUPLICATE
Assignee | ||
Updated•11 years ago
|
Attachment #707388 -
Flags: review?(mwu)
You need to log in
before you can comment on or make changes to this bug.
Description
•