[FFmpeg-devel] [PATCH 4/5] aptx: implement the aptX HD bluetooth codec

[Rostislav Pehlivanov]

On 9 January 2018 at 01:21, Aurelien Jacobs <aurel at gnuage.org> wrote:

> On Mon, Jan 08, 2018 at 12:38:19AM +0000, Rostislav Pehlivanov wrote:
> > On 7 January 2018 at 22:54, Aurelien Jacobs <aurel at gnuage.org> wrote:
> >
> > > On Sun, Jan 07, 2018 at 05:23:24PM +0000, Rostislav Pehlivanov wrote:
> > > > On 6 January 2018 at 16:48, Aurelien Jacobs <aurel at gnuage.org>
> wrote:
> > > >
> > > > > ---
> > > > >  Changelog               |   2 +-
> > > > >  configure               |   2 +
> > > > >  libavcodec/Makefile     |   2 +
> > > > >  libavcodec/allcodecs.c  |   1 +
> > > > >  libavcodec/aptx.c       | 352 ++++++++++++++++++++++++++++++
> > > > > ++++++++++++++----
> > > > >  libavcodec/avcodec.h    |   1 +
> > > > >  libavcodec/codec_desc.c |   7 +
> > > > >  7 files changed, 339 insertions(+), 28 deletions(-)
> > > >
> > > >
> > > > No, don't add a new codec ID for what is very obviously a profile.
> > > >
> > [...]
> > > Anyway, I do understand how I could use a profile instead of a new
> codec
> > > ID, but I really don't understand what advantage it would bring ?
> > >
> > > For a codec known with one name, but supporting a lot of different
> > > profiles with flags in the bitstream so that the decoder can adapt
> > > itself to any profile, that makes a lot of sense. But for aptX and
> > > aptX HD it doesn't make any sense to me.
> > >
> >
> > It makes sense - HD is just a flavor of aptx. You can't call this a brand
> > new codec - it just changes some tables and the way codewords are written
> > (1 function).
>
> Of course it does make sense to us, the few people who are touching
> codecs source code, that they are both flavors of the same code.
>

> > The only people who would call that brand new are in
> > marketing and they're wrong.
>
> Of course calling aptX and aptX HD different codecs is pretty much
> marketting bullshit.
>

> But that's not the point here.
> We are not talking about some internal implementation details.
> We are talking about ffmpeg's end user interface. So what we do
> has to make sense to end user. And end users "knows very well" that
> aptX and aptX HD are 2 different codecs (they are used for exemple
> to install 2 differents libs on android to support both).
>
>
They're not 2 different codecs, regardless of what is being said. The
similarities are unquestionable. You even said it yourself. Now you're just
bikeshedding because you've already done the patches one way and you don't
want to change them to something new and unfamiliar.



> > > I don't think it would make the code simpler.
> > > The decoder itself has no flag in the bitstream to adapt to the correct
> > > "profile".
> > >
> > And I think it would be confusing for end user. aptX HD is publicly know
> > > as a different codec than aptX. A user looking at the list of supported
> > > codec would probably conclude that aptX is supported but not aptX HD.
> > >
> > > Also, the closest thing I can think as a standard container for aptX
> > > is the bluetooth A2DP protocol. And in this protocol, aptX and aptX HD
> > > are differentiated with a different codec ID, just the same way they
> > > are differentiated from SBC or LDAC.
> > >
> > > So in the end, using different codec ID seems pretty natural, while
> > > using different profiles seems akward and doesn't seem to bring any
> > > advantage.
> > >
> > > Can you give any technical reason why think using profile would be
> > > better ?
> > >
> >
> > Yes, a big one - we don't bloat the already humongous API. You might say:
> > "Well, we have hundreds of codec IDs, what's one more?". If we had a new
> > codec ID for every flavor of every codec, we'd have thousands.
>
> So what ? It won't increase binary bloat unless we ever reach more
> than 2^32 codecs.
> And regarding public API bloat, the 2 options are:
>  1) define 2 codec ID consts
>  2) define 1 codec ID const and 2 profile consts
> Which one is bloating the public API more ?
>

The first one.



>
> > The profile systems allows us to discern between actually marketed
> variants
> > of one codec with the same name, for example AAC-LC with AAC-LTP.
>
> Do you really think that AAC-LTP is marketed as a unique codec to end
> users ? I've never stumbled on this.
>

I absolutely do not. Hence I don't see aptx HD being marketed as a unique
codec.



> Anyway, all this discussion is moot as Hendrik pointed out that profile
> can't be set outside of lavc to determine a decoder behavior.
>

What, based on a comment in lavc? Comments there describe the api but they
rarely define it. We're free to adjust them if need be and in this case,
since the codec profile may not be set, the API user is forced to deal with
the lack of such set. Hence, we can clarify that it may be set by lavf as
well as lavc as well as not being set at all. And the decoder may use it.
I maintain that adding a new codec ID for this is unacceptable.


You can keep refusing to change the patches and maintain that they're
separate codecs. However other people who see things the other way are also
free to take the patches and modify them if you don't want to.