Skip to content

Commit

Permalink
Reland cras_alsa_io: avoid sample manipulations on mmap memory
Browse files Browse the repository at this point in the history 
This is a reland of commit 1e84e2f with
memory copy fix in put_buffer of cras_also_io.c.

The access speed of mmap memory can be very slow compared to accessing
the local memory.

To avoid the slow access of mmap memory for alsa devices:
1. Malloc the local memory in configure_dev().
2. Performs the sample manipulations on the local memory.
3. Copies the result from the local memory to the mmap memory before
   calling cras_alsa_mmap_commit().
4. Free the local memory in close_dev().

This CL also fixes the capture pop noise reported in b/296971597
by preserving the processed samples in the local memory instead of
writing them into the non-commit memory and then reading them back
at the next mmap_begin.

BUG=b:299402922
BUG=b:296971597
TEST=unit tests
TEST=tast run ${DUT} arc.AudioOboetesterGlitch.*
TEST=tast run ${DUT} arc.AudioLoopbackCorrectness.*
TEST=tast run ${DUT} audio.CrasStreamMix.*
TEST=atest android.media.cts.AudioRecordTest#testAudioRecordAuditByteBufferResamplerStereoFloat

Change-Id: I2cdf6cda54bacaace8635eb25b7f7031993c6894
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/4957838
Reviewed-by: Li-Yu Yu <aaronyu@google.com>
Tested-by: Judy Hsiao <judyhsiao@google.com>
Tested-by: chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com <chromeos-cop-builder@chromeos-cop.iam.gserviceaccount.com>
Reviewed-by: Yu-Hsuan Hsu <yuhsuan@chromium.org>
Auto-Submit: Judy Hsiao <judyhsiao@google.com>
Commit-Queue: Judy Hsiao <judyhsiao@google.com>
  • Loading branch information
