New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 669662 link

Starred by 9 users

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Volume change from zero to something creates huge audio noise spikes

Project Member Reported by zelidrag@chromium.org, Nov 29 2016

Issue description

This is a super annoying audio jank that's present on all our devices. I ran into it all the time since my Chromebook is 90% kept muted.

Based on this defect and without looking at CRAS code, I suspect that CRAS increases output volume as a step function. When volume is 0, even relatively small volume increase creates an "infinite edge" resulting in huge harmonics spikes perceivable as crack and pop noise (check recorded video at https://drive.google.com/a/google.com/file/d/0By4efH48AaNeM053MDBicnR3TFE/view?usp=sharing).

To fix this problem, simply make all CRAS volume increase gradual over time instead of step increased. 


What steps will reproduce the problem?

1. Lower the volume on your Chromebook to zero.

2. Play some continuous sound form your Chromebook - i.e. 1000Hz sample from https://www.youtube.com/watch?v=TbPh0pmNjo8.

3. Press volume up key. Press volume down key to go back to zero. Repeat.

What is the expected result?

No audio distortion of the output sound.

What happens instead of that?

Super loud and noticeable audio "spikes" on volume up from zero,



 

Comment 1 by dgreid@chromium.org, Nov 29 2016

On Samus this unmute happens in the DSP.  Have you seen it on other devices too?
There could also be a race where CRAS and the DSP don't agree on when to transition from zeros to actual samples.

We should add a ramp either way, there is an old open bug about that that I'll dig up.  We will add a very quick ramp to try and avoid any UI weirdness like what to do with volume keys during the ramp.
Yeah, this is happening on other devices as well from what I can see, but the edge noise noticeable in slighly different ways there (I suspect due to due to DSP diffs):

1. Samus cracks loudly coming form zero,
2. Lulu gives "rigning" sound when keep volume up/down pressed going from 0 to 100% and vice versa.
3. Kevin "rings" like Lulu but also throws less loud cracks that can be heard sometimes

Comment 3 by vsu...@google.com, Nov 29 2016

Cc: vsu...@chromium.org
Owner: cychiang@chromium.org
Status: Started (was: Assigned)
We will change the unmuting sequence.

1. At mute state

CRAS plays zeros to device.
Headphone/Speaker control is off (if there is such control)
Volume is X, might be anywhere 0~100.

2. Current behavior:

(a). Turn on control (if there is such control)
(b). Turn on software scale of data from 0 to 1.

There is no fixed sequence of (a) or (b).
(a) is done in main thread while (b) is done in audio thread.
If (b) happens before (a) the first non-zero data might be large and cause noise.

3. The proposed change:

(a). Turn on control (if there is such control)
(b). Turn on volume from 0 to X gradually.
(c). Turn on software scale of data from 0 to 1 gradually.

I will first make sure the sequence is fixed, then add ramping to 3(b) and 3(c).

As for samus, it seems to be a different issue because the noise at unmute happens even when playing zeros.

This work-in-progress patch https://chromium-review.googlesource.com/415012 add the ramping from mute to unmute and seems to fix the problem 90% of time.
I can reproduce the noise on samus 1 out of 20. I think that might be samus specific issue to be solved.
I will do more testing and add the ramping from unmute to mute, and also fix the sequence of 3(b).
Currently (b) happens before (a).

Comment 7 by dgreid@chromium.org, Nov 30 2016

Without the ramp b should happen before a.  This allows the codec's soft unmute to help us so we don't need to ramp (it can ramp on its own).


Comment 8 by cychiang@google.com, Nov 30 2016

re#7, Thanks! I got it, that's why you put set_alsa_mute in the end of set_alsa_volume.
After looking at the volume control mechanism again, I think we don't need to do ramping of volume to solve this unmute noise issue because in our use case, volume does not change dramatically.

1. If user presses mute and then presses volume up, it will be handled by ramping of data from 0.0 to 1.0.
2. If user presses volume up/down, the volume will change gradually.
3. If user presses volume down all the way until it becomes muted, and then presses volume up, this would be the same as 1.

So the cras_ramp module that I am going to add only needs to provide basic interface like start_ramp_up(), start_ramp_down(), cancel_ramp(), get_scaler().

This would greatly simplify the design, and can solve the annoying noise.

If we really need to support dramatic volume change (I think current UI does not allow user to do that ?) , and find the need of ramping volume, we can add start_ramp(begin, end, interval, step, callback) that calls set_volume periodically to set volume control gradually.
The path from set_node_volume in cras_iodev will need to be changed. (Currently UI uses node volume instead of system volume).
Currently, set_node_volume changes node->volume right away, and calls set_volume after setting node->volume.
set_volume uses node->volume and system volume to determine the volume to be set on mixer control.
We need to change set_volume so it takes a volume argument that can be used in the callback of ramping.
Apart from that, I am not sure whether we need to call cras_iodev_list_notify_node_volume every update, or just once in the beginning of ramp, or just once in the end of ramp. When taking device that does not have volume control, but only support software volume, it becomes even more complicated. So I would like to avoid adding volume ramping if it is not needed.

Thanks!

Comment 9 by dgreid@chromium.org, Nov 30 2016

The user can "jump" the volume by clicking on the volume slider.  It would be good to ramp there too if it is a big change.

Maybe you can break this in to two problems:
A) On the main thread we should ramp volume changes.
B) On the audio thread, we should smooth the transitions from and to playing zeros.

