Skip to content

Commit

Permalink
CRAS: iodev: Track pinned stream count in iodev
Browse files Browse the repository at this point in the history 
Track the number of pinned streams for each iodev instead of looping
through the stream list to check every time. This patch is needed to
support multiple endpoints where a stream is pinned to an iodev group by
clients and may be attached to one or more devices in the group.

BUG=b:250544745
TEST=1.Tested pinned streams in Meet with speakers/headphones,
DMICs/headset mics on Redrix without multiple endpoints.
2.Added trace at count change/check time and verified the count
change/check sequence while using hotword and Meet on Redrix.
3.Tested simultaneous capture from 2 PCM devices on Redrix with the
rest of the patch series.

Change-Id: Ib149f4dad0437d2ba6e7247f7867d5b027a4daf2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/4808469
Tested-by: chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com <chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com>
Reviewed-by: Curtis Malainey <cujomalainey@chromium.org>
Commit-Queue: Curtis Malainey <cujomalainey@chromium.org>
Auto-Submit: Ben Zhang <benzh@chromium.org>
  • Loading branch information
@bzhg
bzhg authored and Chromeos LUCI committed Nov 16, 2023
1 parent 0945af9 commit e230cfc
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 45 deletions.
18 changes: 18 additions & 0 deletions cras/src/server/cras_iodev.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,10 @@ struct cras_iodev {
enum CRAS_NC_PROVIDER active_nc_provider;
// Indicates that this device ignores capture mute.
bool ignore_capture_mute;
// The total number of pinned streams targeting this device from the main
// thread point of view. Some of these pinned streams may not be attached
// actually due to init/attach errors or suspend.
int num_pinned_streams;
struct cras_iodev *prev, *next;
};