@baili0411
baili0411 authored and Chromeos LUCI committed Nov 6, 2023
1 parent b469f70 commit 09c9e51
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 20 deletions.
2 changes: 2 additions & 0 deletions cras/src/server/cras_alsa_common_io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ int cras_alsa_common_close_dev(const struct cras_iodev* iodev) {
aio->hwparams_set = 0;
cras_iodev_free_format(&aio->base);
cras_iodev_free_audio_area(&aio->base);
free(aio->sample_buf);
aio->sample_buf = NULL;
return 0;
}
int cras_alsa_common_open_dev(struct cras_iodev* iodev, const char* pcm_name) {
Expand Down
6 changes: 6 additions & 0 deletions cras/src/server/cras_alsa_common_io.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ struct alsa_common_io {
size_t product_id;
// Last obtained hardware timestamp.
struct timespec hardware_timestamp;
// Pointer to mmap buffer. It's mmap-ed in get_buffer() and
// committed in put_buffer().
uint8_t* mmap_buf;
// Pointer to sample buffer. It's malloc in configure_dev() and
// free in close_dev().
uint8_t* sample_buf;
};

struct cras_ionode* first_plugged_node(struct cras_iodev* iodev);
Expand Down
64 changes: 55 additions & 9 deletions cras/src/server/cras_alsa_io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include "cras/base/check.h"
#include "cras/src/common/cras_alsa_card_info.h"
#include "cras/src/common/cras_string.h"
#include "cras/src/server/audio_thread.h"
#include "cras/src/server/config/cras_card_config.h"
#include "cras/src/server/cras_alsa_common_io.h"
Expand Down Expand Up @@ -306,6 +307,7 @@ static int configure_dev(struct cras_iodev* iodev) {
aio->common.severe_underrun_frames =
SEVERE_UNDERRUN_MS * iodev->format->frame_rate / 1000;

size_t fmt_bytes = cras_get_format_bytes(iodev->format);
cras_iodev_init_audio_area(iodev);

syslog(LOG_DEBUG, "Configure alsa device %s rate %zuHz, %zu channels",
Expand All @@ -317,6 +319,18 @@ static int configure_dev(struct cras_iodev* iodev) {
return rc;
}

if (!aio->common.sample_buf) {
aio->common.sample_buf =
(uint8_t*)calloc(iodev->buffer_size * fmt_bytes, sizeof(uint8_t));
if (!aio->common.sample_buf) {
syslog(LOG_ERR, "cras_alsa_io: configure_dev: calloc: %s",
cras_strerror(errno));
return -ENOMEM;
}
cras_audio_area_config_buf_pointers(iodev->area, iodev->format,
aio->common.sample_buf);
}

// Set channel map to device
rc = cras_alsa_set_channel_map(aio->common.handle, iodev->format);
if (rc < 0) {
Expand Down Expand Up @@ -429,19 +443,30 @@ static int get_buffer(struct cras_iodev* iodev,
struct cras_audio_area** area,
unsigned* frames) {
struct alsa_io* aio = (struct alsa_io*)iodev;
snd_pcm_uframes_t nframes = *frames;
uint8_t* dst = NULL;
size_t format_bytes;
int rc;
snd_pcm_uframes_t nframes = MIN(iodev->buffer_size, *frames);

aio->common.mmap_offset = 0;
format_bytes = cras_get_format_bytes(iodev->format);

rc = cras_alsa_mmap_begin(aio->common.handle, format_bytes, &dst,
&aio->common.mmap_offset, &nframes);
size_t format_bytes = cras_get_format_bytes(iodev->format);

int rc = cras_alsa_mmap_begin(aio->common.handle, format_bytes,
&aio->common.mmap_buf, &aio->common.mmap_offset,
&nframes);
if (rc < 0) {
return rc;
}
iodev->area->frames = nframes;
cras_audio_area_config_buf_pointers(iodev->area, iodev->format, dst);
// Copy mmap_buf data to local memory for faster manipulation.
// Check `cras_bench --benchmark_filter=BM_Alsa/MmapBuffer` for analysis.
if (iodev->direction == CRAS_STREAM_INPUT) {
if (nframes > iodev->input_dsp_offset) {
memcpy(aio->common.sample_buf + iodev->input_dsp_offset * format_bytes,
aio->common.mmap_buf + iodev->input_dsp_offset * format_bytes,
(nframes - iodev->input_dsp_offset) * format_bytes);
} else {
syslog(LOG_WARNING, "nframe is less than input_dsp_offset: %lu > %u",
nframes, iodev->input_dsp_offset);
}
}

*area = iodev->area;
*frames = nframes;
Expand All @@ -451,6 +476,27 @@ static int get_buffer(struct cras_iodev* iodev,

static int put_buffer(struct cras_iodev* iodev, unsigned nwritten) {
struct alsa_io* aio = (struct alsa_io*)iodev;

size_t format_bytes = cras_get_format_bytes(iodev->format);
if (iodev->direction == CRAS_STREAM_OUTPUT) {
memcpy(aio->common.mmap_buf, aio->common.sample_buf,
nwritten * format_bytes);
unsigned int max_offset = cras_iodev_max_stream_offset(iodev);
if (max_offset) {
memmove(aio->common.sample_buf,
aio->common.sample_buf + nwritten * format_bytes,
max_offset * format_bytes);
}
} else {
// CRAS applied input DSP on the uncommitted data, move then to the
// beginning.
if (iodev->input_dsp_offset) {
memmove(aio->common.sample_buf,
aio->common.sample_buf + nwritten * format_bytes,
iodev->input_dsp_offset * format_bytes);
}
}

return cras_alsa_mmap_commit(aio->common.handle, aio->common.mmap_offset,
nwritten);
}
Expand Down
62 changes: 53 additions & 9 deletions cras/src/server/cras_alsa_usb_io.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "cras/src/common/cras_alsa_card_info.h"
#include "cras/src/common/cras_log.h"
#include "cras/src/common/cras_metrics.h"
#include "cras/src/common/cras_string.h"
#include "cras/src/server/audio_thread.h"
#include "cras/src/server/config/cras_card_config.h"
#include "cras/src/server/cras_alsa_common_io.h"
Expand Down Expand Up @@ -156,6 +157,7 @@ static int usb_configure_dev(struct cras_iodev* iodev) {
aio->common.severe_underrun_frames =
SEVERE_UNDERRUN_MS * iodev->format->frame_rate / 1000;

size_t fmt_bytes = cras_get_format_bytes(iodev->format);
cras_iodev_init_audio_area(iodev);

syslog(LOG_DEBUG,
Expand All @@ -168,6 +170,18 @@ static int usb_configure_dev(struct cras_iodev* iodev) {
goto error_out;
}

if (!aio->common.sample_buf) {
aio->common.sample_buf =
(uint8_t*)calloc(iodev->buffer_size * fmt_bytes, sizeof(uint8_t));
if (!aio->common.sample_buf) {
syslog(LOG_ERR, "cras_alsa_io: configure_dev: calloc: %s",
cras_strerror(errno));
return -ENOMEM;
}
cras_audio_area_config_buf_pointers(iodev->area, iodev->format,
aio->common.sample_buf);
}

// Set channel map to device
rc = cras_alsa_set_channel_map(aio->common.handle, iodev->format);
if (rc < 0) {
Expand Down Expand Up @@ -264,19 +278,30 @@ static int usb_get_buffer(struct cras_iodev* iodev,
struct cras_audio_area** area,
unsigned* frames) {
struct alsa_usb_io* aio = (struct alsa_usb_io*)iodev;
snd_pcm_uframes_t nframes = *frames;
uint8_t* dst = NULL;
size_t format_bytes;
int rc;
snd_pcm_uframes_t nframes = MIN(iodev->buffer_size, *frames);

aio->common.mmap_offset = 0;
format_bytes = cras_get_format_bytes(iodev->format);

rc = cras_alsa_mmap_begin(aio->common.handle, format_bytes, &dst,
&aio->common.mmap_offset, &nframes);
size_t format_bytes = cras_get_format_bytes(iodev->format);

int rc = cras_alsa_mmap_begin(aio->common.handle, format_bytes,
&aio->common.mmap_buf, &aio->common.mmap_offset,
&nframes);
if (rc < 0) {
return rc;
}
iodev->area->frames = nframes;
cras_audio_area_config_buf_pointers(iodev->area, iodev->format, dst);
// Copy mmap_buf data to local memory for faster manipulation.
// Check `cras_bench --benchmark_filter=BM_Alsa/MmapBuffer` for analysis.
if (iodev->direction == CRAS_STREAM_INPUT) {
if (nframes > iodev->input_dsp_offset) {
memcpy(aio->common.sample_buf + iodev->input_dsp_offset * format_bytes,
aio->common.mmap_buf + iodev->input_dsp_offset * format_bytes,
(nframes - iodev->input_dsp_offset) * format_bytes);
} else {
syslog(LOG_WARNING, "nframe is less than input_dsp_offset: %lu > %u",
nframes, iodev->input_dsp_offset);
}
}

*area = iodev->area;
*frames = nframes;
Expand All @@ -287,6 +312,25 @@ static int usb_get_buffer(struct cras_iodev* iodev,
static int usb_put_buffer(struct cras_iodev* iodev, unsigned nwritten) {
struct alsa_usb_io* aio = (struct alsa_usb_io*)iodev;

size_t format_bytes = cras_get_format_bytes(iodev->format);
if (iodev->direction == CRAS_STREAM_OUTPUT) {
memcpy(aio->common.mmap_buf, aio->common.sample_buf,
nwritten * format_bytes);
unsigned int max_offset = cras_iodev_max_stream_offset(iodev);
if (max_offset) {
memmove(aio->common.sample_buf,
aio->common.sample_buf + nwritten * format_bytes,
max_offset * format_bytes);
}
} else {
// CRAS applied input DSP on the uncommitted data, move then to the
// beginning.
if (iodev->input_dsp_offset) {
memmove(aio->common.sample_buf,
aio->common.sample_buf + nwritten * format_bytes,
iodev->input_dsp_offset * format_bytes);
}
}
return cras_alsa_mmap_commit(aio->common.handle, aio->common.mmap_offset,
nwritten);
}
Expand Down
1 change: 1 addition & 0 deletions cras/src/tests/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ cc_test(
name = "cras_alsa_usb_io_unittest",
srcs = [
":cras_alsa_usb_io_unittest.cc",
"//cras/src/common:cras_string.c",
"//cras/src/server:cras_alsa_common_io.c",
"//cras/src/server:cras_alsa_mixer_name.c",
"//cras/src/server:cras_alsa_ucm_section.c",
Expand Down
4 changes: 4 additions & 0 deletions cras/src/tests/alsa_io_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,10 @@ static cras_volume_curve default_curve = {
.get_dBFS = fake_get_dBFS,
};

unsigned int cras_iodev_max_stream_offset(const struct cras_iodev* iodev) {
return 0;
}

static struct cras_iodev* alsa_iodev_create_with_default_parameters(
size_t card_index,
const char* dev_id,
Expand Down
13 changes: 11 additions & 2 deletions cras/src/tests/cras_alsa_usb_io_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ static int sys_get_max_headphone_channels_called = 0;
static int sys_get_max_headphone_channels_return_value = 2;
static int cras_iodev_update_underrun_duration_called = 0;

unsigned int cras_iodev_max_stream_offset(const struct cras_iodev* iodev) {
return 0;
}

void cras_dsp_set_variable_integer(struct cras_dsp_context* ctx,
const char* key,
int value) {
Expand Down Expand Up @@ -319,6 +323,7 @@ TEST(AlsaIoInit, DefaultNodeUSBCard) {
ASSERT_EQ(1, aio->common.base.active_node->plugged);
EXPECT_EQ(1, cras_iodev_set_node_plugged_called);
EXPECT_EQ(2, cras_alsa_mixer_get_playback_step_called);
iodev->close_dev(iodev);
cras_alsa_usb_iodev_destroy(iodev);
free(fake_format);
iodev = cras_alsa_usb_iodev_create_with_default_parameters(
Expand All @@ -336,6 +341,7 @@ TEST(AlsaIoInit, DefaultNodeUSBCard) {
ASSERT_EQ(DEFAULT_CAPTURE_VOLUME_DBFS,
aio->common.base.active_node->intrinsic_sensitivity);
ASSERT_EQ(0, aio->common.base.active_node->capture_gain);
iodev->close_dev(iodev);
cras_alsa_usb_iodev_destroy(iodev);
free(fake_format);
}
Expand All @@ -344,6 +350,7 @@ TEST(AlsaIoInit, OpenCaptureSetCaptureGainWithDefaultUsbDevice) {
struct cras_iodev* iodev;
struct cras_audio_format format;

ResetStubData();
iodev = cras_alsa_usb_iodev_create_with_default_parameters(
0, NULL, ALSA_CARD_TYPE_USB, 0, fake_mixer, fake_config, NULL,
CRAS_STREAM_INPUT);
Expand All @@ -356,13 +363,12 @@ TEST(AlsaIoInit, OpenCaptureSetCaptureGainWithDefaultUsbDevice) {
iodev->active_node->intrinsic_sensitivity = DEFAULT_CAPTURE_VOLUME_DBFS;
iodev->active_node->capture_gain = 0;

ResetStubData();
iodev->open_dev(iodev);
iodev->configure_dev(iodev);

// Not change mixer controls for USB devices without UCM config.
EXPECT_EQ(0, alsa_mixer_set_capture_dBFS_called);

iodev->close_dev(iodev);
cras_alsa_usb_iodev_destroy(iodev);
free(fake_format);
}
Expand Down Expand Up @@ -394,6 +400,7 @@ TEST(AlsaIoInit, MaxSupportedChannels) {
EXPECT_EQ(1, cras_alsa_fill_properties_called);
uint32_t max_channels = (cras_alsa_support_8_channels) ? 8 : 2;
EXPECT_EQ(max_channels, aio->common.base.info.max_supported_channels);
iodev->close_dev(iodev);
cras_alsa_usb_iodev_destroy((struct cras_iodev*)iodev);
free(fake_format);
EXPECT_EQ(1, cras_iodev_free_resources_called);
Expand Down Expand Up @@ -472,6 +479,7 @@ class NodeUSBCardSuite : public testing::Test {
aio->common.base.active_node->number_of_volume_steps);
EXPECT_EQ(expect_enable_software_volume,
aio->common.base.active_node->software_volume_needed);
iodev->close_dev(iodev);
cras_alsa_usb_iodev_destroy(iodev);
free(fake_format);
}
Expand Down Expand Up @@ -507,6 +515,7 @@ class NodeUSBCardSuite : public testing::Test {
} else {
EXPECT_EQ(0, cras_volume_curve_create_simple_step_called);
}
iodev->close_dev(iodev);
cras_alsa_usb_iodev_destroy(iodev);
free(fake_format);
}
Expand Down

0 comments on commit 09c9e51

Please sign in to comment.