Re: [PATCH v4 0/7] New API for Bluetooth A2DP codecs

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

 



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

[Index of Archives]     [Linux Audio Users]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux