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

[Aurelien Jacobs]

On Tue, Jan 09, 2018 at 02:21:02PM +0000, Rostislav Pehlivanov wrote:
> On 9 January 2018 at 14:07, Rostislav Pehlivanov <atomnuker at gmail.com>
> wrote:
> 
> >
> >
> > On 9 January 2018 at 09:00, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> >
> >> On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes <h.leppkes at gmail.com>
> >> wrote:
> >> > On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov
> >> > <atomnuker at gmail.com> wrote:
> >> >>
> >> >>> 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.
> >> >>
> >> >
> >> > We already have established methods to select codec sub-variants, we
> >> > don't need to invent a new one just because you feel like it.
> >> >
> >>
> >> On that note, we also have cases where the same codec has different
> >> codec ids based on popular known names.
> >> For example wmv3/vc1 - wmv3 is simple/main profile, vc1 is advanced
> >> profile, different codec ids, same implementation.
> >>
> >> Re-defining public API is what is unacceptable, it requires every
> >> caller of lavc to suddenly start handling one more field for no real
> >> reason.
> >>
> >
> > Then its a good thing I suggested something that doesn't involve having
> > every caller of lavc to handle another field.
> >
> >
> >
> >> Either use a separate codec ID if there is sufficient reason for that
> >> (mostly driven by external factors, if its handled as different codecs
> >> everywhere else, not only in marketing but actual (reference)
> >> implementations), or use a codec_tag if one is available (the codec id
> >> field from a2dp for example)
> >
> > I'd be fine with using codec tags, but not with codec IDs.
>
> Though for encoding using profiles would be best.

Well, here is an updated patch which uses codec tags for the decoder and
profile for the encoder.

I still maintain that this solution is worse than using separate
codec IDs.
It is worse for developper / maintainer as it adds a bit of compexity to
the code.
It is worse for user as it requires adding a mandatory parameter for
encoding to aptX HD:
  ffmpeg -i sample.wav -profile:a 1 sample.aptxhd
Without this -profile parameter, the encoding will just fail but user
will have to guess how to fix it.
And the reported stream is much less explicit:
  Stream #0:0: Audio: aptx ([36][0][215][0] / 0xD70024), 48000 Hz, 2 channels, s32p
vs.
  Stream #0:0: Audio: aptx_hd, 48000 Hz, 2 channels, s32p

So for the good of users and maintainers, I suggest to follow the
wmv3/vc1 example, that is to acknowledge that aptX and aptX HD are
commonly used as 2 separate codecs (search for
libaptX-1.0.0-rel-Android21-ARMv7A.so and
libaptXHD-1.0.0-rel-Android21-ARMv7A.so for example) and use the
original patch with 2 codec IDs.
But anyway, if there is a consensus for using a single codec ID, this
new patch can be applied.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-aptx-implement-the-aptX-HD-bluetooth-codec.patch
Type: text/x-diff
Size: 27152 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180114/f7dcd596/attachment.patch>