[FFmpeg-devel] [PATCH 1/2] aptx: implement the aptX bluetooth codec

[Rostislav Pehlivanov]

On 6 November 2017 at 00:30, Rostislav Pehlivanov <atomnuker at gmail.com>
wrote:

>
>
> On 5 November 2017 at 23:39, Aurelien Jacobs <aurel at gnuage.org> wrote:
>
>> +            *ptr++ = samples[RIGHT][i] >> 8;
>>
>
> How horrible, don't interleave the samples, leave them as planar.
> Change the output format in AVCodec and use
>
> AV_WN16(frame->data[<channel>][<sample>], samples[<channel>][<sample>] >>
> 8);
>
> To write the data. No point to convert to interleaved when the data's
> planar.
>
>
>>
>> +
>> +    *got_frame_ptr = 1;
>> +    return avpkt->size - len;
>>
>
> ?
> Decoders should return the number of bytes read from the packet (if
> convenient) or the packet size, not some random digit.
>
>
>> +}
>> +
>> +static int aptx_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>> +                             const AVFrame *frame, int *got_packet_ptr)
>> +{
>> +    AptXContext *s = avctx->priv_data;
>> +    int16_t *ptr = (int16_t *)frame->data[0];
>> +    int32_t samples[NB_CHANNELS][4];
>> +    int ret;
>> +
>> +    /* input must contain a multiple of 4 samples */
>> +    if (frame->nb_samples & 3 || frame->nb_samples == 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "Frame must have a multiple of 4
>> samples\n");
>> +        return 0;
>> +    }
>> +
>> +    if ((ret = ff_alloc_packet2(avctx, avpkt, frame->nb_samples, 0)) < 0)
>> +        return ret;
>> +
>> +    for (int pos = 0; pos < frame->nb_samples; pos += 4) {
>> +        for (int i = 0; i < 4; i++) {
>> +            /* convert 16 bits interleaved input to 24 bits planar
>> samples */
>> +            samples[LEFT][i]  = ptr[LEFT ] << 8;
>> +            samples[RIGHT][i] = ptr[RIGHT] << 8;
>> +            ptr += NB_CHANNELS;
>> +        }
>>
>
> Once again use planar sample fmt and then AV_RN16(&frame->data[<channel>][<sample_offset>])
> to read them.
>
>
>> +
>> +        aptx_encode_samples(s, samples, avpkt->data + pos);
>> +    }
>> +
>> +    *got_packet_ptr = 1;
>> +    return 0;
>> +}
>> +
>> +
>> +#if CONFIG_APTX_DECODER
>> +AVCodec ff_aptx_decoder = {
>> +    .name                  = "aptx",
>> +    .long_name             = NULL_IF_CONFIG_SMALL("aptX (Audio
>> Processing Technology for Bluetooth)"),
>> +    .type                  = AVMEDIA_TYPE_AUDIO,
>> +    .id                    = AV_CODEC_ID_APTX,
>> +    .priv_data_size        = sizeof(AptXContext),
>> +    .init                  = aptx_init,
>> +    .decode                = aptx_decode_frame,
>> +    .capabilities          = AV_CODEC_CAP_DR1,
>> +    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO,
>> 0},
>> +    .sample_fmts           = (const enum AVSampleFormat[]) {
>> AV_SAMPLE_FMT_S16,
>>
>
> Change to AV_SAMPLE_FMT_S16P
>
>
>> +
>>  AV_SAMPLE_FMT_NONE },
>> +};
>> +#endif
>> +
>> +#if CONFIG_APTX_ENCODER
>> +AVCodec ff_aptx_encoder = {
>> +    .name                  = "aptx",
>> +    .long_name             = NULL_IF_CONFIG_SMALL("aptX (Audio
>> Processing Technology for Bluetooth)"),
>> +    .type                  = AVMEDIA_TYPE_AUDIO,
>> +    .id                    = AV_CODEC_ID_APTX,
>> +    .priv_data_size        = sizeof(AptXContext),
>> +    .init                  = aptx_init,
>> +    .encode2               = aptx_encode_frame,
>> +    .capabilities          = AV_CODEC_CAP_VARIABLE_FRAME_SIZE,
>> +    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO,
>> 0},
>> +    .sample_fmts           = (const enum AVSampleFormat[]) {
>> AV_SAMPLE_FMT_S16,
>>
>
> And here to AV_SAMPLE_FMT_S16P
>
>

Actually no, the sample format should be AV_SAMPLE_FMT_S32P

The code internally seems to use 24bit precision (for some retarded
reason), so NIHing format conversion to 16 bits is pointless as you lose
precision.
Just use a 32 bit sample format and shift down on the encoder side by 8
bits to get normalized 24 bit samples and shift up on the decoder side by 8
to get 32 bit samples.

It would have been better if the encoder and decoder were float tbh but oh
well, the people writing the code were paid to make it run on uselessly low
speed devices.