Add support for dev input jack events

RESOLVED FIXED

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: m1, Assigned: m1)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

GonkSwitch.cpp currently uses a uevent [1] to detect headset/headphone jack insertion/removal, yet not all kernels support this.

An alternative is to use the SW_HEADPHONE_INSERT / SW_MICROPHONE_INSERT input events, yet not all kernels support this and some known kernels running B2G have buggy implementations of these two events.

So we need Gecko support for both, but the dev input events should probably be disabled by default and enabled only by a device system property for now.  

[1] http://dxr.mozilla.org/mozilla-central/source/hal/gonk/GonkSwitch.cpp#l34
Posted patch v0 (obsolete) — Splinter Review
The nsAppShell->GonkSwitch linkage is a little gross, f welcome!
Attachment #778140 - Flags: feedback?(mwu)
See also bug 835211 which I believe was also trying to do the same thing.
Duplicate of this bug: 835211
The SwitchEventRunnable stuff in https://bug835211.bugzilla.mozilla.org/attachment.cgi?id=707388 looks nicer than the hal_impl::NotifyInputSwitch() stuff in attachment 778140 [details] [diff] [review], I'll probably play with that in a day or two unless you'd like to give it a go Alan?   We should be able to make this a patch only to nsAppShell.
(In reply to Michael Vines [:m1] [:evilmachines] from comment #4)
Oh, I was a little confused about https://bugzilla.mozilla.org/show_bug.cgi?id=835211#c9, now I think I know what you mean and attempt to do. Feel free to take it :)
Posted patch v1 (obsolete) — Splinter Review
Attachment #778140 - Attachment is obsolete: true
Attachment #778140 - Flags: feedback?(mwu)
Attachment #779453 - Flags: review?(mwu)
Attachment #779453 - Flags: feedback?(ahuang)
Comment on attachment 779453 [details] [diff] [review]
v1

Hello Randy,
As I know in bug 893516, you've been working on headphone/headset states in AudioManager and gonk. Can you give some feedback for this patch, too? Many thanks.
Attachment #779453 - Flags: feedback?(ahuang) → feedback?(rlin)
Comment on attachment 779453 [details] [diff] [review]
v1

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

This patch looks good to me.
Attachment #779453 - Flags: feedback?(rlin) → feedback+
Comment on attachment 779453 [details] [diff] [review]
v1

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

This is definitely the nicest looking implementation for this I've seen so far. Wish we could just pick either kernel events or input events though.

::: widget/gonk/nsAppShell.cpp
@@ +80,5 @@
>  static int epollfd = 0;
>  static int signalfds[2] = {0};
> +static bool sDevInputAudioJack;
> +static int32_t sHeadphoneState = AKEY_STATE_UNKNOWN;
> +static int32_t sMicrophoneState = AKEY_STATE_UNKNOWN;

I would prefer initializing these in InitInputDevices, or leaving them at 0 if possible.

@@ +720,5 @@
>  
> +    if (sDevInputAudioJack) {
> +        sHeadphoneState  = mReader->getSwitchState(-1, AINPUT_SOURCE_SWITCH, SW_HEADPHONE_INSERT);
> +        sMicrophoneState = mReader->getSwitchState(-1, AINPUT_SOURCE_SWITCH, SW_MICROPHONE_INSERT);
> +        updateHeadphoneSwitch();

Does this let us recover after a crash, or does it just grab events that came in during boot?

@@ +745,5 @@
>  {
> +    char value[PROPERTY_VALUE_MAX];
> +    property_get("ro.moz.devinputjack", value, "0");
> +    if (!strcmp(value, "1")) {
> +        sDevInputAudioJack = true;

sDevInputAudioJack = !strcmp(value, "1");
Attachment #779453 - Flags: review?(mwu) → review+
Posted patch v2.patchSplinter Review
Review comments addressed, carrying forward r+
Attachment #779453 - Attachment is obsolete: true
Attachment #782942 - Flags: review+
(In reply to Michael Wu [:mwu] from comment #9)
> I would prefer initializing these in InitInputDevices, or leaving them at 0
> if possible.

Moved to InitInputDevices().  Sadly AKEY_STATE_UNKNOWN != 0

> Does this let us recover after a crash, or does it just grab events that
> came in during boot?

Yes!
https://hg.mozilla.org/mozilla-central/rev/9262cbb74ed5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Duplicate of this bug: 893516
You need to log in before you can comment on or make changes to this bug.