Fix audio record pre-processing
proc_buf_out consistently initialized.
intermediate scratch buffers consistently initialized.
prevent read failure from overwriting memory.
Test: POC, CTS, camera record
Bug: 62873231
Change-Id: Ie26e12a419a5819c1c5c3a0bcf1876d6d7aca686
(cherry picked from commit 6d7b330c27efba944817e647955da48e54fd74eb)
diff --git a/audio/hal/audio_hw.c b/audio/hal/audio_hw.c
index 70a9a41..c20ba98 100644
--- a/audio/hal/audio_hw.c
+++ b/audio/hal/audio_hw.c
@@ -1023,12 +1023,7 @@
ssize_t frames_wr = 0; /* Number of frames actually read */
size_t bytes_per_sample = audio_bytes_per_sample(stream->common.get_format(&stream->common));
void *proc_buf_out = buffer;
-#ifdef PREPROCESSING_ENABLED
- audio_buffer_t in_buf;
- audio_buffer_t out_buf;
- int i;
- bool has_processing = in->num_preprocessors != 0;
-#endif
+
/* Additional channels might be added on top of main_channels:
* - aux_channels (by processing effects)
* - extra channels due to HW limitations
@@ -1037,7 +1032,34 @@
size_t src_channels = in->config.channels;
size_t dst_channels = audio_channel_count_from_in_mask(in->main_channels);
bool channel_remapping_needed = (dst_channels != src_channels);
- size_t src_buffer_size = frames_num * src_channels * bytes_per_sample;
+ const size_t src_frame_size = src_channels * bytes_per_sample;
+
+#ifdef PREPROCESSING_ENABLED
+ const bool has_processing = in->num_preprocessors != 0;
+#else
+ const bool has_processing = false;
+#endif
+
+ /* With additional channels or processing, we need intermediate buffers */
+ if (channel_remapping_needed || has_processing) {
+ const size_t src_buffer_size = frames_num * src_frame_size;
+
+ if (in->proc_buf_size < src_buffer_size) {
+ in->proc_buf_size = src_buffer_size;
+#ifdef PREPROCESSING_ENABLED
+ /* we always reallocate both buffers in case # of effects change dynamically. */
+ in->proc_buf_in = realloc(in->proc_buf_in, src_buffer_size);
+ ALOG_ASSERT((in->proc_buf_in != NULL),
+ "process_frames() failed to reallocate proc_buf_in");
+#endif
+ in->proc_buf_out = realloc(in->proc_buf_out, src_buffer_size);
+ ALOG_ASSERT((in->proc_buf_out != NULL),
+ "process_frames() failed to reallocate proc_buf_out");
+ }
+ if (channel_remapping_needed) {
+ proc_buf_out = in->proc_buf_out;
+ }
+ }
#ifdef PREPROCESSING_ENABLED
if (has_processing) {
@@ -1046,24 +1068,10 @@
while (frames_wr < frames_num) {
/* first reload enough frames at the end of process input buffer */
if (in->proc_buf_frames < (size_t)frames_num) {
- ssize_t frames_rd;
- if (in->proc_buf_size < (size_t)frames_num) {
- in->proc_buf_size = (size_t)frames_num;
- in->proc_buf_in = realloc(in->proc_buf_in, src_buffer_size);
- ALOG_ASSERT((in->proc_buf_in != NULL),
- "process_frames() failed to reallocate proc_buf_in");
- if (channel_remapping_needed) {
- in->proc_buf_out = realloc(in->proc_buf_out, src_buffer_size);
- ALOG_ASSERT((in->proc_buf_out != NULL),
- "process_frames() failed to reallocate proc_buf_out");
- proc_buf_out = in->proc_buf_out;
- }
- }
- frames_rd = read_frames(in,
- in->proc_buf_in +
- in->proc_buf_frames * src_channels * bytes_per_sample,
- frames_num - in->proc_buf_frames);
- if (frames_rd < 0) {
+ ssize_t frames_rd = read_frames(in,
+ (char *)in->proc_buf_in + in->proc_buf_frames * src_frame_size,
+ frames_num - in->proc_buf_frames);
+ if (frames_rd < 0) {
/* Return error code */
frames_wr = frames_rd;
break;
@@ -1073,17 +1081,20 @@
/* in_buf.frameCount and out_buf.frameCount indicate respectively
* the maximum number of frames to be consumed and produced by process() */
+ audio_buffer_t in_buf;
+ audio_buffer_t out_buf;
+
in_buf.frameCount = in->proc_buf_frames;
- in_buf.s16 = in->proc_buf_in;
+ in_buf.s16 = in->proc_buf_in; /* currently assumes PCM 16 effects */
out_buf.frameCount = frames_num - frames_wr;
- out_buf.s16 = (int16_t *)proc_buf_out + frames_wr * in->config.channels;
+ out_buf.s16 = (int16_t *)proc_buf_out + frames_wr * src_channels;
/* FIXME: this works because of current pre processing library implementation that
* does the actual process only when the last enabled effect process is called.
* The generic solution is to have an output buffer for each effect and pass it as
* input to the next.
*/
- for (i = 0; i < in->num_preprocessors; i++) {
+ for (int i = 0; i < in->num_preprocessors; i++) {
(*in->preprocessors[i].effect_itfe)->process(in->preprocessors[i].effect_itfe,
&in_buf,
&out_buf);
@@ -1096,8 +1107,8 @@
if (in->proc_buf_frames) {
memcpy(in->proc_buf_in,
- in->proc_buf_in + in_buf.frameCount * src_channels * bytes_per_sample,
- in->proc_buf_frames * in->config.channels * audio_bytes_per_sample(in_get_format(in)));
+ (char *)in->proc_buf_in + in_buf.frameCount * src_frame_size,
+ in->proc_buf_frames * src_frame_size);
}
/* if not enough frames were passed to process(), read more and retry. */
@@ -1120,23 +1131,14 @@
#endif //PREPROCESSING_ENABLED
{
/* No processing effects attached */
- if (channel_remapping_needed) {
- /* With additional channels, we cannot use original buffer */
- if (in->proc_buf_size < src_buffer_size) {
- in->proc_buf_size = src_buffer_size;
- in->proc_buf_out = realloc(in->proc_buf_out, src_buffer_size);
- ALOG_ASSERT((in->proc_buf_out != NULL),
- "process_frames() failed to reallocate proc_buf_out");
- }
- proc_buf_out = in->proc_buf_out;
- }
frames_wr = read_frames(in, proc_buf_out, frames_num);
ALOG_ASSERT(frames_wr <= frames_num, "read more frames than requested");
}
- if (channel_remapping_needed) {
+ /* check negative frames_wr (error) before channel remapping to avoid overwriting memory. */
+ if (channel_remapping_needed && frames_wr > 0) {
size_t ret = adjust_channels(proc_buf_out, src_channels, buffer, dst_channels,
- bytes_per_sample, frames_wr * src_channels * bytes_per_sample);
+ bytes_per_sample, frames_wr * src_frame_size);
ALOG_ASSERT(ret == (frames_wr * dst_channels * bytes_per_sample));
}