Expand Down Expand Up @@ -1037,6 +1041,20 @@ bool cras_iodev_is_channel_count_supported(struct cras_iodev* iodev,
// Reset the buffer offset for all streams
void cras_iodev_stream_offset_reset_all(struct cras_iodev* iodev);

/* Checks if the given iodev has any pinned stream targeting it from the main
* thread point of view. Some of the pinned streams may not be attached actually
* due to init/attach errors or suspend, but they still count here.
* Args:
* iodev - The device to check.
* Returns:
* True if the given iodev has at least one pinned stream.
* False otherwise.
*/
static inline bool cras_iodev_has_pinned_stream(
const struct cras_iodev* iodev) {
return iodev->num_pinned_streams;
}

/* Gets the current iodev's group. The iodevs in a group are enabled/disabled
* together. The returned read-only array is freed when all iodevs in the group
* are destroyed. Caller should not modify or free the array.
Expand Down
14 changes: 10 additions & 4 deletions cras/src/server/cras_iodev_list.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,8 @@ static int pinned_stream_added(struct cras_rstream* rstream) {
return -EINVAL;
}

dev->num_pinned_streams++;

rc = init_pinned_device(dev, rstream);
if (rc) {
syslog(LOG_DEBUG, "init_pinned_device failed, rc %d", rc);
Expand Down Expand Up @@ -1249,7 +1251,7 @@ static int possibly_close_enabled_devs(enum CRAS_STREAM_DIRECTION dir) {
/* No more default streams, close any device that doesn't have a stream
* pinned to it. */
DL_FOREACH (enabled_devs[dir], edev) {
if (stream_list_has_pinned_stream(stream_list, edev->dev->info.idx)) {
if (cras_iodev_has_pinned_stream(edev->dev)) {
continue;
}
if (dir == CRAS_STREAM_INPUT) {
Expand Down Expand Up @@ -1285,8 +1287,12 @@ static void pinned_stream_removed(struct cras_rstream* rstream) {
if (!dev) {
return;
}

// The stream has already been drained at this point.
dev->num_pinned_streams--;

if (!cras_iodev_list_dev_is_enabled(dev) &&
!stream_list_has_pinned_stream(stream_list, dev->info.idx)) {
!cras_iodev_has_pinned_stream(dev)) {
close_pinned_device(dev);
}
}
Expand Down Expand Up @@ -1413,7 +1419,7 @@ static int disable_device(struct enabled_dev* edev, bool force) {
}
/* If there's a pinned stream exists, simply disconnect all the normal
* streams off this device and return. */
else if (stream_list_has_pinned_stream(stream_list, dev->info.idx)) {
else if (cras_iodev_has_pinned_stream(dev)) {
DL_FOREACH (stream_list_get(stream_list), stream) {
if (stream->direction != dev->direction) {
continue;
Expand Down Expand Up @@ -1618,7 +1624,7 @@ void cras_iodev_list_resume_dev(unsigned int dev_idx) {
/* If dev initialize succeeded and this is not a pinned device,
* disable the silent fallback device because it's just
* unnecessary. */
if (!stream_list_has_pinned_stream(stream_list, dev_idx)) {
if (!cras_iodev_has_pinned_stream(dev)) {
possibly_disable_fallback(dev->direction);
}
} else {
Expand Down
14 changes: 0 additions & 14 deletions cras/src/server/stream_list.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,20 +170,6 @@ int stream_list_rm_all_client_streams(struct stream_list* list,
return rc;
}

bool stream_list_has_pinned_stream(struct stream_list* list,
unsigned int dev_idx) {
struct cras_rstream* rstream;
DL_FOREACH (list->streams, rstream) {
if (!rstream->is_pinned) {
continue;
}
if (rstream->pinned_dev_idx == dev_idx) {
return true;
}
}
return false;
}

int stream_list_get_num_output(struct stream_list* list) {
struct cras_rstream* rstream;
int num_output_stream = 0;
Expand Down
4 changes: 0 additions & 4 deletions cras/src/server/stream_list.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,6 @@ int stream_list_direct_rm(struct stream_list* list, cras_stream_id_t id);
int stream_list_rm_all_client_streams(struct stream_list* list,
struct cras_rclient* rclient);

// Checks if there is a stream pinned to the given device.
bool stream_list_has_pinned_stream(struct stream_list* list,
unsigned int dev_idx);

// Get the number of output streams in the list.
int stream_list_get_num_output(struct stream_list* list);

Expand Down
28 changes: 5 additions & 23 deletions cras/src/tests/iodev_list_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ static std::vector<struct cras_iodev*> set_mute_dev_vector;
static std::vector<unsigned int> audio_thread_dev_start_ramp_dev_vector;
static int audio_thread_dev_start_ramp_called;
static enum CRAS_IODEV_RAMP_REQUEST audio_thread_dev_start_ramp_req;
static std::map<int, bool> stream_list_has_pinned_stream_ret;
static struct cras_rstream* audio_thread_disconnect_stream_stream;
static int audio_thread_disconnect_stream_called;
static struct cras_iodev fake_sco_in_dev, fake_sco_out_dev;
Expand Down Expand Up @@ -150,7 +149,6 @@ class IodevTests : public TestBase {
audio_thread_disconnect_stream_called = 0;
audio_thread_disconnect_stream_stream = NULL;
audio_thread_is_dev_open_ret = 0;
stream_list_has_pinned_stream_ret.clear();
cras_system_get_noise_cancellation_enabled_ret = false;

sample_rates_[0] = 44100;
Expand Down Expand Up @@ -2104,8 +2102,6 @@ TEST_F(IoDevTestSuite, DisableDevWithPinnedStream) {
stream_add_cb(&rstream1);
}

stream_list_has_pinned_stream_ret[d1_.info.idx] = 1;

{ // Selects to d2_ expect no close dev triggered because pinned stream.
CLEAR_AND_EVENTUALLY(EXPECT_EQ, audio_thread_rm_open_dev_called, 0);
CLEAR_AND_EVENTUALLY(EXPECT_EQ, cras_iodev_close_called, 0);
Expand Down Expand Up @@ -2160,14 +2156,14 @@ TEST_F(IoDevTestSuite, AddRemovePinnedStream) {
CLEAR_AND_EVENTUALLY(EXPECT_EQ, update_active_node_called, 1);
EVENTUALLY(EXPECT_EQ, update_active_node_iodev_val[0], &d1_);

EVENTUALLY(EXPECT_EQ, cras_iodev_has_pinned_stream(&d1_), true);
EVENTUALLY(EXPECT_EQ, cras_iodev_has_pinned_stream(&d2_), false);
EVENTUALLY(EXPECT_EQ, rc, 0);

DL_APPEND(stream_list_get_ret, &rstream);
rc = stream_add_cb(&rstream);
}

stream_list_has_pinned_stream_ret[d1_.info.idx] = 1;

{ // Select d2, check pinned stream is not added to d2.
CLEAR_AND_EVENTUALLY(EXPECT_EQ, audio_thread_add_stream_called, 0);
CLEAR_AND_EVENTUALLY(EXPECT_EQ, update_active_node_called, 2);
Expand All @@ -2182,8 +2178,6 @@ TEST_F(IoDevTestSuite, AddRemovePinnedStream) {
cras_make_node_id(d2_.info.idx, 0));
}

stream_list_has_pinned_stream_ret[d1_.info.idx] = 0;

{ // Remove pinned stream from d1, check d1 is closed after stream removed.
CLEAR_AND_EVENTUALLY(EXPECT_EQ, cras_iodev_close_called, 1);
CLEAR_AND_EVENTUALLY(EXPECT_EQ, cras_iodev_close_dev, &d1_);
Expand All @@ -2192,6 +2186,8 @@ TEST_F(IoDevTestSuite, AddRemovePinnedStream) {
CLEAR_AND_EVENTUALLY(EXPECT_EQ, update_active_node_called, 1);
EVENTUALLY(EXPECT_EQ, update_active_node_iodev_val[0], &d1_);

EVENTUALLY(EXPECT_EQ, cras_iodev_has_pinned_stream(&d1_), false);
EVENTUALLY(EXPECT_EQ, cras_iodev_has_pinned_stream(&d2_), false);
EVENTUALLY(EXPECT_EQ, rc, 0);

DL_DELETE(stream_list_get_ret, &rstream);
Expand Down Expand Up @@ -2251,9 +2247,8 @@ TEST_F(IoDevTestSuite, SuspendResumePinnedStream) {
// Device state enters no_stream after stream is disconnected.
d1_.state = CRAS_IODEV_STATE_NO_STREAM_RUN;

// Device has no pinned stream now. But this pinned stream remains in
// Device has no pinned stream attached now. But this pinned stream remains in
// stream_list.
stream_list_has_pinned_stream_ret[d1_.info.idx] = 0;

{ // Suspend and verify that stream is disconnected and d1 is closed.
CLEAR_AND_EVENTUALLY(EXPECT_EQ, audio_thread_disconnect_stream_called, 1);
Expand Down Expand Up @@ -2764,9 +2759,6 @@ TEST_F(IoDevTestSuite, BlockNoiseCancellationByPinnedSpeaker) {
rc = stream_add_cb(&rstream2);
}

stream_list_has_pinned_stream_ret[d1_.info.idx] = 1;
stream_list_has_pinned_stream_ret[d2_.info.idx] = 0;

{ // Remove pinned stream from d2.
CLEAR_AND_EVENTUALLY(EXPECT_EQ, cras_observer_notify_nodes_called, 0);

Expand All @@ -2780,8 +2772,6 @@ TEST_F(IoDevTestSuite, BlockNoiseCancellationByPinnedSpeaker) {
rc = stream_rm_cb(&rstream2);
}

stream_list_has_pinned_stream_ret[d1_.info.idx] = 0;

{ // Remove pinned stream from d1.
CLEAR_AND_EVENTUALLY(EXPECT_EQ, cras_observer_notify_nodes_called, 1);

Expand Down Expand Up @@ -2941,8 +2931,6 @@ TEST_F(IoDevTestSuite, BlockNoiseCancellationInHybridCases) {
rc = stream_add_cb(&rstream);
}

stream_list_has_pinned_stream_ret[d1_.info.idx] = 0;

{ // Remove pinned stream from d1, which means usb device is closed.
CLEAR_AND_EVENTUALLY(EXPECT_EQ, cras_observer_notify_nodes_called, 0);
EVENTUALLY(EXPECT_EQ, cras_iodev_list_dev_is_enabled(&d1_), 1);
Expand Down Expand Up @@ -3083,8 +3071,6 @@ TEST_F(IoDevTestSuite, BlockNoiseCancellationByTwoNodesInOneDev) {
cras_make_node_id(d1_.info.idx, node1_2.idx));
}

stream_list_has_pinned_stream_ret[d1_.info.idx] = 0;

{ // Remove pinned stream from d1.
EVENTUALLY(EXPECT_EQ, rc, 0);

Expand Down Expand Up @@ -3636,10 +3622,6 @@ int cras_iodev_start_volume_ramp(struct cras_iodev* odev,
bool cras_iodev_is_dsp_aec_use_case(const struct cras_ionode* node) {
return (node->type == CRAS_NODE_TYPE_INTERNAL_SPEAKER);
}
bool stream_list_has_pinned_stream(struct stream_list* list,
unsigned int dev_idx) {
return stream_list_has_pinned_stream_ret[dev_idx];
}

struct stream_list* stream_list_create(stream_callback* add_cb,
stream_callback* rm_cb,
Expand Down

0 comments on commit e230cfc

Please sign in to comment.