-
Notifications
You must be signed in to change notification settings - Fork 19
tinyhal: audio: Impl channel adjustment for input sound flow #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,10 @@ ifeq ($(strip $(TINYHAL_COMPRESS_PLAYBACK)),true) | |
| LOCAL_CFLAGS += -DTINYHAL_COMPRESS_PLAYBACK | ||
| endif | ||
|
|
||
| ifeq ($(TARGET_DEVICE),kingfisher) | ||
| LOCAL_CFLAGS += -DPLATFORM_RCAR3 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... and also use a generic name for this define instead of the name of a platform |
||
| endif | ||
|
|
||
| include $(BUILD_SHARED_LIBRARY) | ||
|
|
||
| endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ | |
| #endif | ||
|
|
||
| #include <audio_utils/resampler.h> | ||
| #include <audio_utils/channels.h> | ||
|
|
||
| #include <tinyhal/audio_config.h> | ||
|
|
||
|
|
@@ -57,6 +58,23 @@ | |
| #include <vendor/cirrus/scchal/scc_audio.h> | ||
| #endif | ||
|
|
||
| #ifdef PLATFORM_RCAR3 | ||
| /* R-Car3 driver supports only 8-channel streams and 48000 sample rate */ | ||
| /* These values are defined in _frames_ (not bytes) to match the ALSA API */ | ||
| #define OUT_PERIOD_SIZE_DEFAULT 256 | ||
| #define OUT_PERIOD_COUNT_DEFAULT 4 | ||
| #define OUT_CHANNEL_MASK_DEFAULT AUDIO_CHANNEL_OUT_7POINT1 | ||
| #define OUT_CHANNEL_COUNT_DEFAULT 8 | ||
| #define OUT_RATE_DEFAULT 48000 | ||
|
|
||
| #define IN_PERIOD_SIZE_DEFAULT 256 | ||
| #define IN_PERIOD_COUNT_DEFAULT 4 | ||
| #define IN_CHANNEL_MASK_DEFAULT AUDIO_CHANNEL_IN_STEREO | ||
| #define IN_CHANNEL_COUNT_DEFAULT 8 | ||
| #define IN_RATE_DEFAULT 48000 | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is all a nasty hack, changing the default values to match some specific hardware instead of using the configuration. The correct way to do this is use a defined constant in the xml file (https://github.com/CirrusLogic/tinyhal/blob/master/audio.example.xml#L342) to set the hardware channel count. Or read the limits from ALSA and work it out automatically.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you use a <set> constant I suggest naming it "hw-channels" |
||
| #else | ||
|
|
||
| /* These values are defined in _frames_ (not bytes) to match the ALSA API */ | ||
| #define OUT_PERIOD_SIZE_DEFAULT 256 | ||
| #define OUT_PERIOD_COUNT_DEFAULT 4 | ||
|
|
@@ -70,6 +88,8 @@ | |
| #define IN_CHANNEL_COUNT_DEFAULT 1 | ||
| #define IN_RATE_DEFAULT 44100 | ||
|
|
||
| #endif | ||
|
|
||
| #define IN_PCM_BUFFER_SIZE_DEFAULT \ | ||
| (IN_PERIOD_SIZE_DEFAULT * IN_CHANNEL_COUNT_DEFAULT * sizeof(uint16_t)) | ||
|
|
||
|
|
@@ -234,6 +254,9 @@ struct stream_in_pcm { | |
| uint32_t period_size; /* ... of PCM input */ | ||
|
|
||
| struct in_resampler resampler; | ||
|
|
||
| void *pre_read_buf; /* Buffer to store read data before channel adjustment*/ | ||
| size_t pre_read_buf_size; | ||
| }; | ||
|
|
||
| static uint32_t out_get_sample_rate(const struct audio_stream *stream); | ||
|
|
@@ -1660,6 +1683,11 @@ static unsigned int in_pcm_cfg_rate(struct stream_in_pcm *in) | |
|
|
||
| static unsigned int in_pcm_cfg_channel_count(struct stream_in_pcm *in) | ||
| { | ||
| #ifdef PLATFORM_RCAR3 | ||
| /* On R-Car3 driver supports only 8 channels reading */ | ||
| return IN_CHANNEL_COUNT_DEFAULT; | ||
| #endif | ||
|
|
||
| if (in->common.channel_count != 0) { | ||
| return in->common.channel_count; | ||
| } else { | ||
|
|
@@ -1741,6 +1769,22 @@ static int do_open_pcm_input(struct stream_in_pcm *in) | |
|
|
||
| ALOGV("input buffer size=0x%zx", in->common.buffer_size); | ||
|
|
||
| if (in->common.channel_count != in->hw_channel_count) { | ||
| ALOGV("Setup buffer to read %d channel stream with adjustment (HW only supports %d channel reading)", | ||
| in->common.channel_count, in->hw_channel_count); | ||
| in->pre_read_buf_size = in->common.buffer_size * in->hw_channel_count / in->common.channel_count; | ||
| ALOGV("in->pre_read_buf_size=0x%zx", in->pre_read_buf_size); | ||
| in->pre_read_buf = calloc(1, in->pre_read_buf_size); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. malloc()? |
||
| if (!in->pre_read_buf) { | ||
| ret = -ENOMEM; | ||
| goto fail; | ||
| } | ||
| } | ||
| else { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Preferred style is } else { all on one line |
||
| in->pre_read_buf = NULL; | ||
| in->pre_read_buf_size = 0; | ||
| } | ||
|
|
||
| /* | ||
| * If the stream rate differs from the PCM rate, we need to | ||
| * create a resampler. | ||
|
|
@@ -1758,6 +1802,8 @@ static int do_open_pcm_input(struct stream_in_pcm *in) | |
| fail: | ||
| pcm_close(in->pcm); | ||
| in->pcm = NULL; | ||
| if (in->pre_read_buf) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need to check for NULL pointer. It is safe to pass NULL to free() |
||
| free(in->pre_read_buf); | ||
| exit: | ||
| ALOGV("-do_open_pcm_input error:%d", ret); | ||
| return ret; | ||
|
|
@@ -1864,6 +1910,8 @@ static ssize_t do_in_pcm_read(struct audio_stream_in *stream, void *buffer, | |
| int ret = 0; | ||
| struct stream_in_pcm *in = (struct stream_in_pcm *)stream; | ||
| size_t frames_rq = bytes / in->common.frame_size; | ||
| void *read_buf; | ||
| size_t read_buf_size; | ||
|
|
||
| ALOGV("+do_in_pcm_read %zu", bytes); | ||
|
|
||
|
|
@@ -1874,10 +1922,39 @@ static ssize_t do_in_pcm_read(struct audio_stream_in *stream, void *buffer, | |
| goto exit; | ||
| } | ||
|
|
||
| if (in->pre_read_buf) { | ||
| read_buf = in->pre_read_buf; | ||
| read_buf_size = in->pre_read_buf_size; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assumes bytes == (pre_read_buf_size / in->hw_channel_count) * in->common.channel_count |
||
| } else { | ||
| read_buf = buffer; | ||
| read_buf_size = bytes; | ||
| } | ||
|
|
||
| if (in->resampler.resampler != NULL) { | ||
| ret = read_resampled_frames(in, buffer, frames_rq); | ||
| ret = read_resampled_frames(in, read_buf, frames_rq); | ||
| } else { | ||
| ret = pcm_read(in->pcm, buffer, bytes); | ||
| ret = pcm_read(in->pcm, read_buf, read_buf_size); | ||
| } | ||
|
|
||
| if (ret < 0) | ||
| goto exit; | ||
|
|
||
rfvirgil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (in->common.channel_count != in->hw_channel_count) { | ||
|
|
||
| ALOGV("do_in_pcm_read(): Adjust %d channel read stream to %d channel stream", | ||
| in->hw_channel_count, in->common.channel_count); | ||
|
|
||
| bytes = adjust_channels(read_buf, /* Input buffer */ | ||
| in->hw_channel_count, /* Input channel count */ | ||
| buffer, /* Output buffer */ | ||
| in->common.channel_count, /* Output channel count */ | ||
| in->common.frame_size / in->common.channel_count, /* Sample size */ | ||
| read_buf_size /* Size of input buffer in bytes */ | ||
| ); | ||
| if (!bytes) { | ||
| ALOGE("do_in_pcm_read(): Failed to adjust channels"); | ||
| ret = -1; | ||
| } | ||
| } | ||
|
|
||
| /* Assume any non-negative return is a successful read */ | ||
|
|
@@ -2015,6 +2092,14 @@ static void do_close_in_pcm(struct audio_stream *stream) | |
| { | ||
| struct stream_in_pcm *in = (struct stream_in_pcm *)stream; | ||
|
|
||
| ALOGV("do_close_in_pcm(%p)", in); | ||
| if (in && in->pre_read_buf) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need to check pre_read_buf for NULL. It is safe to pass NULL to free(). |
||
| { | ||
| free(in->pre_read_buf); | ||
| in->pre_read_buf = NULL; | ||
| in->pre_read_buf_size = 0; | ||
| } | ||
|
|
||
| do_close_in_common(stream); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't create defines and dependencies for a specific target. Make a generic define that describes the functionality it controls and define it in your device.mk. For example see the 3 defines above (lines 66..77)