1183033 - [B2G] Keyboard doesn't have click sound

Status: VERIFIED FIXED

(Firefox OS Graveyard :: AudioChannel, defect, P1)

Description

[Alastor Wu [:alwu]]

After using the new audio channel architecture, the keyboard click sound can't work normally.

Comment 1

[Alastor Wu [:alwu]]

Attachment: Stop agent after the audio channel is activated by system app

[Bug root cause]
In present, when the input audio chunk is null or muted, we would stop the audio channel agent of the Audio Destination Node.

However, the agent might not been activated yet by the system app.

So, we would get this situation,
1. agent call StartPlaying()
   - add agent to window data
2. send notification to system app
   - default is muted, need to wait for the response of the system app
3. agent call StopPlaying()
   - remove agent to window data
4. system app call SetAudioChannelMute()
   - there is no agent in window data, that is a useless operation

[Solution]
Therefore, I postpone the StopPlaying() until the agent really gets the response from the system app. 

---

Hi, Baku,
Could you help me review this patch?
Thanks :)

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Is this related to bug 1180614?

Comment 3

[Alastor Wu [:alwu]]

Different parts but related, this bug is depend on bug1180614.

Comment 4

[Andrea Marchesini [:baku]]

Comment on attachment 8632706 [details] [diff] [review]
Stop agent after the audio channel is activated by system app

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

I want to propose a different approach, can you tell me if this makes sense?

0. Create a Mutedtype like this: enum { MutedByAudioChannel = 1, MutedByStream = 2 }
1. Let's be 'muted' uint32_t and use it as a bitmap.
2. by default this muted is equal to: AudioChannelService::MutedByDefault()
3. When you receive WindowVolumeChanged(volume, muted == true) then:
 3a. send the notification and set the muted to |= MutedByAudioChannel
4. When the stream is muted because no inputs (InputMuted(true/false)) then do muted |= MutedByStream
 4a. and call NotifyStoppedPlaying.

This should fix the problem, correct?
It's a bit more work, but it's similar to what we do in HTMLMediaElement.

