Hi Lukasz,
On Mon, Dec 30, 2013 at 1:33 PM, Lukasz Rymanowski
<lukasz.rymanowski@xxxxxxxxx> wrote:
> Hi Luiz,
>
> On 30 December 2013 12:27, Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote:
>> Hi Lukasz,
>>
>> On Mon, Dec 30, 2013 at 12:17 PM, Lukasz Rymanowski
>> <lukasz.rymanowski@xxxxxxxxx> wrote:
>>> This patch add wrapping struct for audio_hw_dev and audio_stream_out.
>>> We will need this to keep some more addition info related to a2dp stream
>>> and also hal IPC later on
>>>
>>> ---
>>> android/Makefile.am | 1 +
>>> android/hal-audio.c | 97 +++++++++++++++++++++++++++++------------------------
>>> 2 files changed, 55 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>> index dec81ce..eaf39bd 100644
>>> --- a/android/Makefile.am
>>> +++ b/android/Makefile.am
>>> @@ -104,6 +104,7 @@ android_android_tester_LDFLAGS = -pthread
>>> noinst_LTLIBRARIES += android/libaudio-internal.la
>>>
>>> android_libaudio_internal_la_SOURCES = android/hal-audio.c \
>>> + android/hal-audio.h \
>>> android/hardware/audio.h \
>>> android/hardware/audio_effect.h \
>>> android/hardware/hardware.h \
>>> diff --git a/android/hal-audio.c b/android/hal-audio.c
>>> index 7f4a3f2..011a699 100644
>>> --- a/android/hal-audio.c
>>> +++ b/android/hal-audio.c
>>> @@ -25,6 +25,15 @@
>>>
>>> #include "hal-log.h"
>>>
>>> +struct a2dp_audio_dev {
>>> + struct audio_hw_device dev;
>>> + struct a2dp_stream_out *stream_out;
>>> +};
>>> +
>>> +struct a2dp_stream_out {
>>> + struct audio_stream_out stream;
>>> +};
>>> +
>>> static ssize_t out_write(struct audio_stream_out *stream, const void *buffer,
>>> size_t bytes)
>>> {
>>> @@ -230,32 +239,34 @@ static int audio_open_output_stream(struct audio_hw_device *dev,
>>> struct audio_stream_out **stream_out)
>>>
>>> {
>>> - struct audio_stream_out *out;
>>> + struct a2dp_audio_dev *a2dp_dev = (struct a2dp_audio_dev *)dev;
>>> + struct a2dp_stream_out *a2dp_out;
>>>
>>> - out = calloc(1, sizeof(struct audio_stream_out));
>>> - if (!out)
>>> + a2dp_out = calloc(1, sizeof(struct a2dp_stream_out));
>>> + if (!a2dp_out)
>>> return -ENOMEM;
>>>
>>> DBG("");
>>>
>>> - out->common.get_sample_rate = out_get_sample_rate;
>>> - out->common.set_sample_rate = out_set_sample_rate;
>>> - out->common.get_buffer_size = out_get_buffer_size;
>>> - out->common.get_channels = out_get_channels;
>>> - out->common.get_format = out_get_format;
>>> - out->common.set_format = out_set_format;
>>> - out->common.standby = out_standby;
>>> - out->common.dump = out_dump;
>>> - out->common.set_parameters = out_set_parameters;
>>> - out->common.get_parameters = out_get_parameters;
>>> - out->common.add_audio_effect = out_add_audio_effect;
>>> - out->common.remove_audio_effect = out_remove_audio_effect;
>>> - out->get_latency = out_get_latency;
>>> - out->set_volume = out_set_volume;
>>> - out->write = out_write;
>>> - out->get_render_position = out_get_render_position;
>>> + a2dp_out->stream.common.get_sample_rate = out_get_sample_rate;
>>> + a2dp_out->stream.common.set_sample_rate = out_set_sample_rate;
>>> + a2dp_out->stream.common.get_buffer_size = out_get_buffer_size;
>>> + a2dp_out->stream.common.get_channels = out_get_channels;
>>> + a2dp_out->stream.common.get_format = out_get_format;
>>> + a2dp_out->stream.common.set_format = out_set_format;
>>> + a2dp_out->stream.common.standby = out_standby;
>>> + a2dp_out->stream.common.dump = out_dump;
>>> + a2dp_out->stream.common.set_parameters = out_set_parameters;
>>> + a2dp_out->stream.common.get_parameters = out_get_parameters;
>>> + a2dp_out->stream.common.add_audio_effect = out_add_audio_effect;
>>> + a2dp_out->stream.common.remove_audio_effect = out_remove_audio_effect;
>>> + a2dp_out->stream.get_latency = out_get_latency;
>>> + a2dp_out->stream.set_volume = out_set_volume;
>>> + a2dp_out->stream.write = out_write;
>>> + a2dp_out->stream.get_render_position = out_get_render_position;
>>>
>>> - *stream_out = out;
>>> + *stream_out = &a2dp_out->stream;
>>> + a2dp_dev->stream_out = a2dp_out;
>>>
>>> return 0;
>>> }
>>> @@ -381,7 +392,7 @@ static int audio_close(hw_device_t *device)
>>> static int audio_open(const hw_module_t *module, const char *name,
>>> hw_device_t **device)
>>> {
>>> - struct audio_hw_device *audio;
>>> + struct a2dp_audio_dev *a2dp_dev;
>>>
>>> DBG("");
>>>
>>> @@ -391,30 +402,30 @@ static int audio_open(const hw_module_t *module, const char *name,
>>> return -EINVAL;
>>> }
>>>
>>> - audio = calloc(1, sizeof(struct audio_hw_device));
>>> - if (!audio)
>>> + a2dp_dev = calloc(1, sizeof(struct a2dp_audio_dev));
>>> + if (!a2dp_dev)
>>> return -ENOMEM;
>>>
>>> - audio->common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>> - audio->common.module = (struct hw_module_t *) module;
>>> - audio->common.close = audio_close;
>>> -
>>> - audio->init_check = audio_init_check;
>>> - audio->set_voice_volume = audio_set_voice_volume;
>>> - audio->set_master_volume = audio_set_master_volume;
>>> - audio->set_mode = audio_set_mode;
>>> - audio->set_mic_mute = audio_set_mic_mute;
>>> - audio->get_mic_mute = audio_get_mic_mute;
>>> - audio->set_parameters = audio_set_parameters;
>>> - audio->get_parameters = audio_get_parameters;
>>> - audio->get_input_buffer_size = audio_get_input_buffer_size;
>>> - audio->open_output_stream = audio_open_output_stream;
>>> - audio->close_output_stream = audio_close_output_stream;
>>> - audio->open_input_stream = audio_open_input_stream;
>>> - audio->close_input_stream = audio_close_input_stream;
>>> - audio->dump = audio_dump;
>>> -
>>> - *device = &audio->common;
>>> + a2dp_dev->dev.common.version = AUDIO_DEVICE_API_VERSION_CURRENT;
>>> + a2dp_dev->dev.common.module = (struct hw_module_t *) module;
>>> + a2dp_dev->dev.common.close = audio_close;
>>> +
>>> + a2dp_dev->dev.init_check = audio_init_check;
>>> + a2dp_dev->dev.set_voice_volume = audio_set_voice_volume;
>>> + a2dp_dev->dev.set_master_volume = audio_set_master_volume;
>>> + a2dp_dev->dev.set_mode = audio_set_mode;
>>> + a2dp_dev->dev.set_mic_mute = audio_set_mic_mute;
>>> + a2dp_dev->dev.get_mic_mute = audio_get_mic_mute;
>>> + a2dp_dev->dev.set_parameters = audio_set_parameters;
>>> + a2dp_dev->dev.get_parameters = audio_get_parameters;
>>> + a2dp_dev->dev.get_input_buffer_size = audio_get_input_buffer_size;
>>> + a2dp_dev->dev.open_output_stream = audio_open_output_stream;
>>> + a2dp_dev->dev.close_output_stream = audio_close_output_stream;
>>> + a2dp_dev->dev.open_input_stream = audio_open_input_stream;
>>> + a2dp_dev->dev.close_input_stream = audio_close_input_stream;
>>> + a2dp_dev->dev.dump = audio_dump;
>>> +
>>> + *device = &a2dp_dev->dev.common;
>>>
>>> return 0;
>>> }
>>> --
>>> 1.8.4
>>
>> How is this supposed to be accessed, are you planning on adding a
>> global variable?
>>
>>
> Have a look on audio_open_output_stream(). This is actually how they did so
> far and it is fine for me.
> I want to avoid globals here.
I can see that you allocate a different struct for a2dp_dev and
a2dp_out, but then you return a member of the struct back to HAL but
after that you wont access these structs anymore and the HAL only uses
the members that you return so you probably gonna need to lookup for
these structs to access any auxiliary data that we might want to add,
also these structs never seems to be freed which I suppose can
generate memory leaks.
--
Luiz Augusto von Dentz
--
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