B is more important, and with the correct mute control sequencing should eliminate any pops on mute/unmute.

I see. Yes that makes sense. I will work on B first.
Thanks!
With these CLs

https://chromium-review.googlesource.com/415750 [WIP] CRAS: cras_ramp - Add a module for ramping        
https://chromium-review.googlesource.com/415751 [WIP] CRAS: alsa_io - Do not turn off switch for volume zero        
https://chromium-review.googlesource.com/415012 [WIP] CRAS: iodev - Ramp the volume for mute transition

The noise from mute to unmute is reduced most of the time.
There are still some problems left:

problem 1. Press volume up/down near volume = 0 so device changes from unmute -> mute (stay in mute less than 1 seconds) -> unmute repeatedly within a short time. There will be noise sometimes.
problem 2. Press mute will have a small but unpleasant noise.


For 1, I suspect this is because the non-zero samples in the buffer get played when output switch is turned on.
We can not fix this by ramping at unmute because it is the data that already in the buffer cause the noise.

I have done two experiments for this:

 A
  a. cras_test_client --playback_f /dev/zero   (to keep device playing)
  b. play youtube 1k sine tone
  c. change volume to 0
  d. quickly after c, mute youtube.  (all data sent to device should be zero)
  e. change volume to 1, still hear noise.

 B
  a. play youtube 1k sine tone.
  b. adjust volume to somewhere > 50.
  b. press mute key (hear noise, as in problem 2)
  c. press volume up key (NO noise)
  d. repeat b and c quickly.

The hypothesis that "non zero samples in the buffer cause the noise when speaker switch is turned on" can not explain B.
I think this might be specific issue on samus codec that can not be explained by how we feed samples

To proceed, I will test the patches on other devices to make sure ramp works fine for usual mute->unmute cases assuming normal user will not switch mute/unmute repeatedly within a short time.
Then, file an issue for intel/realtek to investigate.
Before that is resolved, do not turn on/off speaker switch when mute/unmute on samus.
We can do that if we use mandatory UCM on samus.

Thanks!

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/f84a427264733703c708132ae7c2f615ef384601

commit f84a427264733703c708132ae7c2f615ef384601
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Thu Dec 01 13:01:39 2016

CRAS: alsa_io - Do not turn off switch for volume zero

Setting volume goes through different path as set_mute.
When the adjusted volume changes from 0 to nonzero or vice versa,
and causes output switch to be turned off/on, ramp needs to be triggered
beforhand.
Since it is not easy to see if there is such a change, do not turn off
switch for volume zero.
Output will still be muted by cras_iodev_put_output_buffer.
There will be no ramp of data in volume changes.

BUG= chromium:669662 
TEST=change volume between 0 and 1 on samus, observe there is no pop
noise.

