Fix in-place audio format conversion issues
memcpy_by_audio_format() corner case of same format in-place.
memcpy_to_p24_from_i16() for the first sample in buffer for in-place.
Update memcpy_by_audio_format() documentation.
Test: added native unit test format_tests
Bug: 78598192
Change-Id: I9e538ed4ef233b319b846f4a91c27a86eb605a20
diff --git a/audio_utils/format.c b/audio_utils/format.c
index 1803e3c..50872fc 100644
--- a/audio_utils/format.c
+++ b/audio_utils/format.c
@@ -34,7 +34,10 @@
case AUDIO_FORMAT_PCM_24_BIT_PACKED:
case AUDIO_FORMAT_PCM_32_BIT:
case AUDIO_FORMAT_PCM_8_24_BIT:
- memcpy(dst, src, count * audio_bytes_per_sample(dst_format));
+ if (dst != src) {
+ // TODO: should assert if memory regions overlap.
+ memcpy(dst, src, count * audio_bytes_per_sample(dst_format));
+ }
return;
default:
break;
diff --git a/audio_utils/include/audio_utils/format.h b/audio_utils/include/audio_utils/format.h
index dfcfecd..842bbf0 100644
--- a/audio_utils/include/audio_utils/format.h
+++ b/audio_utils/include/audio_utils/format.h
@@ -54,8 +54,8 @@
* 2) Both dst_format and src_format are identical and of the list given
* in (1). This is a straight copy.
*
- * The destination and source buffers must be completely separate if the destination
- * format size is larger than the source format size. These routines call functions
+ * The destination and source buffers must be completely separate
+ * or point to the same starting buffer address. These routines call functions
* in primitives.h, so descriptions of detailed behavior can be reviewed there.
*
* Logs a fatal error if dst or src format is not allowed by the conversion rules above.
diff --git a/audio_utils/primitives.c b/audio_utils/primitives.c
index d88d701..594f1c5 100644
--- a/audio_utils/primitives.c
+++ b/audio_utils/primitives.c
@@ -138,14 +138,15 @@
src += count;
for (; count > 0; --count) {
dst -= 3;
+ const int16_t sample = *--src;
#if HAVE_BIG_ENDIAN
- dst[0] = *--src >> 8;
- dst[1] = *src;
+ dst[0] = sample >> 8;
+ dst[1] = sample;
dst[2] = 0;
#else
dst[0] = 0;
- dst[1] = *--src;
- dst[2] = *src >> 8;
+ dst[1] = sample;
+ dst[2] = sample >> 8;
#endif
}
}
diff --git a/audio_utils/tests/Android.bp b/audio_utils/tests/Android.bp
index 8952046..e0403aa 100644
--- a/audio_utils/tests/Android.bp
+++ b/audio_utils/tests/Android.bp
@@ -225,3 +225,25 @@
"-Werror",
],
}
+
+cc_test {
+ name: "format_tests",
+ host_supported: true,
+
+ shared_libs: [
+ "liblog",
+ ],
+ srcs: ["format_tests.cpp"],
+ cflags: [
+ "-Werror",
+ "-Wall",
+ ],
+ target: {
+ android: {
+ shared_libs: ["libaudioutils"],
+ },
+ host: {
+ static_libs: ["libaudioutils"],
+ },
+ }
+}
diff --git a/audio_utils/tests/build_and_run_all_unit_tests.sh b/audio_utils/tests/build_and_run_all_unit_tests.sh
index 2cfff84..78de692 100755
--- a/audio_utils/tests/build_and_run_all_unit_tests.sh
+++ b/audio_utils/tests/build_and_run_all_unit_tests.sh
@@ -35,6 +35,10 @@
adb push $OUT/data/nativetest/string_tests/string_tests /system/bin
adb shell /system/bin/string_tests
+echo "format tests"
+adb push $OUT/data/nativetest/format_tests/format_tests /system/bin
+adb shell /system/bin/format_tests
+
echo "benchmarking primitives"
adb push $OUT/system/bin/primitives_benchmark /system/bin
adb shell /system/bin/primitives_benchmark
diff --git a/audio_utils/tests/format_tests.cpp b/audio_utils/tests/format_tests.cpp
new file mode 100644
index 0000000..0526b30
--- /dev/null
+++ b/audio_utils/tests/format_tests.cpp
@@ -0,0 +1,127 @@
+/*
+ * Copyright 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.
+ */
+
+//#define LOG_NDEBUG 0
+#define LOG_TAG "audio_utils_format_tests"
+#include <log/log.h>
+
+#include <audio_utils/format.h>
+#include <gtest/gtest.h>
+
+/** returns true if the format is a common source or destination format.
+ memcpy_by_audio_format() allows interchange between any PCM format and the
+ "common" PCM 16 bit and PCM float formats. */
+static bool is_common_format(audio_format_t format) {
+ return format == AUDIO_FORMAT_PCM_16_BIT || format == AUDIO_FORMAT_PCM_FLOAT;
+}
+
+// Initialize PCM 16 bit ramp for basic data sanity check (generated from PCM 8 bit data).
+// TODO: consider creating fillPseudoRandomValue().
+template<size_t size>
+static void fillRamp(int16_t(&buffer)[size])
+{
+ // Create PCM 16 bit data based on PCM 8 bit format because PCM 8 bit is convertible
+ // to all other audio formats without loss; hence, round trip conversion preserves equality.
+ uint8_t bytes[size];
+ for (size_t i = 0; i < size; ++i) {
+ bytes[i] = i;
+ }
+ // convert to PCM 16 bit
+ memcpy_by_audio_format(
+ buffer, AUDIO_FORMAT_PCM_16_BIT,
+ bytes, AUDIO_FORMAT_PCM_8_BIT, size);
+
+ uint8_t check[size];
+ memcpy_by_audio_format(
+ check, AUDIO_FORMAT_PCM_8_BIT,
+ buffer, AUDIO_FORMAT_PCM_16_BIT, size);
+ EXPECT_EQ(0, memcmp(check, bytes, size));
+}
+
+class FormatTest : public testing::TestWithParam<std::tuple<audio_format_t, audio_format_t>>
+{
+};
+
+TEST_P(FormatTest, memcpy_by_audio_format)
+{
+ // fetch parameters
+ const auto param = GetParam();
+ const audio_format_t src_encoding = std::get<0>(param);
+ const audio_format_t dst_encoding = std::get<1>(param);
+
+ // either source or destination (or both) need to be a common format
+ if (!is_common_format(src_encoding) && !is_common_format(dst_encoding)) {
+ printf("skip conversion src:%#x dst:%#x\n", src_encoding, dst_encoding);
+ return;
+ }
+
+ constexpr size_t SAMPLES = UINT8_MAX;
+ constexpr audio_format_t orig_encoding = AUDIO_FORMAT_PCM_16_BIT;
+ int16_t orig_data[SAMPLES];
+
+ fillRamp(orig_data);
+
+ // data buffer for in-place conversion (uint32_t is maximum sample size of 4 bytes)
+ uint32_t data[SAMPLES];
+ // check buffer is used to compare out-of-place vs in-place conversion.
+ uint32_t check[SAMPLES];
+
+ printf("trying conversion src:%#x dst:%#x\n", src_encoding, dst_encoding);
+ fflush(stdout);
+ // Copy original data to data buffer at src_encoding.
+ memcpy_by_audio_format(
+ data, src_encoding,
+ orig_data, orig_encoding, SAMPLES);
+
+ // Convert from src encoding to dst encoding.
+ memcpy_by_audio_format(
+ check, dst_encoding,
+ data, src_encoding, SAMPLES);
+
+ // Check in-place is same as out-of-place conversion.
+ memcpy_by_audio_format(
+ data, dst_encoding,
+ data, src_encoding, SAMPLES);
+ EXPECT_EQ(0, memcmp(check, data, SAMPLES * audio_bytes_per_sample(dst_encoding)));
+
+ // Go back to the original data encoding for comparison.
+ memcpy_by_audio_format(
+ data, orig_encoding,
+ data, dst_encoding, SAMPLES);
+
+ // Raw byte compare at the original encoding must succeed - our conversions
+ // must be lossless for PCM 8 bit representation which orig_data was constructed from.
+ EXPECT_EQ(0,
+ memcmp(data, orig_data, SAMPLES * audio_bytes_per_sample(orig_encoding)));
+}
+
+INSTANTIATE_TEST_CASE_P(FormatVariations, FormatTest, ::testing::Combine(
+ ::testing::Values(
+ AUDIO_FORMAT_PCM_8_BIT,
+ AUDIO_FORMAT_PCM_16_BIT,
+ AUDIO_FORMAT_PCM_FLOAT,
+ AUDIO_FORMAT_PCM_24_BIT_PACKED,
+ AUDIO_FORMAT_PCM_32_BIT,
+ AUDIO_FORMAT_PCM_8_24_BIT
+ ),
+ ::testing::Values(
+ AUDIO_FORMAT_PCM_8_BIT,
+ AUDIO_FORMAT_PCM_16_BIT,
+ AUDIO_FORMAT_PCM_FLOAT,
+ AUDIO_FORMAT_PCM_24_BIT_PACKED,
+ AUDIO_FORMAT_PCM_32_BIT,
+ AUDIO_FORMAT_PCM_8_24_BIT
+ )));