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)
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)
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
|
fabrice
:
approval-mozilla-b2g28+
|
Details | Diff | Splinter Review |
1.60 KB,
patch
|
Details | Diff | Splinter Review | |
12.08 KB,
patch
|
fabrice
:
approval-mozilla-b2g28+
|
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.
Updated•10 years ago
|
Keywords: regression,
regressionwindow-wanted
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
Reporter | ||
Comment 1•10 years ago
|
||
FYi, this is also happening in today's buri master build: Gecko-54b609b Gaia-a76c008
Comment 2•10 years ago
|
||
triage: 1.3+ regression Etienne, do you mind looking at this? thanks
blocking-b2g: 1.3? → 1.3+
Flags: needinfo?(etienne)
Comment 3•10 years ago
|
||
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)
Comment 4•10 years ago
|
||
> 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)
Comment 5•10 years ago
|
||
(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.
Comment 6•10 years ago
|
||
Hi Andrea, wonder if you have further feedback base on comment 5? thanks
Flags: needinfo?(amarchesini)
Comment 7•10 years ago
|
||
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)
Comment 8•10 years ago
|
||
Dave, per Andrea comment 7, could you take a look at this please ?
Flags: needinfo?(dhylands)
Comment 9•10 years ago
|
||
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
Keywords: regressionwindow-wanted
Comment 10•10 years ago
|
||
Isabel - Can you retest on the latest 1.3 build? We can't reproduce on our side.
Flags: needinfo?(isabelrios)
Reporter | ||
Comment 11•10 years ago
|
||
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)
Reporter | ||
Comment 12•10 years ago
|
||
(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.
Comment 14•10 years ago
|
||
I'm not currently familiar with the audio stuff, so I'll leave this for marco.
Flags: needinfo?(dhylands)
Comment 15•10 years ago
|
||
It occurs to me though, that since different phones are showing different behaviours, that they might be flashed with a different base image.
Comment 16•10 years ago
|
||
(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)
Comment 18•10 years ago
|
||
(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)
Comment 19•10 years ago
|
||
The internal build ID when a device is flashed to it is 20131219142759, 12-19-2013.
Flags: needinfo?(pbylenga)
Reporter | ||
Comment 20•10 years ago
|
||
(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)
Comment 21•10 years ago
|
||
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)
Reporter | ||
Comment 23•10 years ago
|
||
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)
Comment 24•10 years ago
|
||
(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?
Reporter | ||
Comment 26•10 years ago
|
||
Yes, I have been able to reproduce the issue on Open 2 and Sora devices
Flags: needinfo?(isabelrios)
Comment 27•10 years ago
|
||
Hi Eric, can your team look into this further? Thanks
Flags: needinfo?(echou)
Comment 29•10 years ago
|
||
(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)
Assignee | ||
Comment 30•10 years ago
|
||
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
Reporter | ||
Comment 31•10 years ago
|
||
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
Updated•10 years ago
|
Component: Gaia::Dialer → AudioChannel
Updated•10 years ago
|
Keywords: regressionwindow-wanted
Assignee | ||
Comment 32•10 years ago
|
||
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)
Comment 33•10 years ago
|
||
When you create the AudioContext, just set its `mozAudioChannelType` to the value you want (here, telephony).
Flags: needinfo?(paul)
Comment 34•10 years ago
|
||
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)
Updated•10 years ago
|
Keywords: regressionwindow-wanted
Assignee | ||
Comment 35•10 years ago
|
||
(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)
Assignee | ||
Comment 36•10 years ago
|
||
(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.
Comment 37•10 years ago
|
||
Ha yeah, we need to plumb the channel type further down.
Flags: needinfo?(paul)
Reporter | ||
Comment 38•10 years ago
|
||
Flags: needinfo?(isabelrios)
Reporter | ||
Comment 39•10 years ago
|
||
> 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.
Comment 40•10 years ago
|
||
Readding qawanted to see if we can reproduce this in house.
Keywords: qawanted
Comment 41•10 years ago
|
||
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
Assignee | ||
Comment 42•10 years ago
|
||
(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)
Comment 43•10 years ago
|
||
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)
Updated•10 years ago
|
Whiteboard: [cert]
Comment 45•10 years ago
|
||
(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)
Comment 46•10 years ago
|
||
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
Comment 47•10 years ago
|
||
(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+
Comment 48•10 years ago
|
||
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
Assignee | ||
Comment 49•10 years ago
|
||
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 50•10 years ago
|
||
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)
Assignee | ||
Comment 51•10 years ago
|
||
(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.
Assignee | ||
Comment 52•10 years ago
|
||
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)
Comment 53•10 years ago
|
||
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)
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)
Assignee | ||
Comment 55•10 years ago
|
||
(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)
Assignee | ||
Comment 56•10 years ago
|
||
Add a parameter (AudioChannel Type) for AudioContext web API.
Attachment #8407979 -
Flags: review?(roc)
Attachment #8407979 -
Flags: review?(roc) → review+
Assignee | ||
Comment 57•10 years ago
|
||
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 58•10 years ago
|
||
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")
Updated•10 years ago
|
Attachment #8408047 -
Flags: review?(paul) → review+
Assignee | ||
Comment 59•10 years ago
|
||
(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)
Comment 60•10 years ago
|
||
Oh I think that's fine, since you left the constructor without argument. I had not noticed that, sorry.
Flags: needinfo?(paul)
Assignee | ||
Comment 61•10 years ago
|
||
Mark "depends on: 997701" Because we need gaia team help to use suitable audio channel type to DTMF tone / Busy tone / Waiting tone.
Assignee | ||
Comment 62•10 years ago
|
||
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)
Assignee | ||
Comment 63•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8408915 -
Flags: review?(paul) → review+
Assignee | ||
Comment 64•10 years ago
|
||
1.TBPL result: https://tbpl.mozilla.org/?tree=Try&rev=049a18f8a59a 2.add reviewer in this patch
Attachment #8408915 -
Attachment is obsolete: true
Assignee | ||
Comment 65•10 years ago
|
||
1.add reviewer
Attachment #8407979 -
Attachment is obsolete: true
Assignee | ||
Comment 66•10 years ago
|
||
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
Comment 67•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/cbd4d782c02f https://hg.mozilla.org/integration/b2g-inbound/rev/1f410dde84d9
Keywords: checkin-needed
Updated•10 years ago
|
Comment 68•10 years ago
|
||
sorry had to backout since this might have caused https://tbpl.mozilla.org/php/getParsedLog.php?id=38298280&tree=B2g-Inbound
Assignee | ||
Comment 69•10 years ago
|
||
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+
Assignee | ||
Comment 71•10 years ago
|
||
1.remove static cast 2.add reviewer 3.TPBL result: https://tbpl.mozilla.org/?tree=Try&rev=9c4d25a96c9a
Attachment #8412334 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 72•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/c07bced21c89
Keywords: checkin-needed
Comment 73•10 years ago
|
||
Hi, sorry had to backout because seems this caused test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=38470990&tree=B2g-Inbound
Comment 74•10 years ago
|
||
(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.
Comment 75•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/bbdf06f8088a https://hg.mozilla.org/integration/b2g-inbound/rev/d2d883c84b11
Comment 76•10 years ago
|
||
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)
Comment 77•10 years ago
|
||
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.
status-b2g-v1.3:
--- → affected
status-b2g-v1.4:
--- → affected
status-b2g-v2.0:
--- → fixed
status-firefox29:
--- → wontfix
status-firefox30:
--- → affected
status-firefox31:
--- → fixed
Flags: needinfo?(scheng)
Keywords: branch-patch-needed
Assignee | ||
Comment 78•10 years ago
|
||
(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)
Comment 79•10 years ago
|
||
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.
Assignee | ||
Comment 80•10 years ago
|
||
(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)
Assignee | ||
Comment 81•10 years ago
|
||
This is the patch for v1.3.
Assignee | ||
Comment 82•10 years ago
|
||
This is the webAPI patch for v1.3
Assignee | ||
Comment 83•10 years ago
|
||
This is the patch for v1.4
Assignee | ||
Comment 84•10 years ago
|
||
This is the webAPI patch for v1.4
Assignee | ||
Comment 85•10 years ago
|
||
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?
Assignee | ||
Comment 86•10 years ago
|
||
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?
Assignee | ||
Comment 87•10 years ago
|
||
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?
Assignee | ||
Comment 88•10 years ago
|
||
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?
Assignee | ||
Comment 89•10 years ago
|
||
(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 90•10 years ago
|
||
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?
Updated•10 years ago
|
Attachment #8414368 -
Flags: approval-mozilla-b2g30?
Comment 91•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/b9e84eb3785f https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/2d63daf77093
Comment 92•10 years ago
|
||
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
Comment 93•10 years ago
|
||
Is it just s/audioChannelType/mozAudioChannelType ?
Comment 94•10 years ago
|
||
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.
Comment 95•10 years ago
|
||
(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.
Updated•10 years ago
|
Whiteboard: [cert] → [cert][NO_UPLIFT]
Comment 96•10 years ago
|
||
(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?
Assignee | ||
Comment 97•10 years ago
|
||
fix build breaking
Attachment #8414365 -
Attachment is obsolete: true
Attachment #8414365 -
Flags: approval-mozilla-b2g28?
Assignee | ||
Comment 98•10 years ago
|
||
fix build break
Attachment #8414367 -
Attachment is obsolete: true
Assignee | ||
Comment 99•10 years ago
|
||
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?
Assignee | ||
Comment 100•10 years ago
|
||
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?
Assignee | ||
Comment 101•10 years ago
|
||
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?
Assignee | ||
Comment 102•10 years ago
|
||
(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.
Assignee | ||
Updated•10 years ago
|
Attachment #8414366 -
Flags: approval-mozilla-b2g28?
Assignee | ||
Updated•10 years ago
|
Attachment #8414368 -
Flags: approval-mozilla-b2g30?
Assignee | ||
Updated•10 years ago
|
Attachment #8415157 -
Flags: approval-mozilla-b2g28?
Assignee | ||
Updated•10 years ago
|
Attachment #8415158 -
Flags: approval-mozilla-b2g30?
Assignee | ||
Comment 103•10 years ago
|
||
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)
Assignee | ||
Comment 104•10 years ago
|
||
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.
Comment 105•10 years ago
|
||
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)
Assignee | ||
Comment 106•10 years ago
|
||
(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
Assignee | ||
Comment 107•10 years ago
|
||
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?
Assignee | ||
Comment 108•10 years ago
|
||
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 109•10 years ago
|
||
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.
Assignee | ||
Comment 110•10 years ago
|
||
(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)
Comment 111•10 years ago
|
||
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)
Comment 112•10 years ago
|
||
(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.
Assignee | ||
Comment 113•10 years ago
|
||
(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!
Comment 114•10 years ago
|
||
Eric & Star did the analysis to prove this isn't the cause, so this can be uplifted now.
Whiteboard: [cert][NO_UPLIFT] → [cert]
Assignee | ||
Comment 115•10 years ago
|
||
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?
Assignee | ||
Comment 116•10 years ago
|
||
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?
Assignee | ||
Comment 117•10 years ago
|
||
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
Comment 118•10 years ago
|
||
(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.
Updated•10 years ago
|
Attachment #8414368 -
Flags: approval-mozilla-b2g30?
Comment 119•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/edf33b1d53d1 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/700192ca4f0e
Updated•10 years ago
|
Attachment #8415158 -
Flags: approval-mozilla-b2g30?
Updated•10 years ago
|
Attachment #8414366 -
Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Updated•10 years ago
|
Attachment #8415157 -
Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Comment 120•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/9a8f8ff5a74b https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/fd3e5f7b41bd
Updated•10 years ago
|
status-b2g-v1.3T:
--- → fixed
Comment 121•10 years ago
|
||
It will cause bug 984498 and sprd bug. http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=311363
You need to log in
before you can comment on or make changes to this bug.
Description
•