Change-Id: Ib652eafe85ef1f94fbb439dcc6455ea8e044656f
Reviewed-on: https://chromium-review.googlesource.com/415751
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>

[modify] https://crrev.com/f84a427264733703c708132ae7c2f615ef384601/cras/src/tests/alsa_io_unittest.cc
[modify] https://crrev.com/f84a427264733703c708132ae7c2f615ef384601/cras/src/server/cras_alsa_io.c

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/64350200f69f730bbb1eb33a648c8dd6c840da0b

commit 64350200f69f730bbb1eb33a648c8dd6c840da0b
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Tue Dec 13 08:33:53 2016

CRAS: Ramping up/down for new stream and mute/unmute switching

Let alsa_io hold a cras_ramp member to control ramping.
The start/update of cras_ramp is done in audio thread.
The reset of cras_ramp is done in main thread when closing device.
Ramping is used in these places:

1. When the sample from new stream is ready, ramping up.
2. When system mute state is changed from unmute to mute, ramp samples down.
   Set a callback in cras_ramp so the callback can set mute state on device
   after ramping is done. This process let device plays samples that are
   close to zero before it is turned to mute state.
3. When system mute state is changed from mute to unmute, starts ramping
   up samples and switch mute control to unmute state. This process let
   device plays samples that are close to zero after it is turned to
   unmute state.

For example, when user mutes the system, the flow of 2 is:
 a. Mute state in cras_system_state changed.
 b. iodev_list asks audio thread to start ramping down.
 c. audio_thread gets the message and starts ramping down active
    devices.
 d. audio_thread updates ramped sample in cras_iodev_put_output_buffer
    in each output cycle.
 e. Ramping is done. audio_thread executes callback in cras_ramp to use
    cras_device_monitor_set_device_mute_state to ask main thread to set
    mute state on device.
 f. Main thread receives message and calls cras_iodev_set_mute on
    device to actually change device mute state using set_mute ops.

BUG= chromium:669662 
TEST=make check
TEST=on samus, play sine tone in youtube, press mute and volume up
  repeatedly, observe the ramping up and down without pop noise.
TEST=On samus, play a sine tone in youtube, change volume to somewhere
  close to 0 and press "down down down up" quickly. Observe there is pop
  noise at unmute. This is to be solved in the next patch.

Change-Id: I71659edfa0a15d9aafa9db2e8b933c19ef48793f
Reviewed-on: https://chromium-review.googlesource.com/415012
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/tests/iodev_list_unittest.cc
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/tests/alsa_io_unittest.cc
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/server/cras_iodev.h
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/tests/iodev_unittest.cc
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/server/audio_thread.c
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/tests/audio_thread_unittest.cc
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/server/audio_thread.h
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/server/cras_iodev.c
[modify] https://crrev.com/64350200f69f730bbb1eb33a648c8dd6c840da0b/cras/src/server/cras_alsa_io.c

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/793f7323adea956466192ff09ab5774c66ba3461

commit 793f7323adea956466192ff09ab5774c66ba3461
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Thu Dec 01 08:09:01 2016

CRAS: ramp - Add a module for ramping

Add cras_ramp module to handle ramping of data.
1. When device goes from mute to unmute, ramp the data from last
scaler to 1. If there is no last scaler, start from 0.
2. When device goes from unmute to mute, ramp the data from last
scaler to 0. If there is no last scaler, start from 1.

For case 1, device will have smooth transition from mute to unmute.
For case 2, ramp can not finish its full duration. This is because device
samples are muted in cras_iodev_put_output_buffer. Also, device output
switch is turned off right away. These need to be changed in the future
so we can have smooth transition from unmute to mute.

BUG= chromium:669662 
TEST=make check
Change-Id: I25173becd826053c4fa0b0a3bf9ff0d611352d15
Reviewed-on: https://chromium-review.googlesource.com/415750
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>

[add] https://crrev.com/793f7323adea956466192ff09ab5774c66ba3461/cras/src/tests/ramp_unittest.cc
[add] https://crrev.com/793f7323adea956466192ff09ab5774c66ba3461/cras/src/server/cras_ramp.h
[modify] https://crrev.com/793f7323adea956466192ff09ab5774c66ba3461/cras/src/Makefile.am
[add] https://crrev.com/793f7323adea956466192ff09ab5774c66ba3461/cras/src/server/cras_ramp.c

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/b08926e30cc6373b2eb8e11885fd577189e4ba4a