::: dom/media/webaudio/AudioDestinationNode.cpp
@@ +670,5 @@
>      return;
>    }
>  
>    if (aMuted) {
> +    // Sometime we would stop the agent before the system app opens it, because

I don't like all of this. It's too b2g specific.

Comment 6

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Carrying blocking+ from bug 1189115.

Comment 7

[Alastor Wu [:alwu]]

(In reply to Andrea Marchesini (:baku) from comment #4)
> 
> I want to propose a different approach, can you tell me if this makes sense?
> 
> 0. Create a Mutedtype like this: enum { MutedByAudioChannel = 1,
> MutedByStream = 2 }
> 1. Let's be 'muted' uint32_t and use it as a bitmap.
> 2. by default this muted is equal to: AudioChannelService::MutedByDefault()
> 3. When you receive WindowVolumeChanged(volume, muted == true) then:
>  3a. send the notification and set the muted to |= MutedByAudioChannel
> 4. When the stream is muted because no inputs (InputMuted(true/false)) then
> do muted |= MutedByStream
>  4a. and call NotifyStoppedPlaying.
> 
> This should fix the problem, correct?
> It's a bit more work, but it's similar to what we do in HTMLMediaElement.
> 

Hi, Baku,
Change |mMuted| to a Mutedtype is a good idea, but I have some questions.

Do you means that we still call NotifyStoppedPlaying() in InputMuted()?
However, it doesn't solve the problem that the AudioDestinationNode might be not started before we stop it, right?
Or I have forgot something?

Very appreciate :)

Comment 8

[Andrea Marchesini [:baku]]

> Do you means that we still call NotifyStoppedPlaying() in InputMuted()?

Yes. We should call it because the stream is muted.

> However, it doesn't solve the problem that the AudioDestinationNode might be
> not started before we stop it, right?

Well... in this case the AudioDestinationNode will be muted by the AudioChannelService. Correct?
Then you can unmute it if this node is still playing audio.

Comment 9

[Alastor Wu [:alwu]]

(In reply to Andrea Marchesini (:baku) from comment #8)
> Well... in this case the AudioDestinationNode will be muted by the
> AudioChannelService. Correct?
> Then you can unmute it if this node is still playing audio.

Hi, Baku,
The AudioDestinationNode is muted by InputMuted() before the system app open it.
We need to postpone to unregister audio agent. Otherwise, we can't get WindowVolumeChanged(aMute=false).

---
And the bug situation is we can only heard the first two sound, let me explain why,
(1) First click sound
In the beginning, we would set the InputMuted(false) in AudioDestinationNode::CreateAudioChannelAgent.
That ensure the first sound can be heard.

After the first click sound ended, the AudioChannelAgent would be set to nullptr, so the WindowVolumeChanged(aMute=true) can't be called. (the agent is unregistered)

(2) Second click sound
Because we didn't call WindowVolumeChanged(aMute=true) before, the sound can be heard.

Notice that, although the audio should be muted by default in b2g, I found that the InputMuted(false) (called by having audio chunk) seems that be called after the sound really be heard. In this step, we set WindowVolumeChanged(aMute=true).

And then the audio chunk is null, we set the InputMuted(true). Therefore, we unregister the agent and then we can't receive the open command from the system app. We still stay at the muted state.

(3) Third sound sound
Because we are in the muted state, any sound can't be heard.

Comment 10

[Andrea Marchesini [:baku]]

> And the bug situation is we can only heard the first two sound, let me
> explain why,
> (1) First click sound
> In the beginning, we would set the InputMuted(false) in
> AudioDestinationNode::CreateAudioChannelAgent.
> That ensure the first sound can be heard.

By default, in b2g, any AudioDestinationNode is muted. If the system app doesn't unmute the app, I think it's correct that the AudioDestinatioNode stays muted. If this first sound is active, with the MutedType, this should be fixed.

> After the first click sound ended, the AudioChannelAgent would be set to
> nullptr, so the WindowVolumeChanged(aMute=true) can't be called. (the agent
> is unregistered)

Correct. No audio, no audioChannelAgent.

> (2) Second click sound
> Because we didn't call WindowVolumeChanged(aMute=true) before, the sound can
> be heard.

This seems wrong. The second click should be muted as the first one if the system app didn't change the audio state.
This can be fixed with MutedType. Right?

> (3) Third sound sound
> Because we are in the muted state, any sound can't be heard.

Correct. Tell me more where the issue is.

Comment 11

[Alastor Wu [:alwu]]

(In reply to Andrea Marchesini (:baku) from comment #10)
> By default, in b2g, any AudioDestinationNode is muted. If the system app
> doesn't unmute the app, I think it's correct that the AudioDestinatioNode
> stays muted. If this first sound is active, with the MutedType, this should
> be fixed.

We create a new AudioContext when we open the keyboard app, and then call InputMuted(false) in CreateAudioChannelAgent(). Therefore, before the audio actually be playback, we have already started agent and get the open command from the system app. 

Flow like that,
> - Open app
> - Create AudioContext & Destination node
> - Start agent in CreateAudioChannelAgent()
> - Receive unMuted from the system app, any sound of this window can be playback
> - (Idle)
> - Touch keyboard and the sound can be playback

---

> This seems wrong. The second click should be muted as the first one if the
> system app didn't change the audio state.
> This can be fixed with MutedType. Right?

Here is the strange point. The flow is like that,

> - Touch keyboard (second sound)
> - InputMuted(false)
> - Start agent and call WindowVolumeChanged(aMute=true) 
> - (Heard the sound)
> - InputMuted(true), release agent.
> - System app open the sound (agent has been released, this step is no meaning)

Even if the above steps have set the audio muted, the sound still can be heard. This WindowVolumeChanged(aMute=true) would affect next sound(third click), so we can't hear the third click sound. It seems that the sound has already been playback before we call the InputMuted(false).

Comment 12

[Alastor Wu [:alwu]]

The problems we face are,

(1) Stop agent before the system app un-mutes it 
In DestinationNodeEngine, we call InputMuted(false) first, and then call InputMuted(true) very soon. Therefore, we can't open the AudioDestinationNode.

That is why I want to postpone the NotifyStoppedPlaying(), and I don't understand how to solve it with the enum mute type.

(2) InputMuted is called after the sound actually be heard ()
This is only my assumption, and I can't make sure of that. The description is in comment11 (about why the sound click sound can be heard).

Comment 13

[Alastor Wu [:alwu]]

Attachment: MozReview Request: Bug 1183033 - Don't mute the system channel type.

Bug 1183033 - Muted node only after the node is initialized by AudioChannelAPI.

In this patch, we would check whether the AudioDestinationNode is initialized by AudioChannelAPI. We need to make sure not to close the AudioDestinationNode before its initialization.

This patch is depended on the bug1192748, because that bug has the fix for IsAudioChannelMutedByDefault().

Comment 14

[Alastor Wu [:alwu]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Hi, Baku,
Could you help me review this patch?
Very appreciate :)

Comment 16

[Alastor Wu [:alwu]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Bug 1183033 - Muted node only after the node is initialized by AudioChannelAPI.

In this patch, we would check whether the AudioDestinationNode is initialized by AudioChannelAPI. We need to make sure not to close the AudioDestinationNode before its initialization.

This patch is depended on the bug1192748, because that bug has the fix for IsAudioChannelMutedByDefault().

Comment 17

[Alastor Wu [:alwu]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Rebase the patch.

Comment 18

[Alastor Wu [:alwu]]

Comment on attachment 8632706 [details] [diff] [review]
Stop agent after the audio channel is activated by system app

># HG changeset patch
># User Alastor Wu <[e-mail]>
># Date 1436778998 -28800
>#      Mon Jul 13 17:16:38 2015 +0800
># Node ID ca2df149c3e713215b9e0a138fcc2c3175a6711e
># Parent  f84339ec3943478fdd2084a0ea1611540452cf45
>Bug 1183033 - Stop agent after receving play command from system app
>
>diff --git a/dom/media/webaudio/AudioDestinationNode.cpp b/dom/media/webaudio/AudioDestinationNode.cpp
>--- a/dom/media/webaudio/AudioDestinationNode.cpp
>+++ b/dom/media/webaudio/AudioDestinationNode.cpp
>@@ -318,16 +318,17 @@ AudioDestinationNode::AudioDestinationNo
>   : AudioNode(aContext,
>               aIsOffline ? aNumberOfChannels : 2,
>               ChannelCountMode::Explicit,
>               ChannelInterpretation::Speakers)
>   , mFramesToProduce(aLength)
>   , mAudioChannel(AudioChannel::Normal)
>   , mIsOffline(aIsOffline)
>   , mAudioChannelAgentPlaying(false)
>+  , mNeedToStopAudioChannelAgent(false)
>   , mExtraCurrentTime(0)
>   , mExtraCurrentTimeSinceLastStartedBlocking(0)
>   , mExtraCurrentTimeUpdatedSinceLastStableState(false)
> {
>   bool startWithAudioDriver = true;
>   MediaStreamGraph* graph = aIsOffline ?
>                             MediaStreamGraph::CreateNonRealtimeInstance(aSampleRate) :
>                             MediaStreamGraph::GetInstance(startWithAudioDriver, aChannel);
>@@ -486,25 +487,30 @@ AudioDestinationNode::SetCanPlay(float a
> 
>   mStream->SetTrackEnabled(AudioNodeStream::AUDIO_TRACK, !aMuted);
>   mStream->SetAudioOutputVolume(&gWebAudioOutputKey, aVolume);
> }
> 
> NS_IMETHODIMP
> AudioDestinationNode::WindowVolumeChanged(float aVolume, bool aMuted)
> {
>-  if (aMuted != mAudioChannelAgentPlaying) {
>-    mAudioChannelAgentPlaying = aMuted;
>+  bool isPlayable = !aMuted;
>+  if (isPlayable != mAudioChannelAgentPlaying) {
>+    mAudioChannelAgentPlaying = isPlayable;
> 
>     Context()->DispatchTrustedEvent(
>       !aMuted ? NS_LITERAL_STRING("mozinterruptend")
>               : NS_LITERAL_STRING("mozinterruptbegin"));
>   }
> 
>   SetCanPlay(aVolume, aMuted);
>+
>+  if (mAudioChannelAgentPlaying && mNeedToStopAudioChannelAgent) {
>+    mAudioChannelAgent->StopPlaying();
>+  }
>   return NS_OK;
> }
> 
> AudioChannel
> AudioDestinationNode::MozAudioChannelType() const
> {
>   return mAudioChannel;
> }
>@@ -580,20 +586,16 @@ AudioDestinationNode::CreateAudioChannel
>   if (mAudioChannelAgent) {
>     mAudioChannelAgent->StopPlaying();
>   }
> 
>   mAudioChannelAgent = new AudioChannelAgent();
>   mAudioChannelAgent->InitWithWeakCallback(GetOwner(),
>                                            static_cast<int32_t>(mAudioChannel),
>                                            this);
>-
>-  // The AudioChannelAgent must start playing immediately in order to avoid
>-  // race conditions with mozinterruptbegin/end events.
>-  InputMuted(false);
> }
> 
> void
> AudioDestinationNode::NotifyStableState()
> {
>   mExtraCurrentTimeUpdatedSinceLastStableState = false;
> }
> 
>@@ -664,17 +666,20 @@ AudioDestinationNode::InputMuted(bool aM
> {
>   MOZ_ASSERT(Context() && !Context()->IsOffline());
> 
>   if (!mAudioChannelAgent) {
>     return;
>   }
> 
>   if (aMuted) {
>-    mAudioChannelAgent->StopPlaying();
>+    // Sometime we would stop the agent before the system app opens it, because
>+    // this process is async. The agent should be stoped after receving active
>+    // command from the system app.
>+    mNeedToStopAudioChannelAgent = true;
>     return;
>   }
> 
>   float volume = 0.0;
>   bool muted = true;
>   nsresult rv = mAudioChannelAgent->StartPlaying(&volume, &muted);
>   if (NS_WARN_IF(NS_FAILED(rv))) {
>     return;
>diff --git a/dom/media/webaudio/AudioDestinationNode.h b/dom/media/webaudio/AudioDestinationNode.h
>--- a/dom/media/webaudio/AudioDestinationNode.h
>+++ b/dom/media/webaudio/AudioDestinationNode.h
>@@ -101,16 +101,17 @@ private:
>   nsCOMPtr<nsIAudioChannelAgent> mAudioChannelAgent;
> 
>   nsRefPtr<Promise> mOfflineRenderingPromise;
> 
>   // Audio Channel Type.
>   AudioChannel mAudioChannel;
>   bool mIsOffline;
>   bool mAudioChannelAgentPlaying;
>+  bool mNeedToStopAudioChannelAgent;
> 
>   TimeStamp mStartedBlockingDueToBeingOnlyNode;
>   double mExtraCurrentTime;
>   double mExtraCurrentTimeSinceLastStartedBlocking;
>   bool mExtraCurrentTimeUpdatedSinceLastStableState;
> };
> 
> }

Comment 19

[Alastor Wu [:alwu]]

After offline discussion with Baku and Evan, we decided to use the Gaia solution for this issue.

Now the Gaia would close the audio channel after the sound ended, and we think that is no necessary. If the app is still in the foreground, we should not close its audio channel if it have already asked for the audio channel permission before.

---

Evan will help to this issue.

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Any update?

Comment 21

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Should this be obsolete as we decided to fix it in Gaia?

(I don't know the detail and it was not recorded here...)

Comment 22

[Evan Tseng [:evanxd]]

Yes, I'll fix this in Gaia.

Comment 23

[Evan Tseng [:evanxd]]

After thought deeply, if we fix this in Gaia, we might need to do big change and we might have more regression bugs. I think doing bug change is not suitable for a blocker bug.

We can do the refactor in another bug, no in this blocker bug. I think we can fix this bug in gecko first. Then we can refactor this in another bug.

How do you think, Alastor? Let's discuss this in person tomorrow.

Comment 24

[Evan Tseng [:evanxd]]

And we will do the refactor in Bug 1206691.

Comment 25

[Evan Tseng [:evanxd]]

Hi Bobby,

It doesn't mean this bug depends Bug 1206691.

The Comment 23 means we can fix this in gecko first. Then we can refactor Audio Channel Service in next release or other release for performance things.

I don't think we should do big change in a blocker bug.

Comment 26

[Evan Tseng [:evanxd]]

Hi Alastor,

If we implement Media Focus in gaia, we need to do big changes for refactoring the Audio Channel Service[1].

Currently, we assume all audio channel can not be played be default in Audio Channel Service. But Media Focus assumes audio channel in foreground can be play be default. They're totally different assumptions. So it will be a big change if we implement Media Focus. And big change might cause regression bugs.

My suggestion is that we fix this bug in gecko first, and we implement Media Focus to refactor Audio Channel Service in another bug(Bug 1206691).

[1]: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/audio_channel_service.js

Comment 27

[Alastor Wu [:alwu]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Bug 1183033 - Muted node only after the node is initialized by AudioChannelAPI.

In this patch, we would check whether the AudioDestinationNode is initialized by AudioChannelAPI. We need to make sure not to close the AudioDestinationNode before its initialization.

This patch is depended on the bug1192748, because that bug has the fix for IsAudioChannelMutedByDefault().

Comment 28

[Alastor Wu [:alwu]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Bug 1183033 - Muted node only after the node is initialized by AudioChannelAPI.

In this patch, we would check whether the AudioDestinationNode is initialized by AudioChannelAPI. We need to make sure not to close the AudioDestinationNode before its initialization.

Comment 29

[Alastor Wu [:alwu]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Hi, Baku,
According to the comment26, it seems that we need to have a Gecko solution.
Could you help me feedback this patch?
Very appreciate!

---

FYI, in DestinationNodeEngine::ProcessBlock(),
Because the audio click is too short, it causes that the mute/unmute would be send almost the same time. 

> 09-22 16:42:11.688  7611  7987 I Gecko   : DD | newInputMuted = 0 
> 09-22 16:42:11.688  7611  7987 I Gecko   : DD | newInputMuted = 1

Comment 30

[Alastor Wu [:alwu]]

Hi, Baku,
Because of comment26, we can't solve this issue in Gaia now.
Could you help me feedback this patch?
Very appreciate :)

Comment 33

[Alastor Wu [:alwu]]

Hi, Baku,
Could you help me to feedback this bug?
According comment26, we need to solve it in Gecko now, but the final goal is still to solve it in Gaia.
However, since it's a P1 blocker, we need to solve it ASAP.
Very appreciate :)

Comment 34

[Andrea Marchesini [:baku]]

https://reviewboard.mozilla.org/r/16363/#review15969

::: dom/media/webaudio/AudioDestinationNode.cpp:518
(Diff revision 1)
> -    if (UseAudioChannelAPI()) {
> +  if (aMuted && !ComputeMuted()) {

if (aMuted) {
  MOZ_ASSERT(!ComputeMuted());

::: dom/media/webaudio/AudioDestinationNode.cpp:524
(Diff revision 1)
> +  } else if (!aMuted && ComputeMuted()) {

} else {
  MOZ_ASSERT(ComputeMuted());

::: dom/media/webaudio/AudioDestinationNode.cpp:747
(Diff revision 1)
> +  return (mAudioChannelAgent);

return !!mAudioChannelAgent;

::: dom/media/webaudio/AudioDestinationNode.cpp:228
(Diff revision 4)
> +    , mDestinationNode(aNode)

You don't need this. Just use: NodeMainThread().

::: dom/media/webaudio/AudioDestinationNode.cpp:258
(Diff revision 4)
> +      DispatchMutedRunnable(aStream, mPendingMuted);

I don't understand this part:
newInputMuted can be true or false here, but you are ignoring it. Why?

::: dom/media/webaudio/AudioDestinationNode.cpp:299
(Diff revision 4)
> +  AudioDestinationNode* mDestinationNode;

remove this.

::: dom/media/webaudio/AudioDestinationNode.cpp:727
(Diff revision 4)
> -    mAudioChannelAgent->NotifyStoppedPlaying(nsIAudioChannelAgent::AUDIO_AGENT_NOTIFY);
> +    DestroyAudioChannelAgent();

why do you destroy the audioAgent when this is muted?

Comment 35

[Andrea Marchesini [:baku]]

I propose to have a meeting about this issue. What about your Tuesday 8 UTC? It should be 4pm in Taipei.

Comment 36

[Alastor Wu [:alwu]]

Sure, but Evan have meeting in Tuesday 4pm, we need to wait for his response.
I'll send the invitation after checking his free time.

Comment 37

[Alastor Wu [:alwu]]

Hi, Baku,
Here I propose another solution and it's not just a workaround. 
This solution is equal to what we want to do in Gaia, and I think that is a better solution for you.

However, I must to said, this solution CAN'T solve this problem 100%, we still have a little problem.
Even if we use the Gaia soluion, this problem will still exist.
That is we'll still get FAIL on the FIRST short audio.

Let's me explain why..

---

*Before we start* We have known that if we have a short audio, it might be unregistered before the system app opens it, and that is the FIRST error reason why the keyboard can't be playback. The SECOND error reason is that the system app doesn't need to mute window after that window doesn't have any audible sound.

If we use my new solution or use Gaia solution, then we can solve the second error. BUT, we still have to solve the first error.

If we only solve the second error (using this new patch or Gaia solution), we would find that
> The first sound can't be heard, and the following sound can be heard correctly.

---

Why the first sound can't be heard?
In the present codebase, the AudioDesitinationNode would be muted/unmuted by the DestinationNodeEngine [1]. The DestinationNodeEngine would mute/unmute the AudioDesitinationNode , that depends on whether the audio chunk has data. (also other conditions, but it's not the point)

When the first sound starts to play,
(1) Input audio chunk has data.
    => newInputMuted = true, mLastInputMute = false 
    => call InputMuted(false), assign mLastInputMute to true

(2) In InputMuted(false), starts audio channel agent and get the playable status from AudioChannelService
    => On b2g default is muted, so the AudioDesitinationNode can't playback yet.
    => Need to wait for open permission sent from the system app 

(3) Since it's the short sound, audio chunk has already no data within very short period.
    => newInputMuted = false, mLastInputMute = true 
    => call InputMuted(true), assign mLastInputMute to false

(4) In InputMuted(true), unregister audio channel agent and stop the AudioDesitinationNode (Bug here!!)
   
   


The variable |mLastInputMute| stores the last operation for the AudioDesitinationNode, and the default is true. The purpose of this varialbe is to handle the whether trigger InputMuted()

We would check the input data to decide the parame 

If the input has data, and we haven't unmuted the AudioDesitinationNode, we would call InputMuted(false). This method start to register the AudioChannelAgent, and get the mute status from the AudioChannelService.






[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/webaudio/AudioDestinationNode.cpp#238

Comment 38

[Alastor Wu [:alwu]]

Attachment: Don't mute the non-audible window [new solution]

Hi, Baku,
Here I propose another solution and it's not just a workaround. 
This solution is equal to what we want to do in Gaia, and I think that is a better solution for you.

However, I must to said, this solution CAN'T solve this problem 100%, we still have a little problem.
Even if we use the Gaia soluion, this problem will still exist.
That is we'll still get FAIL on the FIRST short audio.

Let's me explain why..

---

We have known that if we have a short audio, it might be unregistered before the system app opens it, and that is the FIRST error reason why the keyboard can't be playback. The SECOND error reason is that the system app doesn't need to mute window after that window doesn't have any audible sound.

If we use my new solution or use Gaia solution, then we can solve the second error. BUT, we still have to solve the first error.

If we only solve the second error (using this new patch or Gaia solution), we would find that
> The first sound can't be heard, and the following sound can be heard correctly.

Therefore, I think we still need to solve the first bug, and that is what I did in the previous patch. 
I know that my previous patch might not be a good solution, but if you agree with that we need to solve both errors as well, we can try to figure out a better solution.

How do you think?

Comment 39

[Alastor Wu [:alwu]]

Why the first sound can't be heard?
In the present codebase, the AudioDesitinationNode would be muted/unmuted by the DestinationNodeEngine [1]. The DestinationNodeEngine would mute/unmute the AudioDesitinationNode , that depends on whether the audio chunk has data. (also other conditions, but it's not the point)

When the first sound starts to play,
(1) Input audio chunk has data.
    => newInputMuted = true, mLastInputMute = false 
    => call InputMuted(false), assign mLastInputMute to true

(2) In InputMuted(false), starts the AudioChannelAgent and gets the playable status from AudioChannelService
    => on b2g default is muted, so the AudioDesitinationNode can't playback.
    => it needs to wait for open permission sent from the system app 

(3) Since it's the short sound, audio chunk has already no data in very short period.
    => newInputMuted = false, mLastInputMute = true 
    => call InputMuted(true), assign mLastInputMute to false

(4) In InputMuted(true), unregister AudioChannelAgent and stop the AudioDesitinationNode  
 
(5) AudioChannelService receives the open permission, and wants to notify the AudioDesitinationNode
    => since the AudioChannelAgent have been unregistered, the AudioDesitinationNode can't receive the notification
    => the sound can't be heard.

[1] https://dxr.mozilla.org/mozilla-central/source/dom/media/webaudio/AudioDestinationNode.cpp#238

Comment 40

[Alastor Wu [:alwu]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

Bug 1183033 - Don't mute the system channel type.

Comment 41

[Andrea Marchesini [:baku]]

Comment on attachment 8649152 [details]
MozReview Request: Bug 1183033 - Don't mute the system channel type.

https://reviewboard.mozilla.org/r/16363/#review20097

Comment 42

[Alastor Wu [:alwu]]

Try-server result.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5d7ae62b56c2

Comment 43

[Alastor Wu [:alwu]]

After applying my patch, now the keyboard sound can be heard normally.
However, here still is a little problem, that is the first sound can't be heard when we start the keyboard app. But, if you press the cancel button in the search bar, and then click the keyboard button again, the first sound can be heard.

I think that is another bug related with AudioDestinationNode, we can file it and solve it later.

Comment 44

[Pulsebot]

https://hg.mozilla.org/integration/b2g-inbound/rev/4084fa04400b

Comment 45

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4084fa04400b

Comment 46

[Martin Shuman [:Marty]]

This issue is verified fixed on the latest Flame and Aries 2.5 Master build.
Keyboard click sounds are audible, and play properly. 

Environmental Variables:
Device: Aries 2.5
BuildID: 20151026111709
Gaia: a677ddd3aa3a81058775938bd56008d96dbc78b0
Gecko: 5ca03a00d26823ce91ee0eaa2937bed605bd53c1
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 44.0a1 (2.5) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0

Environmental Variables:
Device: Flame 2.5
BuildID: 20151026030217
Gaia: a677ddd3aa3a81058775938bd56008d96dbc78b0
Gecko: 5ca03a00d26823ce91ee0eaa2937bed605bd53c1
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 44.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0