r_submix: Add some tests

Add simple tests to verify remote submix behavior regarding
blocking writes. Currently one of the tests fails (does not finish),
this needs to be fixed.

Also fixed some minor issues in remote submix module code.

Bug: 73175392
Test: r_submix_tests
Change-Id: Ic88d0c385c0102e35b4f751f9c5cd8a6488949c8

3 files changed, 187 insertions, 12 deletions

diff --git a/modules/audio_remote_submix/audio_hw.cpp b/modules/audio_remote_submix/audio_hw.cpp
index eb6ae929..8c0c0971 100644
--- a/modules/audio_remote_submix/audio_hw.cpp
+++ b/modules/audio_remote_submix/audio_hw.cpp
@@ -52,9 +52,9 @@ extern "C" {
 
 namespace android {
 
-// Set to 1 to enable extremely verbose logging in this module.
-#define SUBMIX_VERBOSE_LOGGING 0
-#if SUBMIX_VERBOSE_LOGGING
+// Uncomment to enable extremely verbose logging in this module.
+// #define SUBMIX_VERBOSE_LOGGING
+#if defined(SUBMIX_VERBOSE_LOGGING)
 #define SUBMIX_ALOGV(...) ALOGV(__VA_ARGS__)
 #define SUBMIX_ALOGE(...) ALOGE(__VA_ARGS__)
 #else
@@ -205,7 +205,7 @@ struct submix_stream_in {
     int log_fd;
 #endif // LOG_STREAMS_TO_FILES
 
-    volatile int16_t read_error_count;
+    volatile uint16_t read_error_count;
 };
 
 // Determine whether the specified sample rate is supported by the submix module.
@@ -467,11 +467,9 @@ static void submix_audio_device_release_pipe_l(struct submix_audio_device * cons
             rsxadev->routes[route_idx].address);
     if (rsxadev->routes[route_idx].rsxSink != 0) {
         rsxadev->routes[route_idx].rsxSink.clear();
-        rsxadev->routes[route_idx].rsxSink = 0;
     }
     if (rsxadev->routes[route_idx].rsxSource != 0) {
         rsxadev->routes[route_idx].rsxSource.clear();
-        rsxadev->routes[route_idx].rsxSource = 0;
     }
     memset(rsxadev->routes[route_idx].address, 0, AUDIO_DEVICE_MAX_ADDRESS_LEN);
 #ifdef ENABLE_RESAMPLING
@@ -816,8 +814,8 @@ static ssize_t out_write(struct audio_stream_out *stream, const void* buffer,
             static uint8_t flush_buffer[64];
             const size_t flushBufferSizeFrames = sizeof(flush_buffer) / frame_size;
             size_t frames_to_flush_from_source = frames - availableToWrite;
-            SUBMIX_ALOGV("out_write(): flushing %d frames from the pipe to avoid blocking",
-                         frames_to_flush_from_source);
+            SUBMIX_ALOGV("out_write(): flushing %llu frames from the pipe to avoid blocking",
+                    (unsigned long long)frames_to_flush_from_source);
             while (frames_to_flush_from_source) {
                 const size_t flush_size = min(frames_to_flush_from_source, flushBufferSizeFrames);
                 frames_to_flush_from_source -= flush_size;
@@ -898,7 +896,8 @@ static int out_get_presentation_position(const struct audio_stream_out *stream,
     }
 
     SUBMIX_ALOGV("out_get_presentation_position() got frames=%llu timestamp sec=%llu",
-            frames ? *frames : -1, timestamp ? timestamp->tv_sec : -1);
+            frames ? (unsigned long long)*frames : -1ULL,
+            timestamp ? (unsigned long long)timestamp->tv_sec : -1ULL);
 
     return ret;
 }
@@ -1541,7 +1540,7 @@ static size_t adev_get_input_buffer_size(const struct audio_hw_device *dev,
                 audio_bytes_per_sample(config->format);
         const size_t buffer_size = max_buffer_period_size_frames * frame_size_in_bytes;
         SUBMIX_ALOGV("adev_get_input_buffer_size() returns %zu bytes, %zu frames",
-                 buffer_size, buffer_period_size_frames);
+                 buffer_size, max_buffer_period_size_frames);
         return buffer_size;
     }
     return 0;
@@ -1692,10 +1691,10 @@ static int adev_dump(const audio_hw_device_t *device, int fd)
                     reinterpret_cast<const uint8_t *>(device) -
                             offsetof(struct submix_audio_device, device));
     char msg[100];
