Closed Bug 984498 Opened 10 years ago Closed 10 years ago

[Dialer][Setting] The busy tone is not played when the device is in silent

Categories

(Firefox OS Graveyard :: AudioChannel, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:1.3+, firefox29 wontfix, firefox30 wontfix, firefox31 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed)

RESOLVED FIXED
2.0 S1 (9may)
blocking-b2g 1.3+
Tracking Status
firefox29 --- wontfix
firefox30 --- wontfix
firefox31 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: isabelrios, Assigned: scheng)

References

Details

(Whiteboard: [cert])

Attachments

(9 files, 9 obsolete files)

1.69 KB, patch
Details | Diff | Splinter Review
3.07 MB, video/3gpp
Details
8.80 MB, video/mpeg
Details
1.60 KB, text/plain
Details
13.14 KB, text/plain
Details
1.64 KB, patch
Details | Diff | Splinter Review
1.60 KB, patch
Details | Diff | Splinter Review
12.08 KB, patch
Details | Diff | Splinter Review
11.97 KB, patch
Details | Diff | Splinter Review
Seen on today's buri 1.3 build
Gecko-ede205e
Gaia-0ab8a9c

STR
1. Press the volume down key till set the device in silent
2. Make a call to a busy number

EXPECTED
The busy tone is heard 

ACTUAL
Nothing is heard, user does not know why the call has been finished unless he is looking at the screen to see the message that the other party is busy.
Summary: [Dialer][Setting] The busy tone is not played when the deivce is in silent → [Dialer][Setting] The busy tone is not played when the device is in silent
FYi, this is also happening in today's buri master build:
Gecko-54b609b
Gaia-a76c008
triage: 1.3+ regression 

Etienne, do you mind looking at this? thanks
blocking-b2g: 1.3? → 1.3+
Flags: needinfo?(etienne)
We play the tone on the "normal" audio channel, so the behavior makes sense.

We can change the audio channel if needed, but I'm not sure to what, playing it on the "telephony" channel with no ongoing call doesn't seem to work.

Maybe Andrea will have an idea :)
Flags: needinfo?(etienne) → needinfo?(amarchesini)
> We can change the audio channel if needed, but I'm not sure to what, playing
> it on the "telephony" channel with no ongoing call doesn't seem to work.

