FM App does not recognize wired headset for the first time after boot up

RESOLVED FIXED in Firefox 28

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: vasanth, Assigned: mchen)

Tracking

({regression})

unspecified
1.3 C3/1.4 S3(31jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(blocking-b2g:1.3+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

()

Attachments

(4 attachments, 1 obsolete attachment)

Reporter

Description

6 years ago
Steps to reproduce:
===================
1. Plug in wired Headset to device
2. Reboot the device.
3. After device is powered up, go to FM app.
4. FM  does not start and there is notification on device to plug in headset in order to make FM on. Headset has to be unplugged and plugged in to make FM on.

Initial analysis:
================
Headset is detected by the device, but only FM app does not detect the headset for the first time after reboot. Please see attached screenshot

Only when there is a change in headset state, FM app is notified [1].
When headset is connected before power on and when FM app is opened after that, now since there is no change in headset state, it doesn't know headset is connected

Not sure if there is any trivial fix?

[1] http://mxr.mozilla.org/mozilla-central/source/dom/fmradio/FMRadio.cpp#192
Reporter

Updated

6 years ago
Blocks: 942267
blocking-b2g: --- → 1.3?
I've seen this bug as well. Probably a recent regression.
blocking-b2g: 1.3? → 1.3+
Assignee

Comment 2

6 years ago
Hi Pin,

Refer to the status bar, system app already knew headset plugged in.
So could you help to check what happened inside FMRadip app?

Thanks.
Flags: needinfo?(pzhang)

Updated

6 years ago
QA Contact: jzimbrick

Comment 3

6 years ago
I have not been able to reproduce this issue on the latest mozilla 1.3 and 1.4 builds on a Buri device with the information provided. The phone recognizes the headphones on first boot every time. 

1.3 Environmental Variables:
Device: Buri v1.3 Mozilla RIL
uildID: 20140113004002
Gaia: b3fc4f712562ee92b0ed0bd17abc61be9a36a8da
Gecko: 5bb1837de7c0
Version: 28.0a2
Base Image: V1.2_20131115

1.4 Environmental Variables :
Device: Buri v1.4 Mozilla RIL
BuildID: 20140113040202
Gaia: fd3b9a97cb3c41cfa56be387b46a51db136b4422
Gecko: 12d3ba62a599
Version: 29.0a1
Base Image: V1.2_20131115



Could this issue be device (or even headphone) specific? 
What device, build, and base image was this issue seen on?
Was the reproduction rate on this issue 100%, or lower?
Flags: needinfo?(vasanth)
Vasanth, does this reproduce on the QRD 7x27a w/ v1.3 Gecko/Gaia?
Reporter

Comment 5

6 years ago
(In reply to J Zimbrick from comment #3)
> The phone recognizes the headphones on first boot every time. 

I will test in some other device and update here. From your comment, I guess you imply not only the phone, even the FM app recognized the headphone for the first time.

Comment 6

6 years ago
(In reply to vasanth from comment #5)
> I will test in some other device and update here. From your comment, I guess
> you imply not only the phone, even the FM app recognized the headphone for
> the first time.

Sorry. Yes, to clarfiy: both the phone and the FM app recognized the headphones on first boot each time.
Since JZimbrick can't reproduce, clear the needinfo request here.
Flags: needinfo?(pzhang)
Reporter

Comment 8

6 years ago
We know there are 2 ways to detect headset insert/remove,
1. Using Uevents
2. Using dev input jack events.
This issue is only in devices with #2 so not reproducible in 7x27a

FM app uses GetCurrentSwtichState() to get initial state of headset which internally uses |SwitchEventObserver:: mEventInfo| to get status

Implementation of #1 stores the event in mEventInfo and also notifies the observers [1]
Whereas implementation of #2 just notifies the observers [2]
So in devices with #2, GetCurrentSwtichState() will always return -1 (status unknown)

BTW this issue was covered before, since FM app didn’t check for status unknown
My previous fix uncovered this [3] :-(

[1] http://dxr.mozilla.org/mozilla-central/source/hal/gonk/GonkSwitch.cpp#262
[2] http://dxr.mozilla.org/mozilla-central/source/widget/gonk/nsAppShell.cpp#441
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=953159
Flags: needinfo?(vasanth)
Marco, we're still tracking this one for CS -- can we make progress now based on comment #8?
Flags: needinfo?(mchen)
Assignee

Comment 10

6 years ago
Investigate it by myself first.
Assignee: nobody → mchen
Flags: needinfo?(mchen)
Assignee

Comment 11

6 years ago
Hi Vasanth,

Could you help to verify whether this patch can fix issue here because I have no such device? If it can then will start the review flow. Thanks.
Assignee

Updated

6 years ago
Flags: needinfo?(vasanth)
Reporter

Comment 12

6 years ago
(In reply to Marco Chen [:mchen] from comment #11)
> Could you help to verify whether this patch can fix issue here because I
> have no such device? If it can then will start the review flow. Thanks.

Marco,

It crashes everytime, when b2g process starts itself. Attaching the bt from gdb
Seems UnregisterSwitchObserver frees some observers and NotifySwitchChange is trying to access them.
Flags: needinfo?(mchen)
Reporter

Comment 13

6 years ago
Was able to root cause this.
SwitchEventObserver::mEventInfo.mEvent.device() is initialized only when status is not unknown [1]
Status will be unknown here in devices which doesn't use Uevent method and device value is still -1 which fails to get observers in NotifySwitchChange and crashes

Just initializing the mEvent.device before that check makes it work!

Also can't we just call hal::NotifySwitchStateFromInputDevice from UpdateHeadphoneSwitch instead of using SwitchEventRunnable there?
We already use a SwitchEventRunnable in GonkSwitch Notify()

Thanks Marco for the quick turnaround and patch.

[1] http://dxr.mozilla.org/mozilla-central/source/hal/gonk/GonkSwitch.cpp#323
Flags: needinfo?(vasanth)
Assignee

Comment 14

6 years ago
Hi Vasanth,

I guess that there is no node (ex: /sys/...) to get headset status actively by b2g process.
Could you confirm this?

Then I may add code for initializing the headset status to off when device didn't use UEVENT.
And wait for any notification from input device.
Flags: needinfo?(mchen)
Assignee

Comment 15

6 years ago
(In reply to vasanth from comment #13)
> Also can't we just call hal::NotifySwitchStateFromInputDevice from
> UpdateHeadphoneSwitch instead of using SwitchEventRunnable there?
> We already use a SwitchEventRunnable in GonkSwitch Notify()

As I know that the thread calling HAL functions can be main thread only, that's the reason input reader thread needs a runnable to main thread. And another runnable in gonkswitch is for getting status on I/O thread bug notifying on main thread.
(In reply to Marco Chen [:mchen] from comment #14)
> I guess that there is no node (ex: /sys/...) to get headset status actively
> by b2g process.
> Could you confirm this?

Yep that is the case.  There's only the input switch event.
Assignee

Comment 17

6 years ago
Hi Dave,

Could you help to review this patch?

1. The switch event notification sent by nsAppShell didn't be recorded by GonkSwitch class so HAL::GetCurrentSwitchState() can't get correct status.

2. This patch introduced a new HAL::NotifySwitchStateFromInputDevice(...) and all modules who detected switch status change should call it to update GonkSwitch for recording the current status.

Thanks.
Attachment #8361535 - Attachment is obsolete: true
Attachment #8362419 - Flags: review?(dhylands)
Reporter

Comment 18

6 years ago
Hi Dave Hylands & Marco, Could you please provide an update here? This needs to be closed soon
Flags: needinfo?(mchen)
Comment on attachment 8362419 [details] [diff] [review]
SendHeadsetStateFromInputDevToGonkSwitch-v2

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

So the patch looks fine to me.

Which phones use the "input" method to determine headphone jack state?

The only thing I didn't like was adding a new property. It would be better if it could be done without having to do that, but that can be improved in a later bug.
Attachment #8362419 - Flags: review?(dhylands) → review+
(In reply to Dave Hylands [:dhylands] from comment #19)
> The only thing I didn't like was adding a new property. It would be better
> if it could be done without having to do that, but that can be improved in a
> later bug.

The property was added in bug 895665 actually.  It should definitely be possible to determine the jack event source at runtime though as a follow-up bug.
I can confirm that attachment 8362419 [details] [diff] [review] resolves the bug over here.  Thanks, and please land in v1.3 ASAP
Assignee

Comment 23

6 years ago
It seems that all b2g build on try server are failed always.
Will try to run it tomorrow or I will push to in-bound then monitor the status.
Assignee

Comment 25

6 years ago
Carry reviewer's name.
Attachment #8365148 - Flags: review+
Assignee

Comment 27

6 years ago
(In reply to Dave Hylands [:dhylands] from comment #19)
> The only thing I didn't like was adding a new property. It would be better
> if it could be done without having to do that, but that can be improved in a
> later bug.

Fired bug 964154 as follow up to this suggestion.
https://hg.mozilla.org/mozilla-central/rev/785c3b0d53b7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.