commit b08926e30cc6373b2eb8e11885fd577189e4ba4a
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Mon Dec 05 10:53:33 2016

CRAS: mix - Add ops to scale with increment

Add ops to scale a buffer starting from certain scaler and
increase/descrease for every frame.

BUG= chromium:669662 
TEST=make check

Change-Id: I1332167080aed94de23090fcfeedb75f5738bed5
Reviewed-on: https://chromium-review.googlesource.com/416471
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/b08926e30cc6373b2eb8e11885fd577189e4ba4a/cras/src/tests/mix_unittest.cc
[modify] https://crrev.com/b08926e30cc6373b2eb8e11885fd577189e4ba4a/cras/src/server/cras_mix_ops.h
[modify] https://crrev.com/b08926e30cc6373b2eb8e11885fd577189e4ba4a/cras/src/server/cras_mix.h
[modify] https://crrev.com/b08926e30cc6373b2eb8e11885fd577189e4ba4a/cras/src/server/cras_mix.c
[modify] https://crrev.com/b08926e30cc6373b2eb8e11885fd577189e4ba4a/cras/src/server/cras_mix_ops.c

Project Member

Comment 16 by bugdroid1@chromium.org, Dec 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/961c12cc9ec95b3c65c2a459e702aab37b1eda76

commit 961c12cc9ec95b3c65c2a459e702aab37b1eda76
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Mon Dec 12 09:30:30 2016

CRAS: ramp - Add callback to call when ramping is done

Set callback and callback data at cras_ramp_start. When ramping is done,
call that callback with data. This is to set mute control after ramping
is done.

BUG= chromium:669662 
TEST=make check

Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
Change-Id: I24e7880917acfd982de1c11c030868248d5aa2ce
Reviewed-on: https://chromium-review.googlesource.com/422613
Reviewed-by: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/961c12cc9ec95b3c65c2a459e702aab37b1eda76/cras/src/server/cras_ramp.h
[modify] https://crrev.com/961c12cc9ec95b3c65c2a459e702aab37b1eda76/cras/src/tests/ramp_unittest.cc
[modify] https://crrev.com/961c12cc9ec95b3c65c2a459e702aab37b1eda76/cras/src/server/cras_ramp.c

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/c921f933df5412b85da35d789a63000c44fe48cc

commit c921f933df5412b85da35d789a63000c44fe48cc
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Tue Dec 13 08:33:42 2016

CRAS: iodev - Wrap set_mute with a function

It will be cleaner for users to access set_mute from this interface.

BUG= chromium:669662 
TEST=make check

Change-Id: I41dba8ba8fdba492b71cda3b09b4ca4703953b24
Reviewed-on: https://chromium-review.googlesource.com/422614
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/c921f933df5412b85da35d789a63000c44fe48cc/cras/src/server/cras_iodev.h
[modify] https://crrev.com/c921f933df5412b85da35d789a63000c44fe48cc/cras/src/tests/iodev_unittest.cc
[modify] https://crrev.com/c921f933df5412b85da35d789a63000c44fe48cc/cras/src/server/cras_iodev.c

Project Member

Comment 18 by bugdroid1@chromium.org, Dec 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/cbb23746b63f0d482736cc246bda54b08ff2ab33

commit cbb23746b63f0d482736cc246bda54b08ff2ab33
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Tue Dec 13 08:23:50 2016

CRAS: device_monitor - Add function to set mute state

Device monitor let audio thread request main thread to set mute state on
a device. This is needed when audio thread finds that ramping is done,
and mute/unmute is requested after that using callback in cras_ramp.

BUG= chromium:669662 
TEST=make check