-    int n = sprintf(msg, "\nReroute submix audio module:\n");
+    int n = snprintf(msg, sizeof(msg), "\nReroute submix audio module:\n");
     write(fd, &msg, n);
     for (int i=0 ; i < MAX_ROUTES ; i++) {
-        n = sprintf(msg, " route[%d] rate in=%d out=%d, addr=[%s]\n", i,
+        n = snprintf(msg, sizeof(msg), " route[%d] rate in=%d out=%d, addr=[%s]\n", i,
                 rsxadev->routes[i].config.input_sample_rate,
                 rsxadev->routes[i].config.output_sample_rate,
                 rsxadev->routes[i].address);
diff --git a/modules/audio_remote_submix/tests/Android.bp b/modules/audio_remote_submix/tests/Android.bp
new file mode 100644
index 00000000..8e4d42d0
--- /dev/null
+++ b/modules/audio_remote_submix/tests/Android.bp
@@ -0,0 +1,29 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+cc_test {
+    name: "r_submix_tests",
+
+    srcs: ["remote_submix_tests.cpp"],
+
+    shared_libs: [
+        "libhardware",
+        "liblog",
+        "libutils",
+    ],
+
+    cflags: ["-Wall", "-Werror", "-O0", "-g",],
+
+    header_libs: ["libaudiohal_headers"],
+}
diff --git a/modules/audio_remote_submix/tests/remote_submix_tests.cpp b/modules/audio_remote_submix/tests/remote_submix_tests.cpp
new file mode 100644
index 00000000..e644fd48
--- /dev/null
+++ b/modules/audio_remote_submix/tests/remote_submix_tests.cpp
@@ -0,0 +1,147 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// To run this test (as root):
+// 1) Build it
+// 2) adb push to /vendor/bin
+// 3) adb shell /vendor/bin/r_submix_tests
+
+#define LOG_TAG "RemoteSubmixTest"
+
+#include <memory>
+
+#include <gtest/gtest.h>
+#include <hardware/audio.h>
+#include <utils/Errors.h>
+#include <utils/Log.h>
+
+using namespace android;
+
+static status_t load_audio_interface(const char *if_name, audio_hw_device_t **dev)
+{
+    const hw_module_t *mod;
+    int rc;
+
+    rc = hw_get_module_by_class(AUDIO_HARDWARE_MODULE_ID, if_name, &mod);
+    if (rc) {
+        ALOGE("%s couldn't load audio hw module %s.%s (%s)", __func__,
+                AUDIO_HARDWARE_MODULE_ID, if_name, strerror(-rc));
+        goto out;
+    }
+    rc = audio_hw_device_open(mod, dev);
+    if (rc) {
+        ALOGE("%s couldn't open audio hw device in %s.%s (%s)", __func__,
+                AUDIO_HARDWARE_MODULE_ID, if_name, strerror(-rc));
+        goto out;
+    }
+    if ((*dev)->common.version < AUDIO_DEVICE_API_VERSION_MIN) {
+        ALOGE("%s wrong audio hw device version %04x", __func__, (*dev)->common.version);
+        rc = BAD_VALUE;
+        audio_hw_device_close(*dev);
+        goto out;
+    }
+    return OK;
+
+out:
+    *dev = NULL;
+    return rc;
+}
+
+class RemoteSubmixTest : public testing::Test {
+  protected:
+    void SetUp() override;
+    void TearDown() override;
+
+    void OpenInputStream(const char *address, audio_stream_in_t** streamIn);
+    void OpenOutputStream(const char *address, audio_stream_out_t** streamOut);
+    void WriteIntoStream(audio_stream_out_t* streamOut, size_t bufferSize, size_t repeats);
+
+    audio_hw_device_t* mDev;
+};
+
+void RemoteSubmixTest::SetUp() {
+    mDev = nullptr;
+    ASSERT_EQ(OK, load_audio_interface(AUDIO_HARDWARE_MODULE_ID_REMOTE_SUBMIX, &mDev));
+    ASSERT_NE(nullptr, mDev);
+}
+
+void RemoteSubmixTest::TearDown() {
+    if (mDev != nullptr) {
+        int status = audio_hw_device_close(mDev);
+        mDev = nullptr;
+        ALOGE_IF(status, "Error closing audio hw device %p: %s", mDev, strerror(-status));
+        ASSERT_EQ(0, status);
+    }
+}
+
+void RemoteSubmixTest::OpenInputStream(const char *address, audio_stream_in_t** streamIn) {
+    *streamIn = nullptr;
+    struct audio_config configIn = {};
+    configIn.channel_mask = AUDIO_CHANNEL_IN_MONO;
+    configIn.sample_rate = 48000;
+    status_t result = mDev->open_input_stream(mDev,
+            AUDIO_IO_HANDLE_NONE, AUDIO_DEVICE_NONE, &configIn,
+            streamIn, AUDIO_INPUT_FLAG_NONE, address, AUDIO_SOURCE_DEFAULT);
+    ASSERT_EQ(OK, result);
+    ASSERT_NE(nullptr, *streamIn);
+}
+
+void RemoteSubmixTest::OpenOutputStream(const char *address, audio_stream_out_t** streamOut) {
+    *streamOut = nullptr;
+    struct audio_config configOut = {};
+    configOut.channel_mask = AUDIO_CHANNEL_OUT_MONO;
+    configOut.sample_rate = 48000;
+    status_t result = mDev->open_output_stream(mDev,
+            AUDIO_IO_HANDLE_NONE, AUDIO_DEVICE_NONE, AUDIO_OUTPUT_FLAG_NONE,
+            &configOut, streamOut, address);
+    ASSERT_EQ(OK, result);
+    ASSERT_NE(nullptr, *streamOut);
+}
+
+void RemoteSubmixTest::WriteIntoStream(
+        audio_stream_out_t* streamOut, size_t bufferSize, size_t repeats) {
+    std::unique_ptr<char[]> buffer(new char[bufferSize]);
+    for (size_t i = 0; i < repeats; ++i) {
+        ssize_t result = streamOut->write(streamOut, buffer.get(), bufferSize);
+        EXPECT_EQ(bufferSize, static_cast<size_t>(result));
+    }
+}
+
+TEST_F(RemoteSubmixTest, InitSuccess) {
+    // SetUp must finish with no assertions.
+}
+
+// Verifies that when no input was opened, writing into an output stream does not block.
+TEST_F(RemoteSubmixTest, OutputDoesNotBlockWhenNoInput) {
+    const char *address = "1";
+    audio_stream_out_t* streamOut;
+    OpenOutputStream(address, &streamOut);
+    WriteIntoStream(streamOut, 1024, 16);
+    mDev->close_output_stream(mDev, streamOut);
+}
+
+// Verifies that when input is opened but not reading, writing into an output stream does not block.
+// !!! Currently does not finish because requires setting a parameter from another thread !!!
+TEST_F(RemoteSubmixTest, OutputDoesNotBlockWhenInputStuck) {
+    const char *address = "1";
+    audio_stream_out_t* streamOut;
+    OpenOutputStream(address, &streamOut);
+    audio_stream_in_t* streamIn;
+    OpenInputStream(address, &streamIn);
+    WriteIntoStream(streamOut, 1024, 16);
+    mDev->close_input_stream(mDev, streamIn);
+    mDev->close_output_stream(mDev, streamOut);
+}