Why? it should work. Do you see some particular error or it just doesn't play audio?
Flags: needinfo?(amarchesini)
Attached patch ugly test patchSplinter Review
(In reply to Andrea Marchesini (:baku) from comment #4)
> > We can change the audio channel if needed, but I'm not sure to what, playing
> > it on the "telephony" channel with no ongoing call doesn't seem to work.
> 
> Why? it should work. Do you see some particular error or it just doesn't
> play audio?

No error, just no audio.

Tested with this test patch which:
* plays the busy tone every time the call button is taped (instead of placing the calls)
* always sets the TonePlayer to the "telephony" channel

and a normal volume to 0.
Hi Andrea, wonder if you have further feedback base on comment 5? thanks
Flags: needinfo?(amarchesini)
There is nothing wrong with the AudioChannelService. The issue here is that WebAudio uses the wrong volume channel. And maybe HTMLMediaElements as well.

We should force a particular volume channel when an audiochannel is used.
Dave Hylands, probably know more.
Flags: needinfo?(amarchesini)
Dave, per Andrea comment 7, could you take a look at this please ?
Flags: needinfo?(dhylands)
Unable to find a regression-window here, we were unable to reproduce the issue following the STRs in Comment 0.  We were able to hear the busy signal with Volume turned down to 0 on latest v1.3, v1.4, v1.5 Buri builds.

v1.3 Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20140320164000
Gaia: 5fb5f5ac12be68e056dddf55b61d7c5b6e6c5525
Gecko: d26180deef65
Version: 28.0
Base Image: V1.2-device.cfg

v1.4 Environmental Variables:
Device: Buri v1.4 MOZ RIL
BuildID: 20140321000202
Gaia: b09bf796d82ca7aa00d141a0cb705ee9cabfab47
Gecko: 81bfa88dada6
Version: 30.0a2
Firmware Version: v1.2-device.cfg

1.5 Environmental Variables:
Device: Buri v1.5 MOZ RIL
BuildID: 20140321120007
Gaia: cc13cdff437c3df45374f5622094a3565a414d5b
Gecko: 199e65efd08b
Version: 31.0a1
Base Image: V1.2-device.cfg
Isabel - Can you retest on the latest 1.3 build? We can't reproduce on our side.
Flags: needinfo?(isabelrios)
Just retested with latest 1.3 (03/24) buri build:
Gecko-7a5dfca
Gaia-f7742fb

I do not do anything different from the steps in comment 0. Just press the volume down key, call to a busy number or call to a number which hangs up the call directly.
Flags: needinfo?(isabelrios)
(In reply to Isabel Rios [:isabel_rios] from comment #11)
> Just retested with latest 1.3 (03/24) buri build:
> Gecko-7a5dfca
> Gaia-f7742fb
> 
> I do not do anything different from the steps in comment 0. Just press the
> volume down key, call to a busy number or call to a number which hangs up
> the call directly.

Just re-tested and still reproducing the issue.
marco, can you give a feedback about comment 7 ?
Flags: needinfo?(mchen)
I'm not currently familiar with the audio stuff, so I'll leave this for marco.
Flags: needinfo?(dhylands)
It occurs to me though, that since different phones are showing different behaviours, that they might be flashed with a different base image.
(In reply to Dave Hylands [:dhylands] from comment #15)
> It occurs to me though, that since different phones are showing different
> behaviours, that they might be flashed with a different base image.

Good point.

Peter - What base image are you running your Buri device on?

Isabel - What base image are you running your Buri device on?
Flags: needinfo?(pbylenga)
Flags: needinfo?(isabelrios)
I'm running: v1.2-device.cfg
Flags: needinfo?(pbylenga)
(In reply to Peter Bylenga from comment #17)
> I'm running: v1.2-device.cfg

What's the date of that base image?
Flags: needinfo?(pbylenga)
The internal build ID when a device is flashed to it is 20131219142759, 12-19-2013.
Flags: needinfo?(pbylenga)
(In reply to Jason Smith [:jsmith] from comment #16)
> (In reply to Dave Hylands [:dhylands] from comment #15)
> > It occurs to me though, that since different phones are showing different
> > behaviours, that they might be flashed with a different base image.
> 
> Good point.
> 
> Peter - What base image are you running your Buri device on?
> 
> Isabel - What base image are you running your Buri device on?

I am also using v1.2-device.cfg base image on buri device
Flags: needinfo?(isabelrios)
According to Comment 15, I suspect that the telephony channel is muted by AudioPolicyManager (in Audio HAL) because the same gecko version has different behavior on two devices. 

So I would suggest to look for our partner's help. To make sure whether audio samples is sent to audio HAL properly or not.
Flags: needinfo?(mchen)
Isabel - What is the date of that base image?
Flags: needinfo?(isabelrios)
I do not have access to that file to know the date. How can I know the date of that base image then? The builds are flashed with that base build directly from buri flashing tool.
My guessing is that the date of the build is the same than the one on comment 19, the name is the same and the date matches when the time we started to use it.
Flags: needinfo?(isabelrios)
(In reply to Isabel Rios [:isabel_rios] from comment #23)
> I do not have access to that file to know the date. How can I know the date
> of that base image then? The builds are flashed with that base build
> directly from buri flashing tool.
> My guessing is that the date of the build is the same than the one on
> comment 19, the name is the same and the date matches when the time we
> started to use it.

I don't want to assume that. Dave's theory might be right here - we could be running a different base image here. I'll go ask Naoki if he knows.

Have you been able to reproduce this on a different device type such as the Open C?
needinfo for comment 24
Flags: needinfo?(isabelrios)
Yes, I have been able to reproduce the issue on Open 2 and Sora devices
Flags: needinfo?(isabelrios)
Hi Eric, can your team look into this further? Thanks
Flags: needinfo?(echou)
(In reply to Joe Cheng [:jcheng] from comment #27)
> Hi Eric, can your team look into this further? Thanks

Sure. Star is working on this.
Flags: needinfo?(echou)
AudioManager::SetAudioChannelVolume() @ AudioManager.cpp
  switch (aChannel) {
    case AUDIO_CHANNEL_CONTENT:
      status = SetStreamVolumeIndex(AUDIO_STREAM_MUSIC, aIndex);
      status = SetStreamVolumeIndex(AUDIO_STREAM_SYSTEM, aIndex);
      break;
    case AUDIO_CHANNEL_NOTIFICATION:
      status = SetStreamVolumeIndex(AUDIO_STREAM_NOTIFICATION, aIndex);
      status = SetStreamVolumeIndex(AUDIO_STREAM_RING, aIndex);
      break;
    case AUDIO_CHANNEL_ALARM:
      status = SetStreamVolumeIndex(AUDIO_STREAM_ALARM, aIndex);
      break;
    case AUDIO_CHANNEL_TELEPHONY:
      status = SetStreamVolumeIndex(AUDIO_STREAM_VOICE_CALL, aIndex);
      break;
    default:
      return NS_ERROR_INVALID_ARG;
  }

We can consider below 3 types among the audio channel in AudioManger:

(1) To press volume key in HomeScreen is to adjust the volume of the type of "AUDIO_CHANNEL_NOTIFICATION".
    You can check weather there is the disable speaker icon in status bar or not. If yes, the "AUDIO_CHANNEL_NOTIFICATION" type 
    is silent. And the volume is sync with setting -> volume -> ringer & notification.

(2) To press volume key in Music app is to adjust the volume of the type of "AUDIO_STREAM_MUSIC".
    There is no voice for busy tone because the "AUDIO_CHANNEL_CONTENT" is equal 0. The busy tone stream type    
    "CUBEB_STREAM_TYPE_SYSTEM" is mapping to "AUDIO_STREAM_SYSTEM" in Android HAL, and we get the "STRATEGY_MEDIA" policy strategy
     according to this type.

(3) To press volume key in call is to adjust the volume of the type of "AUDIO_CHANNEL_TELEPHONY".
    User can NOT set zero volume for the voice in call. The lowest level of voice is "1". 


If the "AUDIO_CHANNEL_CONTENT" volume is 0(for example (2) this case), the "busy tone" should be silent. If in case (1), the "busy tone" should NOT be silent. This is the reason that can't duplicate it in the same gecko SW version.
Assignee: nobody → scheng
Hi,

I can reproduce the issue in both scenarios, 1 and 2. The busy tone is not heard when the device has been set to silent pressing volume key down while being in homescreen or in music app.
Please bear in mind that the busy tone should be always played with its regular tone no matter if the device has the volume set to 0.

Thanks
Component: Gaia::Dialer → AudioChannel
According to my investigation, I want to point out 2 problem:

1.web audio creates audio stream as fixed "AUDIO_CHANNEL_NORMAL" type instead of setting from gaia layer.
 -> The Web Audio API creates audio stream as "AUDIO_CHANNEL_NORMAL" type(http://dxr.mozilla.org/
    mozilla- central/source/content/media/MediaStreamGraph.cpp#814). It doesn't work even we set the 
    "telephony" audio channel type in Gaia layer.

2.there is no correct setting for waiting tone/busy tone.
  -> In dialer app, touch keypad will generate DTMF tone. It is set as "telephony" type. So is not be set muted 
     while in call. https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer
     /js/keypad.js#L234) 


Hi, Paul

I am not familiar with MediaStreamGraph. I need your help. Please provide more information about 1. and how to create audiostream base on the type of audiochannel.
Flags: needinfo?(paul)
When you create the AudioContext, just set its `mozAudioChannelType` to the value you want (here, telephony).
Flags: needinfo?(paul)
We are unfortunately unable to reproduce this issue on the 03/24/14 1.3 and the 03/31/14 1.3 & 1.4 builds.

Me and two other testers used the STR from comment 0, and then tested around the steps to see if we could get the issue another way including turning down each volumes individually and all at once from different screens. I also tried basing my device to the 20131209 and 20131115 images, but still could not get a repro.

- 3/24 1.3 -
Gaia  f7742fb4929cc57c9f72955ce5cebb8279745ac0
Gecko e42b778a010f
BuildID 20140324004001
Version 28.0

- 3/31 1.3 -
Gaia  0d54a796bdaeb96ca7fb63da2166a46b4f8859f2
Gecko ad91817a1967
BuildID 20140331004004
Version 28.0

- 3/31 1.4 -
Gaia  4c3b2f57f4229c5f36f0d8fd399e65f4db88f104
Gecko 3aaca223b673
BuildID 20140331000202
Version 30.0a2


Isabel: though the STR appear simple, might there be a useful piece of information you could provide that has perhaps been overlooked? Maybe a video would be helpful here?

Just so you know, we tried turning down (therefore muting) the ringer volume from the Homescreen and the media (music/video) volume from the Music app. We turned them down one at a time and tested in between, and then tested with both of them turned down.
Flags: needinfo?(isabelrios)
(In reply to Paul Adenot (:padenot) from comment #33)
> When you create the AudioContext, just set its `mozAudioChannelType` to the
> value you want (here, telephony).

Another question is audiocontext is not mapping to audiostream. We can set the "telephony" type to audiocontext, but it is always set as the fixed type - "normal". Whatever you set any type in audiocontext, the type always is "normal" for audio stream. That is different from MediaDecoderStateMachine, it use "audioChannelType" parameter  while initializing audio stream instead of fixed audio channel type. Please see the below:


@ MediaStreamGraph.cpp
void
MediaStreamGraphImpl::CreateOrDestroyAudioStreams(GraphTime aAudioOutputStartTime,
                                                  MediaStream* aStream) {

audioOutputStream->mStream->Init(2, tracks->GetRate(), AUDIO_CHANNEL_NORMAL, AudioStream::LowLatency);

}

========================================================================

@ MediaDecoderStateMachine.cpp
void MediaDecoderStateMachine::AudioLoop() {
 
   audioStream->Init(channels, rate, audioChannelType, AudioStream::HighLatency);
}
Flags: needinfo?(paul)
(In reply to Matthew Vaughan from comment #34)
> We are unfortunately unable to reproduce this issue on the 03/24/14 1.3 and
> the 03/31/14 1.3 & 1.4 builds.
> 
> Me and two other testers used the STR from comment 0, and then tested around
> the steps to see if we could get the issue another way including turning
> down each volumes individually and all at once from different screens. I
> also tried basing my device to the 20131209 and 20131115 images, but still
> could not get a repro.
> 
> - 3/24 1.3 -
> Gaia  f7742fb4929cc57c9f72955ce5cebb8279745ac0
> Gecko e42b778a010f
> BuildID 20140324004001
> Version 28.0
> 
> - 3/31 1.3 -
> Gaia  0d54a796bdaeb96ca7fb63da2166a46b4f8859f2
> Gecko ad91817a1967
> BuildID 20140331004004
> Version 28.0
> 
> - 3/31 1.4 -
> Gaia  4c3b2f57f4229c5f36f0d8fd399e65f4db88f104
> Gecko 3aaca223b673
> BuildID 20140331000202
> Version 30.0a2
> 
> 
> Isabel: though the STR appear simple, might there be a useful piece of
> information you could provide that has perhaps been overlooked? Maybe a
> video would be helpful here?
> 
> Just so you know, we tried turning down (therefore muting) the ringer volume
> from the Homescreen and the media (music/video) volume from the Music app.
> We turned them down one at a time and tested in between, and then tested
> with both of them turned down.


I flashed the latest SW for Nexus4 device. I am able to duplicate it. There are 3 STR as following:

1. In HomeScreen, press volume down key till the ringer volume level is zero. (I can see the disable speaker icon appears in status bar) And launch Music app, turn down the music volume (volume level is zero). 
=> result : I am unable to hear waiting tone or DTMF tone. (the type of waiting tone is the same as busy tone)

2. Base on step 1, Launch Music app and press volume up key to set 3~4 level volume.
   (Because of Bug 990877, I suggest Turn the ringer volume down first, and then turn the music volume up.)
=> result : I can hear waiting tone and DTMF tone.

3. Base on Step 2, turn the music volume down and set 3~4 level volume to ringer volume.
=> result : I am unable to hear waiting tone or DTMF tone.
Ha yeah, we need to plumb the channel type further down.
Flags: needinfo?(paul)
Attached video VID_0004.3gp
Flags: needinfo?(isabelrios)
 
> Isabel: though the STR appear simple, might there be a useful piece of
> information you could provide that has perhaps been overlooked? Maybe a
> video would be helpful here?
> 
> Just so you know, we tried turning down (therefore muting) the ringer volume
> from the Homescreen and the media (music/video) volume from the Music app.
> We turned them down one at a time and tested in between, and then tested
> with both of them turned down.

Just attached a video file so you can see how the issue is reproducible with buri device and 04-01 v1.3 build:
Gecko-7192f32
Gaia-5b0008e
You will see that I activated the speaker while calling so you can hear the regular tone but then nothing when the busy tone should be played. In order to get the busy tone, the called party hangs up the call without taking it.
Hope it helps.
Readding qawanted to see if we can reproduce this in house.
Keywords: qawanted
Thank you for the video Isabel.

We retested on the 04/01/14 1.3 build and the 04/02/14 1.4 build, but we are still unable to reproduce this issue, even after viewing the video.

I have attached a video showing the results that we get here.

- 1.3 build -
Device: Buri v1.3 MOZ RIL
BuildID: 20140401004003
Gaia: 24f562fce468fc948ac9e6185e002c23350cb9ee
Gecko: 0adf24a785f2
Version: 28.0
Firmware Version: v1.2-device.cfg

- 1.4 build -
Device: Buri v1.4 MOZ RIL
BuildID: 20140402000202
Gaia: 1db7a4857faec427799de0827964128b445e80ea
Gecko: b1b159c885cc
Version: 30.0a2
Firmware Version: v1.2-device.cfg
Keywords: qawanted
(In reply to Paul Adenot (:padenot) from comment #37)
> Ha yeah, we need to plumb the channel type further down.

Hi, Paul

I am not familiar with MediaStreamGraph, could you provide any advice to plumb audio channel type down?
Flags: needinfo?(paul)
The goal is to plumb the value from [1] to [2].

For this to work, we need to set the value before the AudioStream gets created in the MediaStreamGraph. I think that setting the channel type in the same event loop run as the AudioContext creation would be enough. You need to propagate the value from the AudioDestinationNode to the AudioDestinatinoNodeEngine, that has an AudioNodeStream (which is a MediaStream), and then, when you create the AudioStream, you simply check if the MediaStream for which we are creating an AudioStream is a AudioNodeStream, and get its channel, pass that to the AudioStream constructor.

This might not work, though, because if the AudioStream is already created when we have the value in the MediaStreamGraph, we won't be able to send it to the system, because we don't have an API yet in AudioStream and libcubeb to change the stream type after creation. Maybe we can change that, granted OpenSL allows changing the type of stream after creation (by getting the PlayerConfig inferface from the player and calling the right method).

[1]: http://dxr.mozilla.org/mozilla-central/source/content/media/webaudio/AudioDestinationNode.cpp#417
[2]: http://dxr.mozilla.org/mozilla-central/source/content/media/MediaStreamGraph.cpp?from=MediaStreamGraph.cpp#814
Flags: needinfo?(paul)
Beatriz,

Is this a cert issue?
Flags: needinfo?(brg)
Whiteboard: [cert]
(In reply to Preeti Raghunath(:Preeti) from comment #44)
> Beatriz,
> 
> Is this a cert issue?
Yes! sorry for arriving late to the triage!
Flags: needinfo?(brg)
this is not a regression actually, it has been how our existing devices work. let's put this to backlog. thanks
blocking-b2g: 1.3+ → backlog
Keywords: regression
(In reply to Joe Cheng [:jcheng] from comment #46)
> this is not a regression actually, it has been how our existing devices
> work. let's put this to backlog. thanks

This is an operator blocker for TEF, so cannot be moved to the backlog. Cert blockers block the release no matter what.

We can't prove or disprove if it's a regression though, since for some reason TEF can reproduce, but we can't.
blocking-b2g: backlog → 1.3+
Note that the underlying Gecko issue is also causing the third problem we've experienced in bug 988760: the busy tone sound is too loud. That's the exact same issue as it's being played on the wrong channel and that particular problem wasn't fixed as part of that bug because fixing this one will take care of both.
See Also: → 988760
Attached patch bug-984498-test.patch (obsolete) — Splinter Review
Hi, Paul

According to comment 43, I try to plumb the audiochannel to MediaStreamGraph. 

On this patch, I can see the stream type value is CUBEB_SYSTEM_TYPE_VOICE_CALL in AudioStream::Init() function while play DTMF tone. It means the audio channel type is plumbed from AudioContext to AudioStream.

I push the test code in the attachd file. Could you give some suggestion?

Thanks
Attachment #8404565 - Flags: feedback?(paul)
Comment on attachment 8404565 [details] [diff] [review]
bug-984498-test.patch

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

We've discussed in #media about this. This is racy, it's better to send the channel type in MediaStream::AddAudioOutput
Attachment #8404565 - Flags: feedback?(paul)
(In reply to Paul Adenot (:padenot) from comment #50)
> Comment on attachment 8404565 [details] [diff] [review]
> bug-984498-test.patch
> 
> Review of attachment 8404565 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> We've discussed in #media about this. This is racy, it's better to send the
> channel type in MediaStream::AddAudioOutput


Here are the conclusions after we discussed in #media channel.

We can *not* to set audiochannel before creating AudioDestinationNode Constructor. And we can *not* set audio channel through openSL API as well. (http://androidxref.com/4.1.1/xref/frameworks/wilhelm/src/android/AudioPlayer_to_android.cpp#496) So we should modify or add Context API in order to set audio channel. 

I am going on investigating it.
Hi, Paul

If we move mStream->Init() [*] to MediaStreamGraphImpl::PlayAudio() [**], it can work and tread-safe? Because we always to set audiochannel type for mediastream before invoking playaudio().


[*] http://dxr.mozilla.org/mozilla-central/source/content/media/MediaStreamGraph.cpp#844
[**] http://dxr.mozilla.org/mozilla-central/source/content/media/MediaStreamGraph.cpp#863
Flags: needinfo?(paul)
So, we don't really want to do that, because AudioStream::Init can be super slow, so we would like to call it as much in advance as we can. See bug 919215 for details.
Flags: needinfo?(paul)
Blocks: 988760
Hi Star -

Really sorry to push you on this, but ZTE's IOT is currently blocking on this one. i am wondering could you let know the date you expect to have patch for this issue?

Thanks

Vance
Flags: needinfo?(scheng)
(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #54)
> Hi Star -
> 
> Really sorry to push you on this, but ZTE's IOT is currently blocking on
> this one. i am wondering could you let know the date you expect to have
> patch for this issue?
> 
> Thanks
> 
> Vance


Hi,

I would like to add new a parameter for AudioChannelType in AudioContext constructor. It means we have to modify web API. I have no idea how long will we can get review +, and I expect to throw the patch first before this weekend.
Flags: needinfo?(scheng)
Attached patch bug-984498-webAPI.patch (obsolete) — Splinter Review
Add a parameter (AudioChannel Type) for AudioContext web API.
Attachment #8407979 - Flags: review?(roc)
Attached patch bug-984498-fix.patch (obsolete) — Splinter Review
1.Add a new member variable in MediaStrem
2.To plumb the audio channel type to MediaStrem.
Attachment #8404565 - Attachment is obsolete: true
Attachment #8408047 - Flags: review?(paul)
Comment on attachment 8407979 [details] [diff] [review]
bug-984498-webAPI.patch

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

::: dom/webidl/AudioContext.webidl
@@ +13,5 @@
>  callback DecodeSuccessCallback = void (AudioBuffer decodedData);
>  callback DecodeErrorCallback = void ();
>  
> +[Constructor,
> + Constructor(AudioChannel audioChannelType)]

Should't we make this optional, and always have it at the end of the parameter list (there are discussions in the WG to be able to request a specific sample rate)? Something like:

Constructor(optional AudioChannel audioChannelType = "normal")
Attachment #8408047 - Flags: review?(paul) → review+
(In reply to Paul Adenot (:padenot) from comment #58)
> Comment on attachment 8407979 [details] [diff] [review]
> bug-984498-webAPI.patch
> 
> Review of attachment 8407979 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/webidl/AudioContext.webidl
> @@ +13,5 @@
> >  callback DecodeSuccessCallback = void (AudioBuffer decodedData);
> >  callback DecodeErrorCallback = void ();
> >  
> > +[Constructor,
> > + Constructor(AudioChannel audioChannelType)]
> 
> Should't we make this optional, and always have it at the end of the
> parameter list (there are discussions in the WG to be able to request a
> specific sample rate)? Something like:
> 
> Constructor(optional AudioChannel audioChannelType = "normal")

Hi, Paul

Thanks for your advice and review. I will modify it. 
But this bug is emergent, I would like to land these patch to solve the issue as soon as possible. And I have opened another bug (Bug 997658) to follow this in order to modify it. What do you think about this?
Flags: needinfo?(paul)
Oh I think that's fine, since you left the constructor without argument. I had not noticed that, sorry.
Flags: needinfo?(paul)
Depends on: 997701
Mark "depends on: 997701"

Because we need gaia team help to use suitable audio channel type to DTMF tone / Busy tone / Waiting tone.
Attached patch bug-984498-fix_v2.patch (obsolete) — Splinter Review
Hi, Paul

I am sorry. I miss to plumb the audio channel type to audiochannel service in AudioDestinationNode constructor. Please help to review this v2 patch.

Thanks
Attachment #8408047 - Attachment is obsolete: true
Attachment #8408793 - Flags: review?(paul)
Attached patch bug-984498-fix_v3.patch (obsolete) — Splinter Review
1.I miss to plumb the audio channel type to audiochannel service in AudioDestinationNode constructor
2.Fix test case failure problem
Attachment #8408793 - Attachment is obsolete: true
Attachment #8408793 - Flags: review?(paul)
Attachment #8408915 - Flags: review?(paul)
Blocks: 981885
Attachment #8408915 - Flags: review?(paul) → review+
Attached file bug-984498-fix_v3-patch-reviewed (obsolete) —
1.TBPL result: https://tbpl.mozilla.org/?tree=Try&rev=049a18f8a59a
2.add reviewer in this patch
Attachment #8408915 - Attachment is obsolete: true
1.add reviewer
Attachment #8407979 - Attachment is obsolete: true
check in needed:

1.bug-984498-webAPI-patch-reviewed - new a paramenter for AudioContext API (reviewer: roc)
2.bug-984498-fix_v3-patch-reviewed - plumb audio channel type to audio backend (reviewer: padenot)
Keywords: checkin-needed
Blocks: 997701
No longer depends on: 997701
No longer blocks: 981885
Attached patch bug-984498-fix_v4.patch (obsolete) — Splinter Review
1.To fix conflict in MediaStreamGraph.h
2.To give a value for mAudioChannelType in MediaStream CTOR in order to fix test case fail.
Attachment #8410712 - Attachment is obsolete: true
Attachment #8412334 - Flags: review?(roc)
Comment on attachment 8412334 [details] [diff] [review]
bug-984498-fix_v4.patch

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

::: content/media/MediaStreamGraph.cpp
@@ +1783,5 @@
>    , mMainThreadCurrentTime(0)
>    , mMainThreadFinished(false)
>    , mMainThreadDestroyed(false)
>    , mGraph(nullptr)
> +  , mAudioChannelType(static_cast<dom::AudioChannel> (dom::AudioChannel::Normal))

This cast doesn't make sense
Attachment #8412334 - Flags: review?(roc) → review+
1.remove static cast
2.add reviewer
3.TPBL result: https://tbpl.mozilla.org/?tree=Try&rev=9c4d25a96c9a
Attachment #8412334 - Attachment is obsolete: true
Keywords: checkin-needed
Hi, sorry had to backout because seems this caused test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=38470990&tree=B2g-Inbound
(In reply to Carsten Book [:Tomcat] from comment #73)
> Hi, sorry had to backout because seems this caused test failures like
> https://tbpl.mozilla.org/php/getParsedLog.php?id=38470990&tree=B2g-Inbound

My bad. I forgot checking in another patch. Sorry about that.
https://hg.mozilla.org/mozilla-central/rev/bbdf06f8088a
https://hg.mozilla.org/mozilla-central/rev/d2d883c84b11
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S1 (9may)
Needs rebasing for 1.4 uplift (and most-likely 1.3 as well), and the 1.3 patches will need approval-mozilla-b2g28 to land as well.
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #77)
> Needs rebasing for 1.4 uplift (and most-likely 1.3 as well), and the 1.3
> patches will need approval-mozilla-b2g28 to land as well.


I am working on fixing building breaking with v1.3 & v1.4 patch. After finishing them, and I will locally test them.
Flags: needinfo?(scheng)
This is under suspicion for causing bug 1002538, so we should not uplift this right now. We need to find out if this patch caused that regression. If it did, then bug 1002538 must be resolved before we uplift this.
(In reply to Jason Smith [:jsmith] from comment #79)
> This is under suspicion for causing bug 1002538, so we should not uplift
> this right now. We need to find out if this patch caused that regression. If
> it did, then bug 1002538 must be resolved before we uplift this.

According to the comment-2[*] in bug 1002538, it can *not* be duplicated in Buri device with master build. I think the usage of audiochannel type should be the same in the identical Gecko SW version. But we can get the different test result in different device. I will build v1.3 & v1.4 with these patches, and test the user story of bug 1002538. If there is any information, I will provide the update.


[*]: (https://bugzilla.mozilla.org/show_bug.cgi?id=1002538#c2)
Attached patch bug-984498-fix_for_v1_3.patch (obsolete) — Splinter Review
This is the patch for v1.3.
This is the webAPI patch for v1.3
Attached patch bug-984498-fix_for_v1_4.patch (obsolete) — Splinter Review
This is the patch for v1.4
This is the webAPI patch for v1.4
Comment on attachment 8414365 [details] [diff] [review]
bug-984498-fix_for_v1_3.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.3 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream 
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8414365 - Flags: approval-mozilla-b2g28?
Comment on attachment 8414366 [details] [diff] [review]
bug-984498-webAPI_for_v1_3.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.3 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8414366 - Flags: approval-mozilla-b2g28?
Comment on attachment 8414367 [details] [diff] [review]
bug-984498-fix_for_v1_4.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.4 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8414367 - Flags: approval-mozilla-b2g30?
Comment on attachment 8414368 [details] [diff] [review]
bug-984498-webAPI_for_v1_4.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.4 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8414368 - Flags: approval-mozilla-b2g30?
(In reply to Star Cheng [:scheng] from comment #80)
> (In reply to Jason Smith [:jsmith] from comment #79)
> > This is under suspicion for causing bug 1002538, so we should not uplift
> > this right now. We need to find out if this patch caused that regression. If
> > it did, then bug 1002538 must be resolved before we uplift this.
> 
> According to the comment-2[*] in bug 1002538, it can *not* be duplicated in
> Buri device with master build. I think the usage of audiochannel type should
> be the same in the identical Gecko SW version. But we can get the different
> test result in different device. I will build v1.3 & v1.4 with these
> patches, and test the user story of bug 1002538. If there is any
> information, I will provide the update.
> 
> 
> [*]: (https://bugzilla.mozilla.org/show_bug.cgi?id=1002538#c2)


After finishing building the v1.3 & v1.4 SW with the patches, I tested the user story of bug 1002538 base on the both SW. But I can't duplicate it.
Comment on attachment 8414367 [details] [diff] [review]
bug-984498-fix_for_v1_4.patch

1.3 blockers still have auto-approval to land on 1.4 for the time-being.
Attachment #8414367 - Flags: approval-mozilla-b2g30?
Attachment #8414368 - Flags: approval-mozilla-b2g30?
Backed out from v1.4 for bustage. Please make sure that the v1.3 patches are updated to include whatever fixes are needed too.
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/bd07ab4a573b

https://tbpl.mozilla.org/php/getParsedLog.php?id=38714247&tree=Mozilla-B2g30-v1.4
Is it just s/audioChannelType/mozAudioChannelType ?
Why was this landed without verifying if this caused the smoketest regression? Please don't do that - it would be terrible to get a smoketest regression on 1.3 right now.
(In reply to Jason Smith [:jsmith] from comment #94)
> Why was this landed without verifying if this caused the smoketest
> regression? Please don't do that - it would be terrible to get a smoketest
> regression on 1.3 right now.

If you have concerns over a patch being uplifted while a possible regression is being investigated, the NO_UPLIFT whiteboard note is the proper way to denote it rather than assuming people are going to find it buried amongst all the other comments in this bug. The uplift queries on the B2G Landing page are specifically designed to ignore such bugs.
Whiteboard: [cert] → [cert][NO_UPLIFT]
(In reply to Star Cheng [:scheng] from comment #89)
> (In reply to Star Cheng [:scheng] from comment #80)
> > (In reply to Jason Smith [:jsmith] from comment #79)
> > > This is under suspicion for causing bug 1002538, so we should not uplift
> > > this right now. We need to find out if this patch caused that regression. If
> > > it did, then bug 1002538 must be resolved before we uplift this.
> > 
> > According to the comment-2[*] in bug 1002538, it can *not* be duplicated in
> > Buri device with master build. I think the usage of audiochannel type should
> > be the same in the identical Gecko SW version. But we can get the different
> > test result in different device. I will build v1.3 & v1.4 with these
> > patches, and test the user story of bug 1002538. If there is any
> > information, I will provide the update.
> > 
> > 
> > [*]: (https://bugzilla.mozilla.org/show_bug.cgi?id=1002538#c2)
> 
> 
> After finishing building the v1.3 & v1.4 SW with the patches, I tested the
> user story of bug 1002538 base on the both SW. But I can't duplicate it.

What device did you test this on? Did you test on something JB-based?
fix build breaking
Attachment #8414365 - Attachment is obsolete: true
Attachment #8414365 - Flags: approval-mozilla-b2g28?
fix build break
Attachment #8414367 - Attachment is obsolete: true
Comment on attachment 8415157 [details] [diff] [review]
bug-984498-fix_v2_for_v1_3.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.3 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream 
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8415157 - Flags: approval-mozilla-b2g28?
Comment on attachment 8415158 [details] [diff] [review]
bug-984498-fix_v2_for_v1_4.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.4 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8415158 - Flags: approval-mozilla-b2g30?
Comment on attachment 8414368 [details] [diff] [review]
bug-984498-webAPI_for_v1_4.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.4 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8414368 - Flags: approval-mozilla-b2g30?
(In reply to Jason Smith [:jsmith] from comment #96)
> (In reply to Star Cheng [:scheng] from comment #89)
> > (In reply to Star Cheng [:scheng] from comment #80)
> > > (In reply to Jason Smith [:jsmith] from comment #79)
> > > > This is under suspicion for causing bug 1002538, so we should not uplift
> > > > this right now. We need to find out if this patch caused that regression. If
> > > > it did, then bug 1002538 must be resolved before we uplift this.
> > > 
> > > According to the comment-2[*] in bug 1002538, it can *not* be duplicated in
> > > Buri device with master build. I think the usage of audiochannel type should
> > > be the same in the identical Gecko SW version. But we can get the different
> > > test result in different device. I will build v1.3 & v1.4 with these
> > > patches, and test the user story of bug 1002538. If there is any
> > > information, I will provide the update.
> > > 
> > > 
> > > [*]: (https://bugzilla.mozilla.org/show_bug.cgi?id=1002538#c2)
> > 
> > 
> > After finishing building the v1.3 & v1.4 SW with the patches, I tested the
> > user story of bug 1002538 base on the both SW. But I can't duplicate it.
> 
> What device did you test this on? Did you test on something JB-based?

I test in nexus4 device. It's JS-based.
Attachment #8414366 - Flags: approval-mozilla-b2g28?
Attachment #8414368 - Flags: approval-mozilla-b2g30?
Attachment #8415157 - Flags: approval-mozilla-b2g28?
Attachment #8415158 - Flags: approval-mozilla-b2g30?

patch for v1.4:

1.TPBL result: https://tbpl.mozilla.org/?tree=Try&rev=6bb0291ec43b
2.Android 4.0 Debug (8) test fail 
 - I don't think it should be not caused by me: uncaught exception - NS_ERROR_FAILURE:  at http://mochi.test:8888/tests/js/xpconnect/tests/mochitest/test_getWebIDLCaller.html:20
 - Bug 962658: solve test_badMimeType.html | Assertion count 6 is greater than expected range 0-0 assertions.


Hi, Ryan

I have no idea about that "/tests/js/xpconnect/tests/mochitest/test_getWebIDLCaller.html:20". But it should be not caused by my patch in try server. Do you think I should approval the v1.4 patch?
Flags: needinfo?(ryanvm)
The TBPL result for v1.3 patch:
https://tbpl.mozilla.org/?tree=Try&rev=1cd74e237c44

I have no idea why there are many test case failure. But I looks seems not caused by my patch.
Keep in mind that Try is configured like trunk/master with respect to what builds/tests it runs. Those builds/tests aren't always going to work correctly (or at all!) when pushing older release branches to Try. As long as the builds/tests that normally run on those branches are green, you should be OK. You can look at TBPL for those branches to get a feel for what builds and tests those are.

https://tbpl.mozilla.org/?tree=Mozilla-B2g30-v1.4
https://tbpl.mozilla.org/?tree=Mozilla-B2g28-v1.3
Flags: needinfo?(ryanvm)
Depends on: 1002538
No longer depends on: 1002538
(In reply to Star Cheng [:scheng]  (away from 5/1 ~ 5/4) from comment #103)
> 
> patch for v1.4:
> 
> 1.TPBL result: https://tbpl.mozilla.org/?tree=Try&rev=6bb0291ec43b
> 2.Android 4.0 Debug (8) test fail 
>  - I don't think it should be not caused by me: uncaught exception -
> NS_ERROR_FAILURE:  at
> http://mochi.test:8888/tests/js/xpconnect/tests/mochitest/
> test_getWebIDLCaller.html:20
>  - Bug 962658: solve test_badMimeType.html | Assertion count 6 is greater
> than expected range 0-0 assertions.
> 
> 
> Hi, Ryan
> 
> I have no idea about that
> "/tests/js/xpconnect/tests/mochitest/test_getWebIDLCaller.html:20". But it
> should be not caused by my patch in try server. Do you think I should
> approval the v1.4 patch?

(1)I have commneted for the test case fail. The failure is resolved in Bug 96265.
Android 4.0 Debug    M(8)

(2)The 2 test cases seems not be caused by my patch. They should be image-related test case.

Android 2.3 Opt    M(3) : should color pixels 
Android 2.3 Opt    M(rc3) : The pop-up has been displayed

(3)The test cases seems not be caused by my patch. They should be input-related test case.
Android 2.3 Opt    M(rc1) : Click can not be completed
Comment on attachment 8414368 [details] [diff] [review]
bug-984498-webAPI_for_v1_4.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.4 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8414368 - Flags: approval-mozilla-b2g30?
Comment on attachment 8415158 [details] [diff] [review]
bug-984498-fix_v2_for_v1_4.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.4 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8415158 - Flags: approval-mozilla-b2g30?
See comment 105. Android 4.0 debug tests on Try are irrelevant if you're pushing a patch based on b2g30 since that platform was never intended to run on that old of a branch. I'm sorry if I didn't make that clear enough in my earlier reply.
(In reply to Star Cheng [:scheng]  (away from 5/1 ~ 5/4) from comment #104)
> The TBPL result for v1.3 patch:
> https://tbpl.mozilla.org/?tree=Try&rev=1cd74e237c44
> 
> I have no idea why there are many test case failure. But I looks seems not
> caused by my patch.


Hi, Ryan

I have checked those test cases, but I don't think the failure is caused by the patches. Because it is for v1.3 branch and critical. Could you mind to uplift the patch for v1.3 (attachment 8415157 [details] [diff] [review] and attachment 8414366 [details] [diff] [review]) locally? If the patch can pass, it should be safe to uplift to v1.3 branch.
Flags: needinfo?(ryanvm)
QA added the NO_UPLIFT to this bug, so I'm not going to do anything here until they remove it. Once it's removed, it'll show up in the usual uplift queries. Needinfo? on me isn't going to help with that.
Flags: needinfo?(ryanvm)
(In reply to Star Cheng [:scheng]  (away from 5/1 ~ 5/4) from comment #110)
> (In reply to Star Cheng [:scheng]  (away from 5/1 ~ 5/4) from comment #104)
> > The TBPL result for v1.3 patch:
> > https://tbpl.mozilla.org/?tree=Try&rev=1cd74e237c44
> > 
> > I have no idea why there are many test case failure. But I looks seems not
> > caused by my patch.
> 
> 
> Hi, Ryan
> 
> I have checked those test cases, but I don't think the failure is caused by
> the patches. Because it is for v1.3 branch and critical. Could you mind to
> uplift the patch for v1.3 (attachment 8415157 [details] [diff] [review] and
> attachment 8414366 [details] [diff] [review]) locally? If the patch can
> pass, it should be safe to uplift to v1.3 branch.

Star - Work with Eric to diagnose the cause of the smoketest regression, since he can reproduce the issue. If we firmly prove that it was caused by the other suspected patch, then we can safely uplift this & remove the NO_UPLIFT flag.
(In reply to Jason Smith [:jsmith] from comment #112)
> (In reply to Star Cheng [:scheng]  (away from 5/1 ~ 5/4) from comment #110)
> > (In reply to Star Cheng [:scheng]  (away from 5/1 ~ 5/4) from comment #104)
> > > The TBPL result for v1.3 patch:
> > > https://tbpl.mozilla.org/?tree=Try&rev=1cd74e237c44
> > > 
> > > I have no idea why there are many test case failure. But I looks seems not
> > > caused by my patch.
> > 
> > 
> > Hi, Ryan
> > 
> > I have checked those test cases, but I don't think the failure is caused by
> > the patches. Because it is for v1.3 branch and critical. Could you mind to
> > uplift the patch for v1.3 (attachment 8415157 [details] [diff] [review] and
> > attachment 8414366 [details] [diff] [review]) locally? If the patch can
> > pass, it should be safe to uplift to v1.3 branch.
> 
> Star - Work with Eric to diagnose the cause of the smoketest regression,
> since he can reproduce the issue. If we firmly prove that it was caused by
> the other suspected patch, then we can safely uplift this & remove the
> NO_UPLIFT flag.


Ryan and Jason, Thanks!
Eric & Star did the analysis to prove this isn't the cause, so this can be uplifted now.
Whiteboard: [cert][NO_UPLIFT] → [cert]
Comment on attachment 8414366 [details] [diff] [review]
bug-984498-webAPI_for_v1_3.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.3 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8414366 - Flags: approval-mozilla-b2g28?
Comment on attachment 8415157 [details] [diff] [review]
bug-984498-fix_v2_for_v1_3.patch

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not a regression
User impact if declined: There is no sound for busy tone / waiting tone while the volume is 0 level by pressing physical volume key in homescreen.
Testing completed: download v1.3 gecko and manual test
Risk to taking this patch (and alternatives if risky): low. Because plumb the audiochannel tyep to audiostream 
String or UUID changes made by this patch: add a new paramenter for AudioContext
Attachment #8415157 - Flags: approval-mozilla-b2g28?

1.TPBL result for v1.3: https://tbpl.mozilla.org/?tree=Try&rev=1cd74e237c44
2.I push a empty (there is not any modification) to try server to check weather the test case is normal or not. The result : https://tbpl.mozilla.org/?tree=Try&rev=f2d9ef6ca189
(In reply to Star Cheng [:scheng] from comment #117)
> 2.I push a empty (there is not any modification) to try server to check
> weather the test case is normal or not. The result :
> https://tbpl.mozilla.org/?tree=Try&rev=f2d9ef6ca189

That's really unnecessary. We already run v1.3 on TBPL (as mentioned many times above).
https://tbpl.mozilla.org/?tree=Mozilla-B2g28-v1.3

Please be more considerate of our test resources. A full run like the ones you ran above consumes over 300 hours of machine time and contributes to job backlogs felt by all other developers. Comment 105 and comment 109 spelled out how to check your Try pushes for older branches against production. If you need more assistance, feel free to contact me via email.
Attachment #8414368 - Flags: approval-mozilla-b2g30?
Attachment #8415158 - Flags: approval-mozilla-b2g30?
Attachment #8414366 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Attachment #8415157 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
See Also: → 981885
You need to log in before you can comment on or make changes to this bug.