Hi! On Tuesday 15 January 2019 11:29:24 Luiz Augusto von Dentz wrote: > Hi Pali, > > On Tue, Jan 15, 2019 at 10:34 AM Pali Rohár <pali.rohar@xxxxxxxxx> wrote: > > > > On Tuesday 15 January 2019 10:06:14 Luiz Augusto von Dentz wrote: > > > Hi Pali, > > > > > > On Mon, Jan 14, 2019 at 8:24 PM Pali Rohár <pali.rohar@xxxxxxxxx> wrote: > > > > > > > > This is 4th version of my patch series for modular A2DP codec API and > > > > aptX support. > > > > > > > > This patch series provides new modular API for Bluetooth A2DP codecs, > > > > clean up module-bluez5-device and bluez5-util to be codec independent > > > > and convert SBC codec into this new API for A2DP codecs. Also it adds > > > > support for aptX, aptX HD and FastStream A2DP codecs. > > > > > > It is a bit better since you are introducing a table to add codecs, > > > though what I suggested was having the codecs as separated modules > > > which then register with bluetooth modules to make their codec > > > available. Anyway Im fine with just having a table, at least it is not > > > using #ifdef everywhere. > > > > I think that table is better then having to call module init function > > for each codec, so codec would register into pulseaudio. > > > > With table there is just one ifdef -- for including codec into table, > > which is (at least for me) easier to use or extend. > > Fair enough, I just thought that having as a plugin may make sense to > pass parameters to the codecs, though we don't have anything like that > currently so we can probably leave it for later if we find it really > necessary. Ok. > > > Speaking about aptX, there seems to be 2 libs supporting it, > > > libavcodec and your openaptx, we should probably settle in just one, > > > probably the one that easier to find packages. > > > > In v3 (and therefore also in v4) I updated automake and autoconf code > > for openaptx so there is no hardcoded aptx path. This should work as > > expected. > > > > Anyway, more people are still asking for pkg-config file for openaptx. > > So if you really think that it would help something, I can include code > > into libopenaptx which generates simple pc file at install step. But I'm > > still a bit sceptical that such pc file is useful for simple library > > which do not have any compile flags and no specific link flags... > > > > So is not current autoconf code for aptX in v4 enough? > > > > About libavcodec detection of aptX. It is not easy. You need runtime > > code which ask libavcodec if aptX is support or not. So you cannot write > > just compile check macro in autoconf which returns true/false (aptX is > > supported or not). Runtime code means no possibility to cross compile, > > plus needs to have this runtime code also in pulseaudio to verify that > > configuration detected at compile time matches configuration at > > pulseaudio runtime. > > Alright, well perhaps instead of building as external dependency we > could have the openaptx code include in the PA source tree and > compiled as local library as we probably don't expect any other users, > or do we? Some people already asked me about libopenaptx, so seems that some users of this library there are... > About libavcodec, or gstreamer, the codecs are detected at runtime > thus I guess having them as plugins makes more sense, so I guess it is > not a bad ideia to register the codecs at runtime after all. Anyway we > need to reuse part of the code for D-Bus handling, etc, since those > frameworks normally only deal with the codec payload not the > signalling. Yes, I think it is more complicated to implement "real" module runtime loading properly. And do we need it at all? I think that my current implementation is "feature" enough and is simple. ===== Currently I would like to see some review of these patches. The only missing part is implementation of codec switching, everything else is implemented and a little bit tested. So some other testing would be useful too. -- Pali Rohár pali.rohar@xxxxxxxxx
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ pulseaudio-discuss mailing list pulseaudio-discuss@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
