Re: [RFC] android/hal-audio: Fix wrong memory access

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Andrzej,

>>>>> downmix_buf is allocated to have buffer size FIXED_BUFFER_SIZE / 2, when
>>>>> we access it as (int16_t *) we shall device index by 2.
>>>>> ---
>>>>> android/hal-audio.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> 
>>>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>>>> index 7305bb6..96fa5c3 100644
>>>>> --- a/android/hal-audio.c
>>>>> +++ b/android/hal-audio.c
>>>>> @@ -984,7 +984,7 @@ static void downmix_to_mono(struct a2dp_stream_out *out, const uint8_t *buffer,
>>>>>     int16_t *output = (void *) out->downmix_buf;
>>>>>     size_t i;
>>>>> 
>>>>> -     for (i = 0; i < bytes / 2; i++) {
>>>>> +     for (i = 0; i < bytes / (2 * sizeof(int16_t)); i++) {
>>>>>             int16_t l = le16_to_cpu(get_unaligned(&input[i * 2]));
>>>>>             int16_t r = le16_to_cpu(get_unaligned(&input[i * 2 + 1]));
>>>> 
>>>> I wonder actually what this get_unaligned is doing here? You cast the const void into const int16_t buffer. Is this really needed? Where is our input and output buffer coming from? Aren’t these aligned anyway? Meaning aren’t they allocated?
>>> 
>>> We have this buffer from AudioFlinger so we don't actually know
>>> alignment or if this is pointer to beginning of some internal buffer
>>> (it's most probably both, but I don't think we should assume this).
>> 
>> and audio system that does not give you a guarantee here on the alignment is utterly screwed up. Seriously, that is just bad for performance. Especially bad on ARM CPUs, so I doubt that they have not thought about this.
> 
> This is only to be on safe side and in terms of performance it does
> not really matter here since this code won't probably be used at all -
> who has mono A2DP headset nowadays? But of course we can change this
> with proper comment. Actually I use it like this in aptX where samples
> are accessed from the same buffer.

I have no idea what that means. The important part is that the buffer itself is aligned. With binary codecs like aptX this is even more important since we have no idea if aptX does the unaligned access or not.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux