Fix remote submix crash on null sink
The "remote submix" HAL uses a MonoPipe instance to "pipe" audio
from the sink of this virtual device HAL to its source.
The life-cycle of this pipe is:
- creation when either the input or output stream is open
- destruction when both input and output are closed.
Changes are:
Fix test for pipe destruction: destroy pipe when both
input and output streams are NULL.
Count how many read errors went into the logs and cap them
so as not to spam the logs when the pipe is not properly
set up. Less 'I' logs, 'D' logs instead.
When opening input stream, check for non-null sink before
checking if it's shutdown.
Bug 16653334
Bug 17111907
Change-Id: I634b4192b00f9b74a5109f42242423e9c8cb4c7c
diff --git a/modules/audio_remote_submix/audio_hw.cpp b/modules/audio_remote_submix/audio_hw.cpp
index 014da8e..8fed8e4 100644
--- a/modules/audio_remote_submix/audio_hw.cpp
+++ b/modules/audio_remote_submix/audio_hw.cpp
@@ -95,6 +95,8 @@
// File permissions for stream log files.
#define LOG_STREAM_FILE_PERMISSIONS (S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)
#endif // LOG_STREAMS_TO_FILES
+// limit for number of read error log entries to avoid spamming the logs
+#define MAX_READ_ERROR_LOGS 5
// Common limits macros.
#ifndef min
@@ -194,6 +196,8 @@
#if LOG_STREAMS_TO_FILES
int log_fd;
#endif // LOG_STREAMS_TO_FILES
+
+ volatile int16_t read_error_count;
};
// Determine whether the specified sample rate is supported by the submix module.
@@ -350,7 +354,7 @@
struct submix_stream_out * const out)
{
ALOG_ASSERT(in || out);
- ALOGV("submix_audio_device_create_pipe()");
+ ALOGD("submix_audio_device_create_pipe()");
pthread_mutex_lock(&rsxadev->lock);
// Save a reference to the specified input or output stream and the associated channel
// mask.
@@ -435,7 +439,7 @@
// before they shutdown.
static void submix_audio_device_release_pipe(struct submix_audio_device * const rsxadev)
{
- ALOGV("submix_audio_device_release_pipe()");
+ ALOGD("submix_audio_device_release_pipe()");
rsxadev->rsxSink.clear();
rsxadev->rsxSource.clear();
}
@@ -463,9 +467,9 @@
#endif // ENABLE_LEGACY_INPUT_OPEN
}
if (out != NULL) rsxadev->output = NULL;
- if (rsxadev->input != NULL && rsxadev->output != NULL) {
+ if (rsxadev->input == NULL && rsxadev->output == NULL) {
submix_audio_device_release_pipe(rsxadev);
- ALOGV("submix_audio_device_destroy_pipe(): pipe destroyed");
+ ALOGD("submix_audio_device_destroy_pipe(): pipe destroyed");
}
pthread_mutex_unlock(&rsxadev->lock);
}
@@ -655,7 +659,7 @@
return 0;
}
- ALOGI("out_set_parameters(): shutdown");
+ ALOGD("out_set_parameters(): shutting down MonoPipe sink");
sink->shutdown(true);
} // done using the sink
pthread_mutex_unlock(&rsxadev->lock);
@@ -975,7 +979,9 @@
// about to read from audio source
sp<MonoPipeReader> source = rsxadev->rsxSource;
if (source == NULL) {
- ALOGE("no audio pipe yet we're trying to read!");
+ in->read_error_count++;// ok if it rolls over
+ ALOGE_IF(in->read_error_count < MAX_READ_ERROR_LOGS,
+ "no audio pipe yet we're trying to read! (not all errors will be logged)");
pthread_mutex_unlock(&rsxadev->lock);
usleep(frames_to_read * 1000000 / in_get_sample_rate(&stream->common));
memset(buffer, 0, bytes);
@@ -1192,7 +1198,7 @@
const char *address __unused)
{
struct submix_audio_device * const rsxadev = audio_hw_device_get_submix_audio_device(dev);
- ALOGV("adev_open_output_stream()");
+ ALOGD("adev_open_output_stream()");
struct submix_stream_out *out;
bool force_pipe_creation = false;
(void)handle;
@@ -1247,7 +1253,7 @@
// Store a pointer to the device from the output stream.
out->dev = rsxadev;
// Initialize the pipe.
- ALOGV("adev_open_output_stream(): Initializing pipe");
+ ALOGV("adev_open_output_stream(): about to create pipe");
submix_audio_device_create_pipe(rsxadev, config, DEFAULT_PIPE_SIZE_IN_FRAMES,
DEFAULT_PIPE_PERIOD_COUNT, NULL, out);
#if LOG_STREAMS_TO_FILES
@@ -1267,7 +1273,7 @@
struct audio_stream_out *stream)
{
struct submix_stream_out * const out = audio_stream_out_get_submix_stream_out(stream);
- ALOGV("adev_close_output_stream()");
+ ALOGD("adev_close_output_stream()");
submix_audio_device_destroy_pipe(audio_hw_device_get_submix_audio_device(dev), NULL, out);
#if LOG_STREAMS_TO_FILES
if (out->log_fd >= 0) close(out->log_fd);
@@ -1381,7 +1387,7 @@
{
struct submix_audio_device *rsxadev = audio_hw_device_get_submix_audio_device(dev);
struct submix_stream_in *in;
- ALOGI("adev_open_input_stream()");
+ ALOGD("adev_open_input_stream()");
(void)handle;
(void)devices;
@@ -1402,7 +1408,17 @@
sp<MonoPipe> sink = rsxadev->rsxSink;
ALOG_ASSERT(sink != NULL);
// If the sink has been shutdown, delete the pipe.
- if (sink->isShutdown()) submix_audio_device_release_pipe(rsxadev);
+ if (sink != NULL) {
+ if (sink->isShutdown()) {
+ ALOGD(" Non-NULL shut down sink when opening input stream, releasing, refcount=%d",
+ in->ref_count);
+ submix_audio_device_release_pipe(rsxadev);
+ } else {
+ ALOGD(" Non-NULL sink when opening input stream, refcount=%d", in->ref_count);
+ }
+ } else {
+ ALOGE("NULL sink when opening input stream, refcount=%d", in->ref_count);
+ }
}
pthread_mutex_unlock(&rsxadev->lock);
#else
@@ -1436,7 +1452,9 @@
in->read_counter_frames = 0;
in->output_standby = rsxadev->output_standby;
in->dev = rsxadev;
+ in->read_error_count = 0;
// Initialize the pipe.
+ ALOGV("adev_open_input_stream(): about to create pipe");
submix_audio_device_create_pipe(rsxadev, config, DEFAULT_PIPE_SIZE_IN_FRAMES,
DEFAULT_PIPE_PERIOD_COUNT, in, NULL);
#if LOG_STREAMS_TO_FILES
@@ -1456,7 +1474,7 @@
struct audio_stream_in *stream)
{
struct submix_stream_in * const in = audio_stream_in_get_submix_stream_in(stream);
- ALOGV("adev_close_input_stream()");
+ ALOGD("adev_close_input_stream()");
submix_audio_device_destroy_pipe(audio_hw_device_get_submix_audio_device(dev), in, NULL);
#if LOG_STREAMS_TO_FILES
if (in->log_fd >= 0) close(in->log_fd);
