New issue
Advanced search Search tips

Issue 878423 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Sep 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

CRAS: debug busy loop for output stream

Project Member Reported by cychiang@chromium.org, Aug 28

Issue description

There was a busy loop detected from here:
https://buganizer.corp.google.com/issues/112860209#comment16
 
The bug is related to the change crrev.com/c/1143097
The change moved output device wake up time calculation to function update_dev_wake_time but forgot to call it for cras_iodev_state(odev) != CRAS_IODEV_STATE_NORMAL_RUN.

So the audio_thread will run into busy loop state when the device goes into no stream state.
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 5

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

commit 98f1499da5364697ec2155c6354f7aa7cc4a7b78
Author: paulhsia <paulhsia@chromium.org>
Date: Wed Sep 05 12:11:54 2018

CRAS: Call update_dev_wake_time for non-normal device state

The bug is related to commit

b84cfd65 CRAS: dev_io - Update underrun detector

The change moved output device wake up time calculation from audio_thread
to function update_dev_wake_time but forgot to call it for several
situations like:

cras_iodev_state(odev) != CRAS_IODEV_STATE_NORMAL_RUN

We move the wake time calculation and underrun operations out of
wrtie_output_samples function and create handle_output_dev_err function
to handle output device error.

Update unit test- audio_thread_unittest to test this situation and move
some tests from write_output_samples functions to dev_io_playback_write.

BUG= chromium:878423 
TEST=Open a playback stream for a while and close it.
     $ cras_test_client --playback_f /dev/zero
     There should not be any busy loop captured by snapshot
     $ cras_test_client --dump_e
TEST=audio_thread_unittest

Change-Id: Ica4f2a4d68a4b5498e64dac53fe724ff582f9d29
Reviewed-on: https://chromium-review.googlesource.com/1203677
Commit-Ready: Chih-Yang Hsia <paulhsia@chromium.org>
Tested-by: Chih-Yang Hsia <paulhsia@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/98f1499da5364697ec2155c6354f7aa7cc4a7b78/cras/src/tests/audio_thread_unittest.cc
[modify] https://crrev.com/98f1499da5364697ec2155c6354f7aa7cc4a7b78/cras/src/server/dev_io.c

Labels: Merge-Request-70 M-70
Need to merge this back to m70 to fix the issue caused by crrev.com/c/1143097.
Thanks!
Labels: -Merge-Request-70 Merge-Approved-70
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 7

Labels: merge-merged-release-R70-11021.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/b50a2f88b01dc5b24262ad5b0c2e9497f8672d89

commit b50a2f88b01dc5b24262ad5b0c2e9497f8672d89
Author: paulhsia <paulhsia@chromium.org>
Date: Fri Sep 07 02:12:17 2018

CRAS: Call update_dev_wake_time for non-normal device state

The bug is related to commit

b84cfd65 CRAS: dev_io - Update underrun detector

The change moved output device wake up time calculation from audio_thread
to function update_dev_wake_time but forgot to call it for several
situations like:

cras_iodev_state(odev) != CRAS_IODEV_STATE_NORMAL_RUN

We move the wake time calculation and underrun operations out of
wrtie_output_samples function and create handle_output_dev_err function
to handle output device error.

Update unit test- audio_thread_unittest to test this situation and move
some tests from write_output_samples functions to dev_io_playback_write.

BUG= chromium:878423 
TEST=Open a playback stream for a while and close it.
     $ cras_test_client --playback_f /dev/zero
     There should not be any busy loop captured by snapshot
     $ cras_test_client --dump_e
TEST=audio_thread_unittest

Change-Id: Ica4f2a4d68a4b5498e64dac53fe724ff582f9d29
Previous-Reviewed-on: https://chromium-review.googlesource.com/1203677
(cherry picked from commit 971c4151c977cc60f7162b259a66d82ca681820c)
Reviewed-on: https://chromium-review.googlesource.com/1212622
Reviewed-by: Chih-Yang Hsia <paulhsia@chromium.org>
Commit-Queue: Chih-Yang Hsia <paulhsia@chromium.org>
Tested-by: Chih-Yang Hsia <paulhsia@chromium.org>

[modify] https://crrev.com/b50a2f88b01dc5b24262ad5b0c2e9497f8672d89/cras/src/tests/audio_thread_unittest.cc
[modify] https://crrev.com/b50a2f88b01dc5b24262ad5b0c2e9497f8672d89/cras/src/server/dev_io.c

Labels: -Merge-Approved-70 Merge-Merged
Wait for the change landing to R70 and test it again.
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 8

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

commit 276b6c5946dee9bf1689f993c21c565d9f63ac13
Author: paulhsia <paulhsia@chromium.org>
Date: Sat Sep 08 16:41:10 2018

CRAS: Skip underrun check for non-normal state device

The bug is related to commit

1212622 CRAS: Call update_dev_wake_time for non-normal device state

For a non-normal state device, the underrun check should be skipped.
Or the underrun operation will fill lots of zeros to device when
starting a new stream.

BUG= chromium:878423 
TEST=Open a playback stream for a while and close it.
     $ cras_test_client --playback_f /dev/zero
     There should not be any underrun event captured by snapshot
     $ cras_test_client --dump_e

Change-Id: I786c30d278f0efc58437b9dc27d573460e4d1581
Previous-Reviewed-on: https://chromium-review.googlesource.com/1212253
(cherry picked from commit 1685dd3b87b07ec3db5d5d480dadb8dccd362afa)
Reviewed-on: https://chromium-review.googlesource.com/1212254
Commit-Queue: Chih-Yang Hsia <paulhsia@chromium.org>
Tested-by: Chih-Yang Hsia <paulhsia@chromium.org>
Trybot-Ready: Chih-Yang Hsia <paulhsia@chromium.org>
Reviewed-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>

[modify] https://crrev.com/276b6c5946dee9bf1689f993c21c565d9f63ac13/cras/src/server/dev_io.c

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 9

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

commit 193762bd149a333f9d6cec8dfe9544772500cbc8
Author: paulhsia <paulhsia@chromium.org>
Date: Sun Sep 09 17:33:48 2018

CRAS: Skip underrun check for non-normal state device

The bug is related to commit

1212622 CRAS: Call update_dev_wake_time for non-normal device state

For a non-normal state device, the underrun check should be skipped.
Or the underrun operation will fill lots of zeros to device when
starting a new stream.

BUG= chromium:878423 
TEST=Open a playback stream for a while and close it.
     $ cras_test_client --playback_f /dev/zero
     There should not be any underrun event captured by snapshot
     $ cras_test_client --dump_e

Change-Id: I786c30d278f0efc58437b9dc27d573460e4d1581
Reviewed-on: https://chromium-review.googlesource.com/1212253
Commit-Ready: Chih-Yang Hsia <paulhsia@chromium.org>
Tested-by: Chih-Yang Hsia <paulhsia@chromium.org>
Reviewed-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>

[modify] https://crrev.com/193762bd149a333f9d6cec8dfe9544772500cbc8/cras/src/server/dev_io.c

Status: Verified (was: Assigned)
Verified on kevin with
R71-11052.0.0.

Sign in to add a comment