On 12/16/18 5:48 AM, Pali Rohár wrote:
> On Thursday 13 December 2018 19:43:36 EHfive wrote:
>> diff --git a/src/modules/bluetooth/a2dp/a2dp-api.h b/src/modules/bluetooth/a2dp/a2dp-api.h
>> new file mode 100644
>> index 000000000..b357555d0
>> --- /dev/null
>> +++ b/src/modules/bluetooth/a2dp/a2dp-api.h
>> @@ -0,0 +1,170 @@
>> +#ifndef fooa2dpcodecapifoo
>> +#define fooa2dpcodecapifoo
>> +
>> +#ifdef HAVE_CONFIG_H
>> +
>> +#include <config.h>
>> +
>> +#endif
>> +
>> +#include <pulse/sample.h>
>> +#include <pulse/proplist.h>
>> +#include <pulsecore/hashmap.h>
>> +
>> +#include "a2dp-codecs.h"
>> +#include "rtp.h"
>> +
>> +typedef struct pa_a2dp_codec pa_a2dp_codec_t;
>> +typedef struct pa_a2dp_config pa_a2dp_config_t;
>> +
>> +extern const pa_a2dp_codec_t pa_a2dp_sbc;
>> +
>> +
>> +/* Run from <pa_a2dp_sink_t>.encode */
>> +
>> +typedef void (*pa_a2dp_source_read_cb_t)(const void **read_buf, size_t read_buf_size, void *data);
>> +
>> +typedef void (*pa_a2dp_source_read_buf_free_cb_t)(const void **read_buf, void *data);
>> +
>> +
>> +// Larger index stands for higher priority by default
>> +typedef enum pa_a2dp_codec_index {
>> + PA_A2DP_SINK_MIN,
>> + PA_A2DP_SINK_SBC,
>> + PA_A2DP_SINK_MAX,
>> + PA_A2DP_SOURCE_MIN = PA_A2DP_SINK_MAX,
>> + PA_A2DP_SOURCE_SBC,
>> + PA_A2DP_SOURCE_MAX,
>> + PA_A2DP_CODEC_INDEX_UNAVAILABLE
>> +} pa_a2dp_codec_index_t;
> This priority enum seems to be strange. One enum mixes both sink and
> sources priorities? And why sink max has same value as source min?
Because there are some functions in a2dp-codec needs it, one enum let
functions more concise.
"PA_A2DP_SOURCE_MIN = PA_A2DP_SINK_MAX" is no sense.
>> +typedef struct pa_a2dp_sink {
>> + int priority;
>> +
>> + /* Load decoder if it's not loaded; Return true if it's loaded */
>> + bool (*decoder_load)();
> It is really needed? Cannot be decoder library dynamically linked into
> pulseaudio bluez module, and therefore by definition always loaded?
If <pa_a2dp_codec_index>'s encoder/decoder not loaded, the load function
should return false, then "pa_a2dp_init()" would not add that codec
index to indices lists.
In this patch, all codecs libraries dynamically linked so the
"decoder_load()" always return true;
Except ffmpeg, which require a motion for loading aptX(HD) encoder/decoder.
It just support dynamic loading .
> Support for dlopening decoders at runtime just cause problems. Some
> users would have installed needed libraries and it work without
> problems. Some not and it will not work... And all users have same
> pulseaudio version and compiled with same flags..
>
>> + /* Memory management is pa_a2dp_sink's work */
>> + bool (*init)(void **codec_data);
>> +
>> + /* Optional. Update user configurations
>> + * Note: not transport 'configuration' or 'capabilities' */
>> + int (*update_user_config)(pa_proplist *user_config, void **codec_data);
>> +
>> + void (*config_transport)(pa_sample_spec default_sample_spec, const void *configuration, size_t configuration_size,
>> + pa_sample_spec *sample_spec, void **codec_data);
>> +
>> + void (*get_block_size)(size_t read_link_mtu, size_t *read_block_size, void **codec_data);
>> +
>> + void (*setup_stream)(void **codec_data);
>> +
>> + size_t
>> + (*decode)(const void *read_buf, size_t read_buf_size, void *write_buf, size_t write_buf_size, size_t *decoded,
>> + uint32_t *timestamp, void **codec_data);
>> +
>> + void (*free)(void **codec_data);
>> +} pa_a2dp_sink_t;
>> +
>> +
>> +typedef struct pa_a2dp_source {
>> + int priority;
>> +
>> + /* Load encoder if it's not loaded; Return true if it's loaded */
>> + bool (*encoder_load)();
>> +
>> + /* Memory management is pa_a2dp_source's work */
>> + bool (*init)(pa_a2dp_source_read_cb_t read_cb, pa_a2dp_source_read_buf_free_cb_t free_cb, void **codec_data);
>> +
>> + /* Optional. Update user configurations
>> + * Note: not transport 'configuration' or 'capabilities' */
>> + int (*update_user_config)(pa_proplist *user_config, void **codec_data);
>> +
>> + void (*config_transport)(pa_sample_spec default_sample_spec, const void *configuration, size_t configuration_size,
>> + pa_sample_spec *sample_spec, void **codec_data);
>> +
>> + void (*get_block_size)(size_t write_link_mtu, size_t *write_block_size, void **codec_data);
>> +
>> + void (*setup_stream)(void **codec_data);
>> +
>> + /* The implement should pass read_cb_data to pa_a2dp_source_read_cb, pa_a2dp_source_read_buf_free_cb */
>> + size_t (*encode)(uint32_t timestamp, void *write_buf, size_t write_buf_size, size_t *encoded,
>> + void *read_cb_data, void **codec_data);
>> +
>> + /* Optional */
>> + void (*set_tx_length)(size_t len, void **codec_data);
>> +
>> + /* Optional */
>> + void (*decrease_quality)(void **codec_data);
>> +
>> + void (*free)(void **codec_data);
>> +} pa_a2dp_source_t;
>> +
>> +
>> +struct pa_a2dp_codec {
>> + const char *name;
>> + uint8_t codec;
>> + const a2dp_vendor_codec_t *vendor_codec;
>> + pa_a2dp_sink_t *a2dp_sink;
>> + pa_a2dp_source_t *a2dp_source;
>> +
>> + /* Memory management is pa_a2dp_codec's work */
>> + size_t (*get_capabilities)(void **capabilities);
>> +
>> + void (*free_capabilities)(void **capabilities);
>> +
>> + size_t (*select_configuration)(const pa_sample_spec default_sample_spec, const uint8_t *supported_capabilities,
>> + const size_t capabilities_size, void **configuration);
>> +
>> + void (*free_configuration)(void **configuration);
> You can simplify memory management if caller (instead of callee) would
> be responsible for passing pointer to allocated memory (e.g. also from
> stack) with buffer size. Then API would not need any "free" functions.
Well, the api just "hide" all details.
>> + /* Return true if configuration valid */
>> + bool (*set_configuration)(const uint8_t *selected_configuration, const size_t configuration_size);
> If this function validates configuration, then "validate_configuration"
> is better name.
Yes. I simply use bluez apis names before.
>> +
>> +};
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
