CRAS: occasional underrun at switching device |
||||||||||
Issue descriptionVersion: M52 and later OS: Chrome OS What steps will reproduce the problem? (1) play YouTube (2) plug headphone, manual switch between speaker and headphone repeatedly What is the expected output? Audio switches smoothly What do you see instead? Notice audio switches often with short pop/interrupt at the beginning After investigate the audio dump, found the root cause is that since M52 we optimized the 1st start stream delay, by holding device buffer level and start device until stream shm data arrive. This change reduced the start stream delay from 2*cb_threhold to zero. In CRAS we schedule the fetch stream time with a const period. However the shm data reply delay varies. If the 1st shm reply delay is short while the 2nd reply is long, output iodev underruns. In other words, pushing the start stream delay to zero is not safe. Discussed with Jimmy (cychiang@), the ideal solution is to use the no_streams operation patches which landed in M53 to make both the start stream delay and resume stream delay stick to 1 * cb_threshold. Considering M52 is going to stable, and our code has changed a lot since M52 branched, it's difficult to cherry-pick new fix to M52. We have to revert the commits in #25 #26 #27 of issue 519942 .
,
Jun 27 2016
I am sorry for breaking this. I will work on the proper fix on R53. For ALSA device that supports optimized no_stream ops, device can play zeros right after open, and we can control appl_ptr to 1 cb_level ahead of hw_ptr when there is sample ready. This can unify the "resume from no_stream" behavior for both the first stream and the second stream. For device not supporting optimized no_stream ops, we can write 1 cb_level of zeros to it when there is sample ready.
,
Jun 27 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jun 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/5c013cf347f2120498469c5527c5ed080d875db0 commit 5c013cf347f2120498469c5527c5ed080d875db0 Author: Hsin-Yu Chao <hychao@chromium.org> Date: Mon Jun 27 09:32:37 2016 Revert "CRAS: audio_thread - Do not wake for output device not started" This reverts commit 434ca66fe4c39de69e6be54ba8af25127bc8e529. BUG= chromium:623514 TEST=Revert full patch set, playback audio and switches between headphone and speaker. Verify there is no underrun when audio switches. Change-Id: I4765c912770af555cd0b7c2323a725c1d6533850 Reviewed-on: https://chromium-review.googlesource.com/356351 Reviewed-by: Dylan Reid <dgreid@chromium.org> Commit-Queue: Hsinyu Chao <hychao@chromium.org> Tested-by: Hsinyu Chao <hychao@chromium.org> [modify] https://crrev.com/5c013cf347f2120498469c5527c5ed080d875db0/cras/src/server/audio_thread.c
,
Jun 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/99e6a136d6b9e44a009c2238747edbea14a27070 commit 99e6a136d6b9e44a009c2238747edbea14a27070 Author: Hsin-Yu Chao <hychao@chromium.org> Date: Mon Jun 27 09:32:54 2016 Revert "CRAS: audio_thread - Add audio thread log for starting output device" This reverts commit 19363f71216cd60a189e96bb0cae2e82013c3d81. BUG= chromium:623514 TEST=Revert full patch set, playback audio and switches between headphone and speaker. Verify there is no underrun when audio switches. Change-Id: I2998cf7125956db65301e657ce754c9dfd5e8c7a Reviewed-on: https://chromium-review.googlesource.com/356352 Reviewed-by: Dylan Reid <dgreid@chromium.org> Commit-Queue: Hsinyu Chao <hychao@chromium.org> Tested-by: Hsinyu Chao <hychao@chromium.org> [modify] https://crrev.com/99e6a136d6b9e44a009c2238747edbea14a27070/cras/src/tests/cras_test_client.c [modify] https://crrev.com/99e6a136d6b9e44a009c2238747edbea14a27070/cras/src/server/audio_thread.c [modify] https://crrev.com/99e6a136d6b9e44a009c2238747edbea14a27070/cras/src/common/cras_types.h
,
Jun 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/250e8664c946f9bd27dea4500eb4271cbfd6019d commit 250e8664c946f9bd27dea4500eb4271cbfd6019d Author: Hsin-Yu Chao <hychao@chromium.org> Date: Mon Jun 27 09:33:10 2016 Revert "CRAS: iodev - Add start function to cras_iodev" This reverts commit e91895c69bef2515b60d6e4514a5f8e8c53cb36f. BUG= chromium:623514 TEST=Revert full patch set, playback audio and switches between headphone and speaker. Verify there is no underrun when audio switches. Change-Id: Ia2c95dff27315a770fa2eebcd2d48f351037dfc1 Reviewed-on: https://chromium-review.googlesource.com/356353 Reviewed-by: Dylan Reid <dgreid@chromium.org> Commit-Queue: Hsinyu Chao <hychao@chromium.org> Tested-by: Hsinyu Chao <hychao@chromium.org> [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_a2dp_iodev.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_hfp_iodev.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_empty_iodev.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_iodev.h [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/tests/iodev_unittest.cc [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_loopback_iodev.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/audio_thread.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/tests/audio_thread_unittest.cc [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/tests/alsa_io_unittest.cc [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/test_iodev.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_iodev.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_alsa_io.c [modify] https://crrev.com/250e8664c946f9bd27dea4500eb4271cbfd6019d/cras/src/server/cras_bt_io.c
,
Jun 28 2016
This issue is for M52, revert patches are merged. The TOT fix will be tracked issue 519942 .
,
Jul 14 2016
We will need to merge these two changes to R53. https://chromium-review.googlesource.com/#/c/356319/4 https://chromium-review.googlesource.com/#/c/356875/4 I will file the merge request after they are landed on ToT.
,
Jul 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/749e180511e68c1e2d6c321cdfbe860ecc1870dd commit 749e180511e68c1e2d6c321cdfbe860ecc1870dd Author: Cheng-Yi Chiang <cychiang@chromium.org> Date: Wed Jun 29 07:40:35 2016 CRAS: audio_thread - Fill one min_cb_level of zeros before start Currently, we fill samples to device starting at buffer level 0. This is prone to underrun if the sample of next cycle comes late. To avoid this we should fill 1 min_cb_level of zeros. Add the diagram of state transition of an output device. Also, move the dev_running check out of cras_iodev_no_stream_playback so the state transition can be more readable. Do cras_alsa_attemp_resume in dev_running of alsa_io. This is to make sure dev_running does not return 0 while device is later resumed by other cras_alsa_attemp_resume call in cras_alsa_helpers. BUG= chromium:519942 , chromium:623514 TEST=switch headphone/speaker and see there is no underrun. TEST=make check Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> Change-Id: I0319be98a03cc7aa10bfddcd6e679ba3d8d15424 Reviewed-on: https://chromium-review.googlesource.com/356875 Reviewed-by: Dylan Reid <dgreid@chromium.org> [modify] https://crrev.com/749e180511e68c1e2d6c321cdfbe860ecc1870dd/cras/src/tests/alsa_io_unittest.cc [modify] https://crrev.com/749e180511e68c1e2d6c321cdfbe860ecc1870dd/cras/src/tests/iodev_unittest.cc [modify] https://crrev.com/749e180511e68c1e2d6c321cdfbe860ecc1870dd/cras/src/server/audio_thread.c [modify] https://crrev.com/749e180511e68c1e2d6c321cdfbe860ecc1870dd/cras/src/tests/audio_thread_unittest.cc [modify] https://crrev.com/749e180511e68c1e2d6c321cdfbe860ecc1870dd/cras/src/server/cras_iodev.c [modify] https://crrev.com/749e180511e68c1e2d6c321cdfbe860ecc1870dd/cras/src/server/cras_alsa_io.c
,
Jul 18 2016
Add merge request for CL in #9 to M53. Thanks!
,
Jul 18 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/adhd/+/50f369ac906dd65082c31a33bf4cae0023332e4b commit 50f369ac906dd65082c31a33bf4cae0023332e4b Author: Cheng-Yi Chiang <cychiang@chromium.org> Date: Wed Jun 29 07:40:35 2016 CRAS: audio_thread - Fill one min_cb_level of zeros before start Currently, we fill samples to device starting at buffer level 0. This is prone to underrun if the sample of next cycle comes late. To avoid this we should fill 1 min_cb_level of zeros. Add the diagram of state transition of an output device. Also, move the dev_running check out of cras_iodev_no_stream_playback so the state transition can be more readable. Do cras_alsa_attemp_resume in dev_running of alsa_io. This is to make sure dev_running does not return 0 while device is later resumed by other cras_alsa_attemp_resume call in cras_alsa_helpers. BUG= chromium:519942 , chromium:623514 TEST=switch headphone/speaker and see there is no underrun. TEST=make check Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org> Change-Id: I0319be98a03cc7aa10bfddcd6e679ba3d8d15424 Previous-Reviewed-on: https://chromium-review.googlesource.com/356875 (cherry picked from commit 0c68bf021ff724585de08ca692e275bff73327ed) Reviewed-on: https://chromium-review.googlesource.com/361172 [modify] https://crrev.com/50f369ac906dd65082c31a33bf4cae0023332e4b/cras/src/tests/alsa_io_unittest.cc [modify] https://crrev.com/50f369ac906dd65082c31a33bf4cae0023332e4b/cras/src/tests/iodev_unittest.cc [modify] https://crrev.com/50f369ac906dd65082c31a33bf4cae0023332e4b/cras/src/server/audio_thread.c [modify] https://crrev.com/50f369ac906dd65082c31a33bf4cae0023332e4b/cras/src/tests/audio_thread_unittest.cc [modify] https://crrev.com/50f369ac906dd65082c31a33bf4cae0023332e4b/cras/src/server/cras_iodev.c [modify] https://crrev.com/50f369ac906dd65082c31a33bf4cae0023332e4b/cras/src/server/cras_alsa_io.c
,
Jul 21 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 22 2016
,
Aug 2 2016
Verified on M52 build 8350.66.0 Not able to reproduce this issue. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by hychao@chromium.org
, Jun 27 2016