Closed Bug 1306483 Opened 3 years ago Closed 3 years ago

Audio "skips" when using out-of-process decoder

Categories

(Firefox for Android :: Audio/Video, defect, P2)

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: snorp, Assigned: jhlin)

References

Details

Attachments

(2 files)

I've been running Nightly with the remote media codec enabled (media.android-remote-codec.enabled = true), and sometimes I have a problem with audio. It sounds like it's skipping or missing samples. It's fairly easy to reproduce, but it doesn't seem to always happen. I don't have a consistent test URL at the moment, but if I find one I'll attach to this bug.
Once bug 1295106 is fixed, it should be able to fix this one as well.
Depends on: 1295106
Priority: -- → P2
Unfortunately bug 1295106 patches mitigates but not completely eliminates audio skips in my own tests. Will continue to investigate the problem.
The methods of CallbackSender[1] call sendMessage()[2][3] to queue buffer notifications in its looper. Since Codec doesn't supply a looper for it, the BufferPoller looper is shared by both and messages for polling will delay the delivery of output buffers for AudioStream to consume. Sending notification immediately instead of queuing solves the problem.

Unfortunately, I found that on my Nexus 5, after playing video for a while (10+ min), the CPU governor will shutdown all cores but one and it starts to stutter very, very seriously. Next I will investigate what triggers CPU governor.

[1] http://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/media/JellyBeanAsyncCodec.java#71
[2] http://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/media/JellyBeanAsyncCodec.java#90
[3] http://searchfox.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/media/JellyBeanAsyncCodec.java#100
From kernel log, it looks like the cores were shut down by thermal regulator. :(

<6>[83408.055426] msm_thermal: Set Offline: CPU3 Temp: 81
<6>[83408.305412] msm_thermal: Set Offline: CPU2 Temp: 80
<6>[83408.555443] msm_thermal: Set Offline: CPU1 Temp: 81
Comment on attachment 8803260 [details]
Bug 1306483 - Part 1: use concurrent queue to fix race condition.

https://reviewboard.mozilla.org/r/87558/#review87068
Attachment #8803260 - Flags: review?(snorp) → review+
Comment on attachment 8803261 [details]
Bug 1306483 - Part 2: send notification immediately when caller sender shares looper with buffer poller.

https://reviewboard.mozilla.org/r/87560/#review87076

Decoding audio shouldn't cause thermal throttling, there must be an issue somewhere else.

::: mobile/android/base/java/org/mozilla/gecko/media/JellyBeanAsyncCodec.java:97
(Diff revision 2)
> +
> +        private void processMessage(Message msg) {
> +            if (Looper.myLooper() == getLooper()) {
> +                handleMessage(msg);
> +            } else {
> -            sendMessage(msg);
> +                sendMessage(msg);

Remove ">>>>".

::: mobile/android/base/java/org/mozilla/gecko/media/JellyBeanAsyncCodec.java:98
(Diff revision 2)
> +        private void processMessage(Message msg) {
> +            if (Looper.myLooper() == getLooper()) {
> +                handleMessage(msg);
> +            } else {
> -            sendMessage(msg);
> +                sendMessage(msg);
> -        }
> +            }

Remove ">>>>".
Attachment #8803261 - Flags: review?(esawin) → review+
Weird, review board showed ">>>>" explicitly, bug?
Comment on attachment 8803261 [details]
Bug 1306483 - Part 2: send notification immediately when caller sender shares looper with buffer poller.

https://reviewboard.mozilla.org/r/87560/#review87076

It takes longer but happens when pref is off too. Doesn't happen on Z3C or Nexus 6. Maybe Nexus 5 thermal engine has more aggressive settings?s

> Remove ">>>>".

Downloaded diff/patch file and couldn't see '>>>>'. I guess it's the Reviewboard diff viewer showing added whitespaces.
Pushed by jolin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/77c515e8350a
Part 1: use concurrent queue to fix race condition. r=snorp
https://hg.mozilla.org/integration/autoland/rev/9f2f66e3762f
Part 2: send notification immediately when caller sender shares looper with buffer poller. r=esawin
Assignee: nobody → jolin
https://hg.mozilla.org/mozilla-central/rev/77c515e8350a
https://hg.mozilla.org/mozilla-central/rev/9f2f66e3762f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
You need to log in before you can comment on or make changes to this bug.