Change-Id: I34baace5898ccd2fbe5884f68fb24738ea0b0ac9
Reviewed-on: https://chromium-review.googlesource.com/422615
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/cbb23746b63f0d482736cc246bda54b08ff2ab33/cras/src/server/cras_device_monitor.c
[modify] https://crrev.com/cbb23746b63f0d482736cc246bda54b08ff2ab33/cras/src/server/cras_device_monitor.h
[modify] https://crrev.com/cbb23746b63f0d482736cc246bda54b08ff2ab33/cras/src/tests/device_monitor_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Dec 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/bba1e29846e375d554bdc2f07e56e8d395000eaf

commit bba1e29846e375d554bdc2f07e56e8d395000eaf
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Fri Dec 23 13:10:38 2016

CRAS: alsa_io - Separate volume and mute ops

Now that we have ramp in alsa_io, we should let ramp finish first and
then set mute control after that.
Putting volume and mute control together causes mute control to be
turned to mute state before ramp is finished.
1. System state has mute = 1 and volume = 0
2. set_alsa_volume is called. It sees mute in system state is 1 so it
   set mute state to muted.
To avoid above problem, we should separate set_volume and set_mute ops
of alsa_io.
This simplifies each ops. Now set_volume only changes
volume, and set_mute only changes mute state.

BUG= chromium:669662 
TEST=make check
TEST=On samus, check ramping down works fine when pressing volume down
  quickly and repeatedly until 0.
TEST=On samus, play a sine tone in youtube. Press volume down until
  volume is the lowest but not zero. Press "down down down up" very
  quickly. There will be no pop noise.

Change-Id: I722367f9577653b0b05f56965ea61038d227cc91
Reviewed-on: https://chromium-review.googlesource.com/423447
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/bba1e29846e375d554bdc2f07e56e8d395000eaf/cras/src/tests/alsa_io_unittest.cc
[modify] https://crrev.com/bba1e29846e375d554bdc2f07e56e8d395000eaf/cras/src/server/cras_alsa_io.c

There are some scenarios not covered yet (discovered in https://buganizer.corp.google.com/issues/34152797 ):

In below cases, we consider system that does not have mute switch so mute/unmute problem will not be masked by mute control.


1. play -> stop playback -> mute -> play (sound) -> stop -> play (sound) -> stop -> play (sound)

This is because we start ramping up for new samples being ready in cras_iodev_output_event_sample_ready, so ramping is in progress.
cras_iodev_put_output_buffer does not mute samples if ramping is in progress. Therefore, samples are not muted.

Fix cras_iodev_output_event_sample_ready so we don't start ramping when output should be muted.

2. play -> stop playback -> mute -> play (sound) -> stop -> play (silence) -> stop -> play (silence)

After the fix in 1, there is still sound at the first play after mute.
This is because "mute" triggers ramping down. This ramping does not move forward because it only consider valid samples.
Device in no_stream state does not have valid samples, so ramping does not move forward.

3. play -> mute (ramp to silence) -> mute (sound) -> mute (sound)

Multiple mute request cause ramping to be started again.
We should fix cras_system_state such that only triggers cras_observer_notify_output_mute when there is mute->unmute or unmute->mute change.
These CL addressed the above issues.

remote:   https://chromium-review.googlesource.com/428730 CRAS: iodev - Do not ramping up when output should mute        
remote:   https://chromium-review.googlesource.com/428731 CRAS: iodev_list - Ramp up/down for unmute/mute only in normal run        
remote:   https://chromium-review.googlesource.com/428732 CRAS: system_state - Only notify observer if needed 
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/f39a21b14e8991554281d25997e44f5914a150bb

commit f39a21b14e8991554281d25997e44f5914a150bb
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Mon Jan 16 11:01:43 2017

CRAS: iodev - Do not ramping up when output should mute

When output should be muted, do not ramping up for sample being ready.
This is for the case where stream is added and removed while system
remains muted.

BUG= chromium:669662 
TEST=make check
TEST=Let system remain muted, play sine tone and stop repeatedly.
Unexpected audio happens only in the first time, and system remains
silence for all other trials. The unexpected audio will be fixed in the
next CL.

Change-Id: I55cd544faac42cf1ac72b9b5445bc7a5c18a3a05
Reviewed-on: https://chromium-review.googlesource.com/428730
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Chinyue Chen <chinyue@chromium.org>

[modify] https://crrev.com/f39a21b14e8991554281d25997e44f5914a150bb/cras/src/tests/iodev_unittest.cc
[modify] https://crrev.com/f39a21b14e8991554281d25997e44f5914a150bb/cras/src/server/cras_iodev.c

Project Member

Comment 23 by bugdroid1@chromium.org, Jan 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/0596a54330b671d02a11ce1b0df01a7488025b80

commit 0596a54330b671d02a11ce1b0df01a7488025b80
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Mon Jan 16 11:28:16 2017

CRAS: system_state - Only notify observer if needed

We should only notify observer when mute/unmute state is changed.
Otherwise, there will be redundant ramping for mute/unmute.

BUG= chromium:669662 
TEST=make check
TEST=Play sine tone. Press mute multiple times. System remains silence.
Change-Id: Ie0c97d4e7f0ef34dce46dc61720c43db12db9933
Reviewed-on: https://chromium-review.googlesource.com/428732
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Chinyue Chen <chinyue@chromium.org>

[modify] https://crrev.com/0596a54330b671d02a11ce1b0df01a7488025b80/cras/src/server/cras_system_state.c
[modify] https://crrev.com/0596a54330b671d02a11ce1b0df01a7488025b80/cras/src/tests/system_state_unittest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Jan 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/86fd177005c8b0f8e53294ea843ee2342431a5ac

commit 86fd177005c8b0f8e53294ea843ee2342431a5ac
Author: Cheng-Yi Chiang <cychiang@chromium.org>
Date: Mon Jan 16 11:05:50 2017

CRAS: iodev_list - Ramp up/down for unmute/mute only in normal run

When device is not in normal run state, there is no need to start
ramping. This avoid the unwanted ramping down where device is
in no_stream state, and user changes mute state.

The scenario is:
1. Start playback
2. Stop playback
3. Device in no_stream state.
4. Press mute. Device starts ramping down.
5. Ramping does not proceed because there is no valid sample being
   played.
6. Start playback. Valid sample is being played. Ramping proceeds.
   cras_iodev_put_output_buffer does not mute samples because ramping is
   in progress, therefore, we have unexpected audio.

This CL avoids the unwanted ramping down request at 5 to avoid
unexpected audio.

BUG= chromium:669662 
TEST=make check
TEST=Follow above scenario and check there is no unexpected audio.

Change-Id: Ia7c20d41f2e1def93ba0e966d8d482b6f703b038
Reviewed-on: https://chromium-review.googlesource.com/428731
Commit-Ready: Cheng-Yi Chiang <cychiang@chromium.org>
Tested-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Chinyue Chen <chinyue@chromium.org>

[modify] https://crrev.com/86fd177005c8b0f8e53294ea843ee2342431a5ac/cras/src/server/cras_iodev_list.c
[modify] https://crrev.com/86fd177005c8b0f8e53294ea843ee2342431a5ac/cras/src/tests/iodev_list_unittest.cc

Comment 25 by vsu...@google.com, Jan 18 2017

Cc: rjahagir@chromium.org ka...@chromium.org
kalin@ & rjahagir@, I believe this issue is the reverse transition of what we were discussing for automation. May be worth looking into as well. 
Cc: cychiang@chromium.org
 Issue 681890  has been merged into this issue.
Status: Fixed (was: Started)
Status: Started (was: Fixed)
2. in #20 is fixed on chell, but not on peppy.
I think there is subtle difference related to software volume.
I am working on it.
Status: Fixed (was: Started)
Oops, that was a false alarm.
One of the fix merged in 9193.0. The other two merged in 9194.0.
I was using 9193.0 on peppy, that's why I got the unexpected sound.
Verified on 9194.0
Status: Verified (was: Fixed)
Verified in Chrome OS 9199.0.0, 57.0.2984.0 on peppy/samus/kevin. 
 Issue 683489  has been merged into this issue.
Cc: mu...@chromium.org hsiangc@chromium.org avkodipelli@chromium.org rohi...@chromium.org chinyue@chromium.org yini...@chromium.org
 Issue 545967  has been merged into this issue.

Sign in to add a comment