diff mbox

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

Message ID 20180106164808.26162-5-aurel@gnuage.org
State Superseded
Headers show

Commit Message

Aurelien Jacobs Jan. 6, 2018, 4:48 p.m. UTC
---
 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(-)

Comments

Rostislav Pehlivanov Jan. 7, 2018, 5:23 p.m. UTC | #1
On 6 January 2018 at 16:48, Aurelien Jacobs <aurel@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(-)
>
> diff --git a/Changelog b/Changelog
> index 3d966c202b..9349bf1e8d 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -11,7 +11,7 @@ version <next>:
>  - TiVo ty/ty+ demuxer
>  - Intel QSV-accelerated MJPEG encoding
>  - PCE support for extended channel layouts in the AAC encoder
> -- native aptX encoder and decoder
> +- native aptX and aptX HD encoder and decoder
>  - Raw aptX muxer and demuxer
>  - NVIDIA NVDEC-accelerated H.264, HEVC, MPEG-1/2/4, VC1, VP8/9 hwaccel
> decoding
>  - Intel QSV-accelerated overlay filter
> diff --git a/configure b/configure
> index 1d2fffa132..c496346a06 100755
> --- a/configure
> +++ b/configure
> @@ -2459,6 +2459,8 @@ apng_encoder_deps="zlib"
>  apng_encoder_select="llvidencdsp"
>  aptx_decoder_select="audio_frame_queue"
>  aptx_encoder_select="audio_frame_queue"
> +aptx_hd_decoder_select="audio_frame_queue"
> +aptx_hd_encoder_select="audio_frame_queue"
>  asv1_decoder_select="blockdsp bswapdsp idctdsp"
>  asv1_encoder_select="bswapdsp fdctdsp pixblockdsp"
>  asv2_decoder_select="blockdsp bswapdsp idctdsp"
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index cfacd6b70c..a9ecf7ea5e 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -190,6 +190,8 @@ OBJS-$(CONFIG_ANSI_DECODER)            += ansi.o
> cga_data.o
>  OBJS-$(CONFIG_APE_DECODER)             += apedec.o
>  OBJS-$(CONFIG_APTX_DECODER)            += aptx.o
>  OBJS-$(CONFIG_APTX_ENCODER)            += aptx.o
> +OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
> +OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index ed1e7ab06e..93d31f8688 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -333,6 +333,7 @@ static void register_all(void)
>      REGISTER_DECODER(AMRWB,             amrwb);
>      REGISTER_DECODER(APE,               ape);
>      REGISTER_ENCDEC (APTX,              aptx);
> +    REGISTER_ENCDEC (APTX_HD,           aptx_hd);
>      REGISTER_DECODER(ATRAC1,            atrac1);
>      REGISTER_DECODER(ATRAC3,            atrac3);
>      REGISTER_DECODER(ATRAC3AL,          atrac3al);
> diff --git a/libavcodec/aptx.c b/libavcodec/aptx.c
> index 4173402d03..6c0f3d35a9 100644
> --- a/libavcodec/aptx.c
> +++ b/libavcodec/aptx.c
> @@ -89,6 +89,8 @@ typedef struct {
>  } Channel;
>
>  typedef struct {
> +    int hd;
> +    int block_size;
>      int32_t sync_idx;
>      Channel channels[NB_CHANNELS];
>      AudioFrameQueue afq;
> @@ -182,6 +184,205 @@ static const int16_t quantize_factor_select_offset_HF[5]
> = {
>      0, -8, 33, 95, 262,
>  };
>
> +
> +static const int32_t hd_quantize_intervals_LF[257] = {
> +      -2436,    2436,    7308,   12180,   17054,   21930,   26806,
>  31686,
> +      36566,   41450,   46338,   51230,   56124,   61024,   65928,
>  70836,
> +      75750,   80670,   85598,   90530,   95470,  100418,  105372,
> 110336,
> +     115308,  120288,  125278,  130276,  135286,  140304,  145334,
> 150374,
> +     155426,  160490,  165566,  170654,  175756,  180870,  185998,
> 191138,
> +     196294,  201466,  206650,  211850,  217068,  222300,  227548,
> 232814,
> +     238096,  243396,  248714,  254050,  259406,  264778,  270172,
> 275584,
> +     281018,  286470,  291944,  297440,  302956,  308496,  314056,
> 319640,
> +     325248,  330878,  336532,  342212,  347916,  353644,  359398,
> 365178,
> +     370986,  376820,  382680,  388568,  394486,  400430,  406404,
> 412408,
> +     418442,  424506,  430600,  436726,  442884,  449074,  455298,
> 461554,
> +     467844,  474168,  480528,  486922,  493354,  499820,  506324,
> 512866,
> +     519446,  526064,  532722,  539420,  546160,  552940,  559760,
> 566624,
> +     573532,  580482,  587478,  594520,  601606,  608740,  615920,
> 623148,
> +     630426,  637754,  645132,  652560,  660042,  667576,  675164,
> 682808,
> +     690506,  698262,  706074,  713946,  721876,  729868,  737920,
> 746036,
> +     754216,  762460,  770770,  779148,  787594,  796108,  804694,
> 813354,
> +     822086,  830892,  839774,  848736,  857776,  866896,  876100,
> 885386,
> +     894758,  904218,  913766,  923406,  933138,  942964,  952886,
> 962908,
> +     973030,  983254,  993582, 1004020, 1014566, 1025224, 1035996,
> 1046886,
> +    1057894, 1069026, 1080284, 1091670, 1103186, 1114838, 1126628,
> 1138558,
> +    1150634, 1162858, 1175236, 1187768, 1200462, 1213320, 1226346,
> 1239548,
> +    1252928, 1266490, 1280242, 1294188, 1308334, 1322688, 1337252,
> 1352034,
> +    1367044, 1382284, 1397766, 1413494, 1429478, 1445728, 1462252,
> 1479058,
> +    1496158, 1513562, 1531280, 1549326, 1567710, 1586446, 1605550,
> 1625034,
> +    1644914, 1665208, 1685932, 1707108, 1728754, 1750890, 1773542,
> 1796732,
> +    1820488, 1844840, 1869816, 1895452, 1921780, 1948842, 1976680,
> 2005338,
> +    2034868, 2065322, 2096766, 2129260, 2162880, 2197708, 2233832,
> 2271352,
> +    2310384, 2351050, 2393498, 2437886, 2484404, 2533262, 2584710,
> 2639036,
> +    2696578, 2757738, 2822998, 2892940, 2968278, 3049896, 3138912,
> 3236760,
> +    3345312, 3467068, 3605434, 3765154, 3952904, 4177962, 4452178,
> 4787134,
> +    5187290, 5647128, 6159120, 6720518, 7332904, 8000032, 8726664,
> 9518152,
> +    10380372,
> +};
> +static const int32_t hd_invert_quantize_dither_factors_LF[257] = {
> +      2436,   2436,   2436,   2436,   2438,   2438,   2438,   2440,
> +      2442,   2442,   2444,   2446,   2448,   2450,   2454,   2456,
> +      2458,   2462,   2464,   2468,   2472,   2476,   2480,   2484,
> +      2488,   2492,   2498,   2502,   2506,   2512,   2518,   2524,
> +      2528,   2534,   2540,   2548,   2554,   2560,   2568,   2574,
> +      2582,   2588,   2596,   2604,   2612,   2620,   2628,   2636,
> +      2646,   2654,   2664,   2672,   2682,   2692,   2702,   2712,
> +      2722,   2732,   2742,   2752,   2764,   2774,   2786,   2798,
> +      2810,   2822,   2834,   2846,   2858,   2870,   2884,   2896,
> +      2910,   2924,   2938,   2952,   2966,   2980,   2994,   3010,
> +      3024,   3040,   3056,   3070,   3086,   3104,   3120,   3136,
> +      3154,   3170,   3188,   3206,   3224,   3242,   3262,   3280,
> +      3300,   3320,   3338,   3360,   3380,   3400,   3422,   3442,
> +      3464,   3486,   3508,   3532,   3554,   3578,   3602,   3626,
> +      3652,   3676,   3702,   3728,   3754,   3780,   3808,   3836,
> +      3864,   3892,   3920,   3950,   3980,   4010,   4042,   4074,
> +      4106,   4138,   4172,   4206,   4240,   4276,   4312,   4348,
> +      4384,   4422,   4460,   4500,   4540,   4580,   4622,   4664,
> +      4708,   4752,   4796,   4842,   4890,   4938,   4986,   5036,
> +      5086,   5138,   5192,   5246,   5300,   5358,   5416,   5474,
> +      5534,   5596,   5660,   5726,   5792,   5860,   5930,   6002,
> +      6074,   6150,   6226,   6306,   6388,   6470,   6556,   6644,
> +      6736,   6828,   6924,   7022,   7124,   7228,   7336,   7448,
> +      7562,   7680,   7802,   7928,   8058,   8192,   8332,   8476,
> +      8624,   8780,   8940,   9106,   9278,   9458,   9644,   9840,
> +     10042,  10252,  10472,  10702,  10942,  11194,  11458,  11734,
> +     12024,  12328,  12648,  12986,  13342,  13720,  14118,  14540,
> +     14990,  15466,  15976,  16520,  17102,  17726,  18398,  19124,
> +     19908,  20760,  21688,  22702,  23816,  25044,  26404,  27922,
> +     29622,  31540,  33720,  36222,  39116,  42502,  46514,  51334,
> +     57218,  64536,  73830,  85890, 101860, 123198, 151020, 183936,
> +    216220, 243618, 268374, 293022, 319362, 347768, 378864, 412626,
> 449596,
> +};
> +static const int32_t hd_quantize_dither_factors_LF[256] = {
> +       0,    0,    0,    1,    0,    0,    1,    1,
> +       0,    1,    1,    1,    1,    1,    1,    1,
> +       1,    1,    1,    1,    1,    1,    1,    1,
> +       1,    2,    1,    1,    2,    2,    2,    1,
> +       2,    2,    2,    2,    2,    2,    2,    2,
> +       2,    2,    2,    2,    2,    2,    2,    3,
> +       2,    3,    2,    3,    3,    3,    3,    3,
> +       3,    3,    3,    3,    3,    3,    3,    3,
> +       3,    3,    3,    3,    3,    4,    3,    4,
> +       4,    4,    4,    4,    4,    4,    4,    4,
> +       4,    4,    4,    4,    5,    4,    4,    5,
> +       4,    5,    5,    5,    5,    5,    5,    5,
> +       5,    5,    6,    5,    5,    6,    5,    6,
> +       6,    6,    6,    6,    6,    6,    6,    7,
> +       6,    7,    7,    7,    7,    7,    7,    7,
> +       7,    7,    8,    8,    8,    8,    8,    8,
> +       8,    9,    9,    9,    9,    9,    9,    9,
> +      10,   10,   10,   10,   10,   11,   11,   11,
> +      11,   11,   12,   12,   12,   12,   13,   13,
> +      13,   14,   14,   14,   15,   15,   15,   15,
> +      16,   16,   17,   17,   17,   18,   18,   18,
> +      19,   19,   20,   21,   21,   22,   22,   23,
> +      23,   24,   25,   26,   26,   27,   28,   29,
> +      30,   31,   32,   33,   34,   35,   36,   37,
> +      39,   40,   42,   43,   45,   47,   49,   51,
> +      53,   55,   58,   60,   63,   66,   69,   73,
> +      76,   80,   85,   89,   95,  100,  106,  113,
> +     119,  128,  136,  146,  156,  168,  182,  196,
> +     213,  232,  254,  279,  307,  340,  380,  425,
> +     480,  545,  626,  724,  847, 1003, 1205, 1471,
> +    1830, 2324, 3015, 3993, 5335, 6956, 8229, 8071,
> +    6850, 6189, 6162, 6585, 7102, 7774, 8441, 9243,
> +};
> +static const int16_t hd_quantize_factor_select_offset_LF[257] = {
> +      0, -22, -21, -21, -20, -20, -19, -19,
> +    -18, -18, -17, -17, -16, -16, -15, -14,
> +    -14, -13, -13, -12, -12, -11, -11, -10,
> +    -10,  -9,  -9,  -8,  -7,  -7,  -6,  -6,
> +     -5,  -5,  -4,  -4,  -3,  -3,  -2,  -1,
> +     -1,   0,   0,   1,   1,   2,   2,   3,
> +      4,   4,   5,   5,   6,   6,   7,   8,
> +      8,   9,   9,  10,  11,  11,  12,  12,
> +     13,  14,  14,  15,  15,  16,  17,  17,
> +     18,  19,  19,  20,  20,  21,  22,  22,
> +     23,  24,  24,  25,  26,  26,  27,  28,
> +     28,  29,  30,  30,  31,  32,  33,  33,
> +     34,  35,  35,  36,  37,  38,  38,  39,
> +     40,  41,  41,  42,  43,  44,  44,  45,
> +     46,  47,  48,  48,  49,  50,  51,  52,
> +     52,  53,  54,  55,  56,  57,  58,  58,
> +     59,  60,  61,  62,  63,  64,  65,  66,
> +     67,  68,  69,  69,  70,  71,  72,  73,
> +     74,  75,  77,  78,  79,  80,  81,  82,
> +     83,  84,  85,  86,  87,  89,  90,  91,
> +     92,  93,  94,  96,  97,  98,  99, 101,
> +    102, 103, 105, 106, 107, 109, 110, 112,
> +    113, 115, 116, 118, 119, 121, 122, 124,
> +    125, 127, 129, 130, 132, 134, 136, 137,
> +    139, 141, 143, 145, 147, 149, 151, 153,
> +    155, 158, 160, 162, 164, 167, 169, 172,
> +    174, 177, 180, 182, 185, 188, 191, 194,
> +    197, 201, 204, 208, 211, 215, 219, 223,
> +    227, 232, 236, 241, 246, 251, 257, 263,
> +    269, 275, 283, 290, 298, 307, 317, 327,
> +    339, 352, 367, 384, 404, 429, 458, 494,
> +    522, 522, 522, 522, 522, 522, 522, 522, 522,
> +};
> +
> +
> +static const int32_t hd_quantize_intervals_MLF[33] = {
> +      -21236,   21236,   63830,  106798,  150386,  194832,  240376,
> 287258,
> +      335726,  386034,  438460,  493308,  550924,  611696,  676082,
> 744626,
> +      817986,  896968,  982580, 1076118, 1179278, 1294344, 1424504,
> 1574386,
> +     1751090, 1966260, 2240868, 2617662, 3196432, 4176450, 5658260,
> 7671068,
> +    10380372,
> +};
> +static const int32_t hd_invert_quantize_dither_factors_MLF[33] = {
> +    21236,  21236,  21360,  21608,  21978,  22468,  23076,   23806,
> +    24660,  25648,  26778,  28070,  29544,  31228,  33158,   35386,
> +    37974,  41008,  44606,  48934,  54226,  60840,  69320,   80564,
> +    96140, 119032, 155576, 221218, 357552, 622468, 859344, 1153464,
> 1555840,
> +};
> +static const int32_t hd_quantize_dither_factors_MLF[32] = {
> +       0,   31,    62,    93,   123,   152,   183,    214,
> +     247,  283,   323,   369,   421,   483,   557,    647,
> +     759,  900,  1082,  1323,  1654,  2120,  2811,   3894,
> +    5723, 9136, 16411, 34084, 66229, 59219, 73530, 100594,
> +};
> +static const int16_t hd_quantize_factor_select_offset_MLF[33] = {
> +      0, -21, -16, -12,  -7,  -2,   3,   8,
> +     13,  19,  24,  30,  36,  43,  50,  57,
> +     65,  74,  83,  93, 104, 117, 131, 147,
> +    166, 189, 219, 259, 322, 427, 521, 521, 521,
> +};
> +
> +
> +static const int32_t hd_quantize_intervals_MHF[9] = {
> +    -95044, 95044, 295844, 528780, 821332, 1226438, 1890540, 3344850,
> 6450664,
> +};
> +static const int32_t hd_invert_quantize_dither_factors_MHF[9] = {
> +    95044, 95044, 105754, 127180, 165372, 39736, 424366, 1029946, 2075866,
> +};
> +static const int32_t hd_quantize_dither_factors_MHF[8] = {
> +    0, 2678, 5357, 9548, -31409, 96158, 151395, 261480,
> +};
> +static const int16_t hd_quantize_factor_select_offset_MHF[9] = {
> +    0, -17, 5, 30, 62, 105, 177, 334, 518,
> +};
> +
> +
> +static const int32_t hd_quantize_intervals_HF[17] = {
> +     -45754,   45754,  138496,  234896,  337336,  448310,  570738,
> 708380,
> +     866534, 1053262, 1281958, 1577438, 1993050, 2665984, 3900982,
> 5902844,
> +    8897462,
> +};
> +static const int32_t hd_invert_quantize_dither_factors_HF[17] = {
> +    45754,  45754,  46988,  49412,  53026,  57950,  64478,   73164,
> +    84988, 101740, 126958, 168522, 247092, 425842, 809154, 1192708,
> 1801910,
> +};
> +static const int32_t hd_quantize_dither_factors_HF[16] = {
> +       0,  309,   606,   904,  1231,  1632,  2172,   2956,
> +    4188, 6305, 10391, 19643, 44688, 95828, 95889, 152301,
> +};
> +static const int16_t hd_quantize_factor_select_offset_HF[17] = {
> +     0, -18,  -8,   2,  13,  25,  38,  53,
> +    70,  90, 115, 147, 192, 264, 398, 521, 521,
> +};
> +
>  typedef const struct {
>      const int32_t *quantize_intervals;
>      const int32_t *invert_quantize_dither_factors;
> @@ -192,7 +393,8 @@ typedef const struct {
>      int32_t prediction_order;
>  } ConstTables;
>
> -static ConstTables tables[NB_SUBBANDS] = {
> +static ConstTables tables[2][NB_SUBBANDS] = {
> +{
>      [LF]  = { quantize_intervals_LF,
>                invert_quantize_dither_factors_LF,
>                quantize_dither_factors_LF,
> @@ -217,6 +419,33 @@ static ConstTables tables[NB_SUBBANDS] = {
>                quantize_factor_select_offset_HF,
>                FF_ARRAY_ELEMS(quantize_intervals_HF),
>                0x15FF, 12 },
> +},
> +{
> +    [LF]  = { hd_quantize_intervals_LF,
> +              hd_invert_quantize_dither_factors_LF,
> +              hd_quantize_dither_factors_LF,
> +              hd_quantize_factor_select_offset_LF,
> +              FF_ARRAY_ELEMS(hd_quantize_intervals_LF),
> +              0x11FF, 24 },
> +    [MLF] = { hd_quantize_intervals_MLF,
> +              hd_invert_quantize_dither_factors_MLF,
> +              hd_quantize_dither_factors_MLF,
> +              hd_quantize_factor_select_offset_MLF,
> +              FF_ARRAY_ELEMS(hd_quantize_intervals_MLF),
> +              0x14FF, 12 },
> +    [MHF] = { hd_quantize_intervals_MHF,
> +              hd_invert_quantize_dither_factors_MHF,
> +              hd_quantize_dither_factors_MHF,
> +              hd_quantize_factor_select_offset_MHF,
> +              FF_ARRAY_ELEMS(hd_quantize_intervals_MHF),
> +              0x16FF, 6 },
> +    [HF]  = { hd_quantize_intervals_HF,
> +              hd_invert_quantize_dither_factors_HF,
> +              hd_quantize_dither_factors_HF,
> +              hd_quantize_factor_select_offset_HF,
> +              FF_ARRAY_ELEMS(hd_quantize_intervals_HF),
> +              0x15FF, 12 },
> +}
>  };
>
>  static const int16_t quantization_factors[32] = {
> @@ -494,7 +723,7 @@ static void aptx_quantize_difference(Quantize
> *quantize,
>      quantize->quantized_sample_parity_change = parity_change    ^ inv;
>  }
>
> -static void aptx_encode_channel(Channel *channel, int32_t samples[4])
> +static void aptx_encode_channel(Channel *channel, int32_t samples[4], int
> hd)
>  {
>      int32_t subband_samples[4];
>      int subband;
> @@ -505,7 +734,7 @@ static void aptx_encode_channel(Channel *channel,
> int32_t samples[4])
>          aptx_quantize_difference(&channel->quantize[subband], diff,
>                                   channel->dither[subband],
>                                   channel->invert_quantize[
> subband].quantization_factor,
> -                                 &tables[subband]);
> +                                 &tables[hd][subband]);
>      }
>  }
>
> @@ -616,7 +845,7 @@ static void aptx_process_subband(InvertQuantize
> *invert_quantize,
>                                tables->prediction_order);
>  }
>
> -static void aptx_invert_quantize_and_prediction(Channel *channel)
> +static void aptx_invert_quantize_and_prediction(Channel *channel, int hd)
>  {
>      int subband;
>      for (subband = 0; subband < NB_SUBBANDS; subband++)
> @@ -624,7 +853,7 @@ static void aptx_invert_quantize_and_prediction(Channel
> *channel)
>                               &channel->prediction[subband],
>                               channel->quantize[subband].quantized_sample,
>                               channel->dither[subband],
> -                             &tables[subband]);
> +                             &tables[hd][subband]);
>  }
>
>  static int32_t aptx_quantized_parity(Channel *channel)
> @@ -678,6 +907,15 @@ static uint16_t aptx_pack_codeword(Channel *channel)
>           | (((channel->quantize[0].quantized_sample & 0x7F)         )
> <<  0);
>  }
>
> +static uint32_t aptxhd_pack_codeword(Channel *channel)
> +{
> +    int32_t parity = aptx_quantized_parity(channel);
> +    return (((channel->quantize[3].quantized_sample & 0x01E) | parity)
> << 19)
> +         | (((channel->quantize[2].quantized_sample & 0x00F)         )
> << 15)
> +         | (((channel->quantize[1].quantized_sample & 0x03F)         )
> <<  9)
> +         | (((channel->quantize[0].quantized_sample & 0x1FF)         )
> <<  0);
> +}
> +
>  static void aptx_unpack_codeword(Channel *channel, uint16_t codeword)
>  {
>      channel->quantize[0].quantized_sample = sign_extend(codeword >>  0,
> 7);
> @@ -688,35 +926,53 @@ static void aptx_unpack_codeword(Channel *channel,
> uint16_t codeword)
>                                            | aptx_quantized_parity(channel)
> ;
>  }
>
> +static void aptxhd_unpack_codeword(Channel *channel, uint32_t codeword)
> +{
> +    channel->quantize[0].quantized_sample = sign_extend(codeword >>  0,
> 9);
> +    channel->quantize[1].quantized_sample = sign_extend(codeword >>  9,
> 6);
> +    channel->quantize[2].quantized_sample = sign_extend(codeword >> 15,
> 4);
> +    channel->quantize[3].quantized_sample = sign_extend(codeword >> 19,
> 5);
> +    channel->quantize[3].quantized_sample = (channel->quantize[3].quantized_sample
> & ~1)
> +                                          | aptx_quantized_parity(channel)
> ;
> +}
> +
>  static void aptx_encode_samples(AptXContext *ctx,
>                                  int32_t samples[NB_CHANNELS][4],
> -                                uint8_t output[2*NB_CHANNELS])
> +                                uint8_t *output)
>  {
>      int channel;
>      for (channel = 0; channel < NB_CHANNELS; channel++)
> -        aptx_encode_channel(&ctx->channels[channel], samples[channel]);
> +        aptx_encode_channel(&ctx->channels[channel], samples[channel],
> ctx->hd);
>
>      aptx_insert_sync(ctx->channels, &ctx->sync_idx);
>
>      for (channel = 0; channel < NB_CHANNELS; channel++) {
> -        aptx_invert_quantize_and_prediction(&ctx->channels[channel]);
> -        AV_WB16(output + 2*channel, aptx_pack_codeword(&ctx->
> channels[channel]));
> +        aptx_invert_quantize_and_prediction(&ctx->channels[channel],
> ctx->hd);
> +        if (ctx->hd)
> +            AV_WB24(output + 3*channel,
> +                    aptxhd_pack_codeword(&ctx->channels[channel]));
> +        else
> +            AV_WB16(output + 2*channel,
> +                    aptx_pack_codeword(&ctx->channels[channel]));
>      }
>  }
>
>  static int aptx_decode_samples(AptXContext *ctx,
> -                                const uint8_t input[2*NB_CHANNELS],
> +                                const uint8_t *input,
>                                  int32_t samples[NB_CHANNELS][4])
>  {
>      int channel, ret;
>
>      for (channel = 0; channel < NB_CHANNELS; channel++) {
> -        uint16_t codeword;
>          aptx_generate_dither(&ctx->channels[channel]);
>
> -        codeword = AV_RB16(input + 2*channel);
> -        aptx_unpack_codeword(&ctx->channels[channel], codeword);
> -        aptx_invert_quantize_and_prediction(&ctx->channels[channel]);
> +        if (ctx->hd)
> +            aptxhd_unpack_codeword(&ctx->channels[channel],
> +                                   AV_RB24(input + 3*channel));
> +        else
> +            aptx_unpack_codeword(&ctx->channels[channel],
> +                                 AV_RB16(input + 2*channel));
> +        aptx_invert_quantize_and_prediction(&ctx->channels[channel],
> ctx->hd);
>      }
>
>      ret = aptx_check_parity(ctx->channels, &ctx->sync_idx);
> @@ -733,11 +989,15 @@ static av_cold int aptx_init(AVCodecContext *avctx)
>      AptXContext *s = avctx->priv_data;
>      int chan, subband;
>
> +    s->hd = avctx->codec->id == AV_CODEC_ID_APTX_HD;
> +    s->block_size = s->hd ? 6 : 4;
> +
>      if (avctx->frame_size == 0)
> -        avctx->frame_size = 1024;
> +        avctx->frame_size = 256 * s->block_size;
>
> -    if (avctx->frame_size & 3) {
> -        av_log(avctx, AV_LOG_ERROR, "Frame size must be a multiple of 4
> samples\n");
> +    if (avctx->frame_size % s->block_size) {
> +        av_log(avctx, AV_LOG_ERROR,
> +               "Frame size must be a multiple of %d samples\n",
> s->block_size);
>          return AVERROR(EINVAL);
>      }
>
> @@ -759,9 +1019,9 @@ static int aptx_decode_frame(AVCodecContext *avctx,
> void *data,
>  {
>      AptXContext *s = avctx->priv_data;
>      AVFrame *frame = data;
> -    int pos, channel, sample, ret;
> +    int pos, opos, channel, sample, ret;
>
> -    if (avpkt->size < 4) {
> +    if (avpkt->size < s->block_size) {
>          av_log(avctx, AV_LOG_ERROR, "Packet is too small\n");
>          return AVERROR_INVALIDDATA;
>      }
> @@ -769,11 +1029,11 @@ static int aptx_decode_frame(AVCodecContext
> *avctx, void *data,
>      /* get output buffer */
>      frame->channels = NB_CHANNELS;
>      frame->format = AV_SAMPLE_FMT_S32P;
> -    frame->nb_samples = avpkt->size & ~3;
> +    frame->nb_samples = 4 * avpkt->size / s->block_size;
>      if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
>          return ret;
>
> -    for (pos = 0; pos < frame->nb_samples; pos += 4) {
> +    for (pos = 0, opos = 0; opos < frame->nb_samples; pos +=
> s->block_size, opos += 4) {
>          int32_t samples[NB_CHANNELS][4];
>
>          if (aptx_decode_samples(s, &avpkt->data[pos], samples)) {
> @@ -783,32 +1043,33 @@ static int aptx_decode_frame(AVCodecContext
> *avctx, void *data,
>
>          for (channel = 0; channel < NB_CHANNELS; channel++)
>              for (sample = 0; sample < 4; sample++)
> -                AV_WN32A(&frame->data[channel][4*(sample+pos)],
> +                AV_WN32A(&frame->data[channel][4*(opos+sample)],
>                           samples[channel][sample] << 8);
>      }
>
>      *got_frame_ptr = 1;
> -    return frame->nb_samples;
> +    return s->block_size * frame->nb_samples / 4;
>  }
>
>  static int aptx_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>                               const AVFrame *frame, int *got_packet_ptr)
>  {
>      AptXContext *s = avctx->priv_data;
> -    int pos, channel, sample, ret;
> +    int pos, ipos, channel, sample, output_size, ret;
>
>      if ((ret = ff_af_queue_add(&s->afq, frame)) < 0)
>          return ret;
>
> -    if ((ret = ff_alloc_packet2(avctx, avpkt, frame->nb_samples, 0)) < 0)
> +    output_size = s->block_size * frame->nb_samples/4;
> +    if ((ret = ff_alloc_packet2(avctx, avpkt, output_size, 0)) < 0)
>          return ret;
>
> -    for (pos = 0; pos < frame->nb_samples; pos += 4) {
> +    for (pos = 0, ipos = 0; pos < output_size; pos += s->block_size, ipos
> += 4) {
>          int32_t samples[NB_CHANNELS][4];
>
>          for (channel = 0; channel < NB_CHANNELS; channel++)
>              for (sample = 0; sample < 4; sample++)
> -                samples[channel][sample] = (int32_t)AV_RN32A(&frame->data[channel][4*(sample+pos)])
> >> 8;
> +                samples[channel][sample] = (int32_t)AV_RN32A(&frame->
> data[channel][4*(ipos+sample)]) >> 8;
>
>          aptx_encode_samples(s, samples, avpkt->data + pos);
>      }
> @@ -844,6 +1105,24 @@ AVCodec ff_aptx_decoder = {
>  };
>  #endif
>
> +#if CONFIG_APTX_HD_DECODER
> +AVCodec ff_aptx_hd_decoder = {
> +    .name                  = "aptx_hd",
> +    .long_name             = NULL_IF_CONFIG_SMALL("aptX HD (Audio
> Processing Technology for Bluetooth)"),
> +    .type                  = AVMEDIA_TYPE_AUDIO,
> +    .id                    = AV_CODEC_ID_APTX_HD,
> +    .priv_data_size        = sizeof(AptXContext),
> +    .init                  = aptx_init,
> +    .decode                = aptx_decode_frame,
> +    .close                 = aptx_close,
> +    .capabilities          = AV_CODEC_CAP_DR1,
> +    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE,
> +    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO, 0},
> +    .sample_fmts           = (const enum AVSampleFormat[]) {
> AV_SAMPLE_FMT_S32P,
> +
>  AV_SAMPLE_FMT_NONE },
> +};
> +#endif
> +
>  #if CONFIG_APTX_ENCODER
>  AVCodec ff_aptx_encoder = {
>      .name                  = "aptx",
> @@ -862,3 +1141,22 @@ AVCodec ff_aptx_encoder = {
>      .supported_samplerates = (const int[]) {8000, 16000, 24000, 32000,
> 44100, 48000, 0},
>  };
>  #endif
> +
> +#if CONFIG_APTX_HD_ENCODER
> +AVCodec ff_aptx_hd_encoder = {
> +    .name                  = "aptx_hd",
> +    .long_name             = NULL_IF_CONFIG_SMALL("aptX HD (Audio
> Processing Technology for Bluetooth)"),
> +    .type                  = AVMEDIA_TYPE_AUDIO,
> +    .id                    = AV_CODEC_ID_APTX_HD,
> +    .priv_data_size        = sizeof(AptXContext),
> +    .init                  = aptx_init,
> +    .encode2               = aptx_encode_frame,
> +    .close                 = aptx_close,
> +    .capabilities          = AV_CODEC_CAP_SMALL_LAST_FRAME,
> +    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE,
> +    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO, 0},
> +    .sample_fmts           = (const enum AVSampleFormat[]) {
> AV_SAMPLE_FMT_S32P,
> +
>  AV_SAMPLE_FMT_NONE },
> +    .supported_samplerates = (const int[]) {8000, 16000, 24000, 32000,
> 44100, 48000, 0},
> +};
> +#endif
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index c13deb599f..95d164abc1 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -634,6 +634,7 @@ enum AVCodecID {
>      AV_CODEC_ID_ATRAC3PAL,
>      AV_CODEC_ID_DOLBY_E,
>      AV_CODEC_ID_APTX,
> +    AV_CODEC_ID_APTX_HD,
>
>      /* subtitle codecs */
>      AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID
> pointing at the start of subtitle codecs.
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index c3688de1d6..ca18bb2b67 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -2866,6 +2866,13 @@ static const AVCodecDescriptor codec_descriptors[]
> = {
>          .long_name = NULL_IF_CONFIG_SMALL("aptX (Audio Processing
> Technology for Bluetooth)"),
>          .props     = AV_CODEC_PROP_LOSSY,
>      },
> +    {
> +        .id        = AV_CODEC_ID_APTX_HD,
> +        .type      = AVMEDIA_TYPE_AUDIO,
> +        .name      = "aptx_hd",
> +        .long_name = NULL_IF_CONFIG_SMALL("aptX HD (Audio Processing
> Technology for Bluetooth)"),
> +        .props     = AV_CODEC_PROP_LOSSY,
> +    },
>
>      /* subtitle codecs */
>      {
> --
> 2.15.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


No, don't add a new codec ID for what is very obviously a profile.

Here's what you need to do:

1.) Add FF_PROFILE_APTX_HD to libavcodec/avcodec.h
2.) During parsing set par->profile to FF_PROFILE_APTX_HD
3.) During decoding init set s->hd to avctx->profile == FF_PROFILE_APTX_HD
     - or better yet don't add a bool variable but do this check every time
something is different


Could you do this for sbc as well so we can get that merged finally?
Aurelien Jacobs Jan. 7, 2018, 10:54 p.m. UTC | #2
On Sun, Jan 07, 2018 at 05:23:24PM +0000, Rostislav Pehlivanov wrote:
> On 6 January 2018 at 16:48, Aurelien Jacobs <aurel@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(-)
> >
> > diff --git a/Changelog b/Changelog
> > index 3d966c202b..9349bf1e8d 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -11,7 +11,7 @@ version <next>:
> >  - TiVo ty/ty+ demuxer
> >  - Intel QSV-accelerated MJPEG encoding
> >  - PCE support for extended channel layouts in the AAC encoder
> > -- native aptX encoder and decoder
> > +- native aptX and aptX HD encoder and decoder
> >  - Raw aptX muxer and demuxer
> >  - NVIDIA NVDEC-accelerated H.264, HEVC, MPEG-1/2/4, VC1, VP8/9 hwaccel
> > decoding
> >  - Intel QSV-accelerated overlay filter
> > diff --git a/configure b/configure
> > index 1d2fffa132..c496346a06 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2459,6 +2459,8 @@ apng_encoder_deps="zlib"
> >  apng_encoder_select="llvidencdsp"
> >  aptx_decoder_select="audio_frame_queue"
> >  aptx_encoder_select="audio_frame_queue"
> > +aptx_hd_decoder_select="audio_frame_queue"
> > +aptx_hd_encoder_select="audio_frame_queue"
> >  asv1_decoder_select="blockdsp bswapdsp idctdsp"
> >  asv1_encoder_select="bswapdsp fdctdsp pixblockdsp"
> >  asv2_decoder_select="blockdsp bswapdsp idctdsp"
> > diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> > index cfacd6b70c..a9ecf7ea5e 100644
> > --- a/libavcodec/Makefile
> > +++ b/libavcodec/Makefile
> > @@ -190,6 +190,8 @@ OBJS-$(CONFIG_ANSI_DECODER)            += ansi.o
> > cga_data.o
> >  OBJS-$(CONFIG_APE_DECODER)             += apedec.o
> >  OBJS-$(CONFIG_APTX_DECODER)            += aptx.o
> >  OBJS-$(CONFIG_APTX_ENCODER)            += aptx.o
> > +OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
> > +OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
> >  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
> >  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
> >  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index ed1e7ab06e..93d31f8688 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > [...]
> > @@ -844,6 +1105,24 @@ AVCodec ff_aptx_decoder = {
> >  };
> >  #endif
> >
> > +#if CONFIG_APTX_HD_DECODER
> > +AVCodec ff_aptx_hd_decoder = {
> > +    .name                  = "aptx_hd",
> > +    .long_name             = NULL_IF_CONFIG_SMALL("aptX HD (Audio
> > Processing Technology for Bluetooth)"),
> > +    .type                  = AVMEDIA_TYPE_AUDIO,
> > +    .id                    = AV_CODEC_ID_APTX_HD,
> > +    .priv_data_size        = sizeof(AptXContext),
> > +    .init                  = aptx_init,
> > +    .decode                = aptx_decode_frame,
> > +    .close                 = aptx_close,
> > +    .capabilities          = AV_CODEC_CAP_DR1,
> > +    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE,
> > +    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO, 0},
> > +    .sample_fmts           = (const enum AVSampleFormat[]) {
> > AV_SAMPLE_FMT_S32P,
> > +
> >  AV_SAMPLE_FMT_NONE },
> > +};
> > +#endif
> > +
> >  #if CONFIG_APTX_ENCODER
> >  AVCodec ff_aptx_encoder = {
> >      .name                  = "aptx",
> > @@ -862,3 +1141,22 @@ AVCodec ff_aptx_encoder = {
> >      .supported_samplerates = (const int[]) {8000, 16000, 24000, 32000,
> > 44100, 48000, 0},
> >  };
> >  #endif
> > +
> > +#if CONFIG_APTX_HD_ENCODER
> > +AVCodec ff_aptx_hd_encoder = {
> > +    .name                  = "aptx_hd",
> > +    .long_name             = NULL_IF_CONFIG_SMALL("aptX HD (Audio
> > Processing Technology for Bluetooth)"),
> > +    .type                  = AVMEDIA_TYPE_AUDIO,
> > +    .id                    = AV_CODEC_ID_APTX_HD,
> > +    .priv_data_size        = sizeof(AptXContext),
> > +    .init                  = aptx_init,
> > +    .encode2               = aptx_encode_frame,
> > +    .close                 = aptx_close,
> > +    .capabilities          = AV_CODEC_CAP_SMALL_LAST_FRAME,
> > +    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE,
> > +    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO, 0},
> > +    .sample_fmts           = (const enum AVSampleFormat[]) {
> > AV_SAMPLE_FMT_S32P,
> > +
> >  AV_SAMPLE_FMT_NONE },
> > +    .supported_samplerates = (const int[]) {8000, 16000, 24000, 32000,
> > 44100, 48000, 0},
> > +};
> > +#endif
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index c13deb599f..95d164abc1 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -634,6 +634,7 @@ enum AVCodecID {
> >      AV_CODEC_ID_ATRAC3PAL,
> >      AV_CODEC_ID_DOLBY_E,
> >      AV_CODEC_ID_APTX,
> > +    AV_CODEC_ID_APTX_HD,
> >
> >      /* subtitle codecs */
> >      AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID
> > pointing at the start of subtitle codecs.
> > diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> > index c3688de1d6..ca18bb2b67 100644
> > --- a/libavcodec/codec_desc.c
> > +++ b/libavcodec/codec_desc.c
> > @@ -2866,6 +2866,13 @@ static const AVCodecDescriptor codec_descriptors[]
> > = {
> >          .long_name = NULL_IF_CONFIG_SMALL("aptX (Audio Processing
> > Technology for Bluetooth)"),
> >          .props     = AV_CODEC_PROP_LOSSY,
> >      },
> > +    {
> > +        .id        = AV_CODEC_ID_APTX_HD,
> > +        .type      = AVMEDIA_TYPE_AUDIO,
> > +        .name      = "aptx_hd",
> > +        .long_name = NULL_IF_CONFIG_SMALL("aptX HD (Audio Processing
> > Technology for Bluetooth)"),
> > +        .props     = AV_CODEC_PROP_LOSSY,
> > +    },
> >
> >      /* subtitle codecs */
> >      {
> 
> 
> No, don't add a new codec ID for what is very obviously a profile.
> 
> Here's what you need to do:
> 
> 1.) Add FF_PROFILE_APTX_HD to libavcodec/avcodec.h
> 2.) During parsing set par->profile to FF_PROFILE_APTX_HD

Parsing ? Do you mean demuxer ?
There is no aptx parser for now.

> 3.) During decoding init set s->hd to avctx->profile == FF_PROFILE_APTX_HD
>      - or better yet don't add a bool variable but do this check every time
> something is different

The bool variable makes things easier because the avctx is not
accessible to functions using it.

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.

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 ?

> Could you do this for sbc as well so we can get that merged finally?

I already replied to this in the SBC thread, and the discussion should
probably continue there, but in the case of SBC, using profile is
even more problematic as SBC and mSBC don't support the same samplerate,
and having a single codec prevent to have too different samplerate
lists and prevent ffmpeg to automatically convert input audio to the
only samplerate supported by mSBC.
Rostislav Pehlivanov Jan. 8, 2018, 12:38 a.m. UTC | #3
On 7 January 2018 at 22:54, Aurelien Jacobs <aurel@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@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.
> >
> > Here's what you need to do:
> >
> > 1.) Add FF_PROFILE_APTX_HD to libavcodec/avcodec.h
> > 2.) During parsing set par->profile to FF_PROFILE_APTX_HD
>
> Parsing ? Do you mean demuxer ?
> There is no aptx parser for now.
>

Sorry, I meant demuxer.



> > 3.) During decoding init set s->hd to avctx->profile ==
> FF_PROFILE_APTX_HD
> >      - or better yet don't add a bool variable but do this check every
> time
> > something is different
>
> The bool variable makes things easier because the avctx is not
> accessible to functions using it.
>

That's okay, I don't mind.



> 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). The only people who would call that brand new are in
marketing and they're wrong. This is just changes the tables and the way
codewords are written in order to optimize the codec for a different
bitrate. That's all, not a new codec, its too small a change. Its a profile.



> 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. The profile
system saves us a decent dozen for when there are differences besides the
"there's one extra block of bits/there's some reordering" - which remain
hidden by the decoder. In that case, as far as the user is concerned
they're not decoding a Chinese Y528 codec, an H264 variant with 1 extra bit
of padding found in one province's TV broadcast to region lock, they're
decoding regular H264.
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. The LTP
profile of AAC is quite similar in what it aims to do compared to aptx HD -
its just the same baseline codec with some changes to make it work better
for a particular bitrate.



> > Could you do this for sbc as well so we can get that merged finally?
>
> I already replied to this in the SBC thread, and the discussion should
> probably continue there, but in the case of SBC, using profile is
> even more problematic as SBC and mSBC don't support the same samplerate,
> and having a single codec prevent to have too different samplerate
> lists and prevent ffmpeg to automatically convert input audio to the
> only samplerate supported by mSBC.
>

That's okay - for encoding switch the profile depending on both the
avctx->profile setting and the samplerate and list all supported
samplerates for all profiles in the AVCodec structure. We do something
similar in the AAC encoder where we pick different settings depending on
the profile and change the profile depending on the settings listed.
If there's an overlap, pick a sane default unless the user forces a profile
using profile:a. If forced and the samplerate isn't supported - error out
and let the user know.
For decoding set the profile via the demuxer. There doesn't have to be just
one demuxer for a codec. If there are major changes in how you parse
profiles go ahead and make a new one which sets the codec ID and the
profile.

In case the differences between the profiles are big yet there's still one
decoder which will decode both, we can also call the encoder something
different. In the case of Dirac and VC2, which are both AV_CODEC_ID_DIRAC,
we just call the vc2 encoder "vc2".



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
compn Jan. 8, 2018, 12:56 a.m. UTC | #4
On Mon, 8 Jan 2018 00:38:19 +0000, Rostislav Pehlivanov
<atomnuker@gmail.com> wrote:
> > > 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 ?
> > itself to any profile, that makes a lot of sense. But for aptX and
> > aptX HD it doesn't make any sense to me.

in ffmpeg we generally try not to create new original internal fourcc /
isom tags. because then people think our tag is the real spec tag, and
then bad things happen.

if another software uses the tag or the sample of that tag is in the
wild then we use it.

i dont know enough about aptx / aptx hd to make an opinion. 

some codecs make different tags for stupid things, like sony hdcam
has different tags for different fps. we support these tags because
other software uses them and samples exist in the wild.

not sure if i've helped here.

-compn
Hendrik Leppkes Jan. 8, 2018, 10:32 a.m. UTC | #5
On Mon, Jan 8, 2018 at 1:38 AM, Rostislav Pehlivanov
<atomnuker@gmail.com> wrote:
>
> That's okay - for encoding switch the profile depending on both the
> avctx->profile setting and the samplerate and list all supported
> samplerates for all profiles in the AVCodec structure. We do something
> similar in the AAC encoder where we pick different settings depending on
> the profile and change the profile depending on the settings listed.
> If there's an overlap, pick a sane default unless the user forces a profile
> using profile:a. If forced and the samplerate isn't supported - error out
> and let the user know.
> For decoding set the profile via the demuxer. There doesn't have to be just
> one demuxer for a codec. If there are major changes in how you parse
> profiles go ahead and make a new one which sets the codec ID and the
> profile.
>

I don't think there is any precedent for "profile" being mandatory for
a decoder to work, so that might be iffy. Decoders usually set
profile, don't require it.
ie. from the docs:
* - decoding: Set by libavcodec.

There is plenty precedent for using "codec_tag" however, so that may
be a better choice - and can hold the tag from the "container" as-is
as well.

- Hendrik
Carl Eugen Hoyos Jan. 8, 2018, 12:27 p.m. UTC | #6
2018-01-08 11:32 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> On Mon, Jan 8, 2018 at 1:38 AM, Rostislav Pehlivanov
> <atomnuker@gmail.com> wrote:
>>
>> That's okay - for encoding switch the profile depending on both the
>> avctx->profile setting and the samplerate and list all supported
>> samplerates for all profiles in the AVCodec structure. We do something
>> similar in the AAC encoder where we pick different settings depending on
>> the profile and change the profile depending on the settings listed.
>> If there's an overlap, pick a sane default unless the user forces a profile
>> using profile:a. If forced and the samplerate isn't supported - error out
>> and let the user know.
>> For decoding set the profile via the demuxer. There doesn't have to be just
>> one demuxer for a codec. If there are major changes in how you parse
>> profiles go ahead and make a new one which sets the codec ID and the
>> profile.
>>
>
> I don't think there is any precedent for "profile" being mandatory for
> a decoder to work, so that might be iffy. Decoders usually set
> profile, don't require it.
> ie. from the docs:
> * - decoding: Set by libavcodec.
>
> There is plenty precedent for using "codec_tag" however, so that may
> be a better choice - and can hold the tag from the "container" as-is
> as well.

While I don't understand why even having more than one codec_id for
the same codec (which afaiu isn't the case here) would be an issue, I
don't think we should invent codec_tags unless necessary.

Carl Eugen
Hendrik Leppkes Jan. 8, 2018, 12:37 p.m. UTC | #7
On Mon, Jan 8, 2018 at 1:27 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2018-01-08 11:32 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
>> On Mon, Jan 8, 2018 at 1:38 AM, Rostislav Pehlivanov
>> <atomnuker@gmail.com> wrote:
>>>
>>> That's okay - for encoding switch the profile depending on both the
>>> avctx->profile setting and the samplerate and list all supported
>>> samplerates for all profiles in the AVCodec structure. We do something
>>> similar in the AAC encoder where we pick different settings depending on
>>> the profile and change the profile depending on the settings listed.
>>> If there's an overlap, pick a sane default unless the user forces a profile
>>> using profile:a. If forced and the samplerate isn't supported - error out
>>> and let the user know.
>>> For decoding set the profile via the demuxer. There doesn't have to be just
>>> one demuxer for a codec. If there are major changes in how you parse
>>> profiles go ahead and make a new one which sets the codec ID and the
>>> profile.
>>>
>>
>> I don't think there is any precedent for "profile" being mandatory for
>> a decoder to work, so that might be iffy. Decoders usually set
>> profile, don't require it.
>> ie. from the docs:
>> * - decoding: Set by libavcodec.
>>
>> There is plenty precedent for using "codec_tag" however, so that may
>> be a better choice - and can hold the tag from the "container" as-is
>> as well.
>
> While I don't understand why even having more than one codec_id for
> the same codec (which afaiu isn't the case here) would be an issue, I
> don't think we should invent codec_tags unless necessary.
>

We're not inventing them if they are a copy from the container
bitstream tag field.

- Hendrik
Carl Eugen Hoyos Jan. 8, 2018, 1:19 p.m. UTC | #8
2018-01-08 13:37 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> On Mon, Jan 8, 2018 at 1:27 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> 2018-01-08 11:32 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
>>> On Mon, Jan 8, 2018 at 1:38 AM, Rostislav Pehlivanov
>>> <atomnuker@gmail.com> wrote:
>>>>
>>>> That's okay - for encoding switch the profile depending on both the
>>>> avctx->profile setting and the samplerate and list all supported
>>>> samplerates for all profiles in the AVCodec structure. We do something
>>>> similar in the AAC encoder where we pick different settings depending on
>>>> the profile and change the profile depending on the settings listed.
>>>> If there's an overlap, pick a sane default unless the user forces a profile
>>>> using profile:a. If forced and the samplerate isn't supported - error out
>>>> and let the user know.
>>>> For decoding set the profile via the demuxer. There doesn't have to be just
>>>> one demuxer for a codec. If there are major changes in how you parse
>>>> profiles go ahead and make a new one which sets the codec ID and the
>>>> profile.
>>>>
>>>
>>> I don't think there is any precedent for "profile" being mandatory for
>>> a decoder to work, so that might be iffy. Decoders usually set
>>> profile, don't require it.
>>> ie. from the docs:
>>> * - decoding: Set by libavcodec.
>>>
>>> There is plenty precedent for using "codec_tag" however, so that may
>>> be a better choice - and can hold the tag from the "container" as-is
>>> as well.
>>
>> While I don't understand why even having more than one codec_id for
>> the same codec (which afaiu isn't the case here) would be an issue, I
>> don't think we should invent codec_tags unless necessary.
>>
>
> We're not inventing them if they are a copy from the container
> bitstream tag field.

I completely agree!

Is there a container bitstream tag field in this case?

Carl Eugen
Aurelien Jacobs Jan. 9, 2018, 12:42 a.m. UTC | #9
On Mon, Jan 08, 2018 at 01:27:16PM +0100, Carl Eugen Hoyos wrote:
> 2018-01-08 11:32 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> > On Mon, Jan 8, 2018 at 1:38 AM, Rostislav Pehlivanov
> > <atomnuker@gmail.com> wrote:
> >>
> >> That's okay - for encoding switch the profile depending on both the
> >> avctx->profile setting and the samplerate and list all supported
> >> samplerates for all profiles in the AVCodec structure. We do something
> >> similar in the AAC encoder where we pick different settings depending on
> >> the profile and change the profile depending on the settings listed.
> >> If there's an overlap, pick a sane default unless the user forces a profile
> >> using profile:a. If forced and the samplerate isn't supported - error out
> >> and let the user know.
> >> For decoding set the profile via the demuxer. There doesn't have to be just
> >> one demuxer for a codec. If there are major changes in how you parse
> >> profiles go ahead and make a new one which sets the codec ID and the
> >> profile.
> >>
> >
> > I don't think there is any precedent for "profile" being mandatory for
> > a decoder to work, so that might be iffy. Decoders usually set
> > profile, don't require it.
> > ie. from the docs:
> > * - decoding: Set by libavcodec.

Oh, very true. I guess that settles it for "profile". Public API do not
allows using profile to select the appropriate decoder.

> > There is plenty precedent for using "codec_tag" however, so that may
> > be a better choice - and can hold the tag from the "container" as-is
> > as well.
> 
> While I don't understand why even having more than one codec_id for
> the same codec (which afaiu isn't the case here) would be an issue,

Same for me, I don't understand why adding a codec_id would be an issue.

> I don't think we should invent codec_tags unless necessary.

I agree. And I don't think there is a container used for aptX or aptX HD
in the wild with some kind of codec_tag. So this is probably not an
option either.

I maintain that using 2 codec IDs is the most appropriate in this
specific case.
Aurelien Jacobs Jan. 9, 2018, 1:21 a.m. UTC | #10
On Mon, Jan 08, 2018 at 12:38:19AM +0000, Rostislav Pehlivanov wrote:
> On 7 January 2018 at 22:54, Aurelien Jacobs <aurel@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@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).

> > 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 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.

Anyway, all this discussion is moot as Hendrik pointed out that profile
can't be set outside of lavc to determine a decoder behavior.
Rostislav Pehlivanov Jan. 9, 2018, 4:07 a.m. UTC | #11
On 9 January 2018 at 01:21, Aurelien Jacobs <aurel@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@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@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.
Carl Eugen Hoyos Jan. 9, 2018, 5:25 a.m. UTC | #12
2018-01-09 5:07 GMT+01:00 Rostislav Pehlivanov <atomnuker@gmail.com>:
> On 9 January 2018 at 01:21, Aurelien Jacobs <aurel@gnuage.org> wrote:

>> 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.

No?

Carl Eugen
Hendrik Leppkes Jan. 9, 2018, 8:33 a.m. UTC | #13
On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov
<atomnuker@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.

- Hendrik
Hendrik Leppkes Jan. 9, 2018, 9 a.m. UTC | #14
On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov
> <atomnuker@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.
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)

- Hendrik
Rostislav Pehlivanov Jan. 9, 2018, 2:07 p.m. UTC | #15
On 9 January 2018 at 09:00, Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes <h.leppkes@gmail.com>
> wrote:
> > On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov
> > <atomnuker@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)
>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

I'd be fine with using codec tags, but not with codec IDs.
Rostislav Pehlivanov Jan. 9, 2018, 2:21 p.m. UTC | #16
On 9 January 2018 at 14:07, Rostislav Pehlivanov <atomnuker@gmail.com>
wrote:

>
>
> On 9 January 2018 at 09:00, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
>> On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes <h.leppkes@gmail.com>
>> wrote:
>> > On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov
>> > <atomnuker@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)
>>
>> - Hendrik
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> I'd be fine with using codec tags, but not with codec IDs.
>

Though for encoding using profiles would be best.
Hendrik Leppkes Jan. 9, 2018, 2:30 p.m. UTC | #17
On Tue, Jan 9, 2018 at 3:21 PM, Rostislav Pehlivanov
<atomnuker@gmail.com> wrote:
> On 9 January 2018 at 14:07, Rostislav Pehlivanov <atomnuker@gmail.com>
> wrote:
>
>>
>>
>> On 9 January 2018 at 09:00, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>>
>>> On Tue, Jan 9, 2018 at 9:33 AM, Hendrik Leppkes <h.leppkes@gmail.com>
>>> wrote:
>>> > On Tue, Jan 9, 2018 at 5:07 AM, Rostislav Pehlivanov
>>> > <atomnuker@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)
>>>
>>> - Hendrik
>>
>> I'd be fine with using codec tags, but not with codec IDs.
>>
>
> Though for encoding using profiles would be best.

For encoding, profiles is perfectly normal and expected, and used by
many other codecs as well. Just for decoding that field is not defined
to be required to be set externally to even make decoding of a certain
stream possible.

- Hendrik
diff mbox

Patch

diff --git a/Changelog b/Changelog
index 3d966c202b..9349bf1e8d 100644
--- a/Changelog
+++ b/Changelog
@@ -11,7 +11,7 @@  version <next>:
 - TiVo ty/ty+ demuxer
 - Intel QSV-accelerated MJPEG encoding
 - PCE support for extended channel layouts in the AAC encoder
-- native aptX encoder and decoder
+- native aptX and aptX HD encoder and decoder
 - Raw aptX muxer and demuxer
 - NVIDIA NVDEC-accelerated H.264, HEVC, MPEG-1/2/4, VC1, VP8/9 hwaccel decoding
 - Intel QSV-accelerated overlay filter
diff --git a/configure b/configure
index 1d2fffa132..c496346a06 100755
--- a/configure
+++ b/configure
@@ -2459,6 +2459,8 @@  apng_encoder_deps="zlib"
 apng_encoder_select="llvidencdsp"
 aptx_decoder_select="audio_frame_queue"
 aptx_encoder_select="audio_frame_queue"
+aptx_hd_decoder_select="audio_frame_queue"
+aptx_hd_encoder_select="audio_frame_queue"
 asv1_decoder_select="blockdsp bswapdsp idctdsp"
 asv1_encoder_select="bswapdsp fdctdsp pixblockdsp"
 asv2_decoder_select="blockdsp bswapdsp idctdsp"
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index cfacd6b70c..a9ecf7ea5e 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -190,6 +190,8 @@  OBJS-$(CONFIG_ANSI_DECODER)            += ansi.o cga_data.o
 OBJS-$(CONFIG_APE_DECODER)             += apedec.o
 OBJS-$(CONFIG_APTX_DECODER)            += aptx.o
 OBJS-$(CONFIG_APTX_ENCODER)            += aptx.o
+OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
+OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
 OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
 OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
 OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index ed1e7ab06e..93d31f8688 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -333,6 +333,7 @@  static void register_all(void)
     REGISTER_DECODER(AMRWB,             amrwb);
     REGISTER_DECODER(APE,               ape);
     REGISTER_ENCDEC (APTX,              aptx);
+    REGISTER_ENCDEC (APTX_HD,           aptx_hd);
     REGISTER_DECODER(ATRAC1,            atrac1);
     REGISTER_DECODER(ATRAC3,            atrac3);
     REGISTER_DECODER(ATRAC3AL,          atrac3al);
diff --git a/libavcodec/aptx.c b/libavcodec/aptx.c
index 4173402d03..6c0f3d35a9 100644
--- a/libavcodec/aptx.c
+++ b/libavcodec/aptx.c
@@ -89,6 +89,8 @@  typedef struct {
 } Channel;
 
 typedef struct {
+    int hd;
+    int block_size;
     int32_t sync_idx;
     Channel channels[NB_CHANNELS];
     AudioFrameQueue afq;
@@ -182,6 +184,205 @@  static const int16_t quantize_factor_select_offset_HF[5] = {
     0, -8, 33, 95, 262,
 };
 
+
+static const int32_t hd_quantize_intervals_LF[257] = {
+      -2436,    2436,    7308,   12180,   17054,   21930,   26806,   31686,
+      36566,   41450,   46338,   51230,   56124,   61024,   65928,   70836,
+      75750,   80670,   85598,   90530,   95470,  100418,  105372,  110336,
+     115308,  120288,  125278,  130276,  135286,  140304,  145334,  150374,
+     155426,  160490,  165566,  170654,  175756,  180870,  185998,  191138,
+     196294,  201466,  206650,  211850,  217068,  222300,  227548,  232814,
+     238096,  243396,  248714,  254050,  259406,  264778,  270172,  275584,
+     281018,  286470,  291944,  297440,  302956,  308496,  314056,  319640,
+     325248,  330878,  336532,  342212,  347916,  353644,  359398,  365178,
+     370986,  376820,  382680,  388568,  394486,  400430,  406404,  412408,
+     418442,  424506,  430600,  436726,  442884,  449074,  455298,  461554,
+     467844,  474168,  480528,  486922,  493354,  499820,  506324,  512866,
+     519446,  526064,  532722,  539420,  546160,  552940,  559760,  566624,
+     573532,  580482,  587478,  594520,  601606,  608740,  615920,  623148,
+     630426,  637754,  645132,  652560,  660042,  667576,  675164,  682808,
+     690506,  698262,  706074,  713946,  721876,  729868,  737920,  746036,
+     754216,  762460,  770770,  779148,  787594,  796108,  804694,  813354,
+     822086,  830892,  839774,  848736,  857776,  866896,  876100,  885386,
+     894758,  904218,  913766,  923406,  933138,  942964,  952886,  962908,
+     973030,  983254,  993582, 1004020, 1014566, 1025224, 1035996, 1046886,
+    1057894, 1069026, 1080284, 1091670, 1103186, 1114838, 1126628, 1138558,
+    1150634, 1162858, 1175236, 1187768, 1200462, 1213320, 1226346, 1239548,
+    1252928, 1266490, 1280242, 1294188, 1308334, 1322688, 1337252, 1352034,
+    1367044, 1382284, 1397766, 1413494, 1429478, 1445728, 1462252, 1479058,
+    1496158, 1513562, 1531280, 1549326, 1567710, 1586446, 1605550, 1625034,
+    1644914, 1665208, 1685932, 1707108, 1728754, 1750890, 1773542, 1796732,
+    1820488, 1844840, 1869816, 1895452, 1921780, 1948842, 1976680, 2005338,
+    2034868, 2065322, 2096766, 2129260, 2162880, 2197708, 2233832, 2271352,
+    2310384, 2351050, 2393498, 2437886, 2484404, 2533262, 2584710, 2639036,
+    2696578, 2757738, 2822998, 2892940, 2968278, 3049896, 3138912, 3236760,
+    3345312, 3467068, 3605434, 3765154, 3952904, 4177962, 4452178, 4787134,
+    5187290, 5647128, 6159120, 6720518, 7332904, 8000032, 8726664, 9518152,
+    10380372,
+};
+static const int32_t hd_invert_quantize_dither_factors_LF[257] = {
+      2436,   2436,   2436,   2436,   2438,   2438,   2438,   2440,
+      2442,   2442,   2444,   2446,   2448,   2450,   2454,   2456,
+      2458,   2462,   2464,   2468,   2472,   2476,   2480,   2484,
+      2488,   2492,   2498,   2502,   2506,   2512,   2518,   2524,
+      2528,   2534,   2540,   2548,   2554,   2560,   2568,   2574,
+      2582,   2588,   2596,   2604,   2612,   2620,   2628,   2636,
+      2646,   2654,   2664,   2672,   2682,   2692,   2702,   2712,
+      2722,   2732,   2742,   2752,   2764,   2774,   2786,   2798,
+      2810,   2822,   2834,   2846,   2858,   2870,   2884,   2896,
+      2910,   2924,   2938,   2952,   2966,   2980,   2994,   3010,
+      3024,   3040,   3056,   3070,   3086,   3104,   3120,   3136,
+      3154,   3170,   3188,   3206,   3224,   3242,   3262,   3280,
+      3300,   3320,   3338,   3360,   3380,   3400,   3422,   3442,
+      3464,   3486,   3508,   3532,   3554,   3578,   3602,   3626,
+      3652,   3676,   3702,   3728,   3754,   3780,   3808,   3836,
+      3864,   3892,   3920,   3950,   3980,   4010,   4042,   4074,
+      4106,   4138,   4172,   4206,   4240,   4276,   4312,   4348,
+      4384,   4422,   4460,   4500,   4540,   4580,   4622,   4664,
+      4708,   4752,   4796,   4842,   4890,   4938,   4986,   5036,
+      5086,   5138,   5192,   5246,   5300,   5358,   5416,   5474,
+      5534,   5596,   5660,   5726,   5792,   5860,   5930,   6002,
+      6074,   6150,   6226,   6306,   6388,   6470,   6556,   6644,
+      6736,   6828,   6924,   7022,   7124,   7228,   7336,   7448,
+      7562,   7680,   7802,   7928,   8058,   8192,   8332,   8476,
+      8624,   8780,   8940,   9106,   9278,   9458,   9644,   9840,
+     10042,  10252,  10472,  10702,  10942,  11194,  11458,  11734,
+     12024,  12328,  12648,  12986,  13342,  13720,  14118,  14540,
+     14990,  15466,  15976,  16520,  17102,  17726,  18398,  19124,
+     19908,  20760,  21688,  22702,  23816,  25044,  26404,  27922,
+     29622,  31540,  33720,  36222,  39116,  42502,  46514,  51334,
+     57218,  64536,  73830,  85890, 101860, 123198, 151020, 183936,
+    216220, 243618, 268374, 293022, 319362, 347768, 378864, 412626, 449596,
+};
+static const int32_t hd_quantize_dither_factors_LF[256] = {
+       0,    0,    0,    1,    0,    0,    1,    1,
+       0,    1,    1,    1,    1,    1,    1,    1,
+       1,    1,    1,    1,    1,    1,    1,    1,
+       1,    2,    1,    1,    2,    2,    2,    1,
+       2,    2,    2,    2,    2,    2,    2,    2,
+       2,    2,    2,    2,    2,    2,    2,    3,
+       2,    3,    2,    3,    3,    3,    3,    3,
+       3,    3,    3,    3,    3,    3,    3,    3,
+       3,    3,    3,    3,    3,    4,    3,    4,
+       4,    4,    4,    4,    4,    4,    4,    4,
+       4,    4,    4,    4,    5,    4,    4,    5,
+       4,    5,    5,    5,    5,    5,    5,    5,
+       5,    5,    6,    5,    5,    6,    5,    6,
+       6,    6,    6,    6,    6,    6,    6,    7,
+       6,    7,    7,    7,    7,    7,    7,    7,
+       7,    7,    8,    8,    8,    8,    8,    8,
+       8,    9,    9,    9,    9,    9,    9,    9,
+      10,   10,   10,   10,   10,   11,   11,   11,
+      11,   11,   12,   12,   12,   12,   13,   13,
+      13,   14,   14,   14,   15,   15,   15,   15,
+      16,   16,   17,   17,   17,   18,   18,   18,
+      19,   19,   20,   21,   21,   22,   22,   23,
+      23,   24,   25,   26,   26,   27,   28,   29,
+      30,   31,   32,   33,   34,   35,   36,   37,
+      39,   40,   42,   43,   45,   47,   49,   51,
+      53,   55,   58,   60,   63,   66,   69,   73,
+      76,   80,   85,   89,   95,  100,  106,  113,
+     119,  128,  136,  146,  156,  168,  182,  196,
+     213,  232,  254,  279,  307,  340,  380,  425,
+     480,  545,  626,  724,  847, 1003, 1205, 1471,
+    1830, 2324, 3015, 3993, 5335, 6956, 8229, 8071,
+    6850, 6189, 6162, 6585, 7102, 7774, 8441, 9243,
+};
+static const int16_t hd_quantize_factor_select_offset_LF[257] = {
+      0, -22, -21, -21, -20, -20, -19, -19,
+    -18, -18, -17, -17, -16, -16, -15, -14,
+    -14, -13, -13, -12, -12, -11, -11, -10,
+    -10,  -9,  -9,  -8,  -7,  -7,  -6,  -6,
+     -5,  -5,  -4,  -4,  -3,  -3,  -2,  -1,
+     -1,   0,   0,   1,   1,   2,   2,   3,
+      4,   4,   5,   5,   6,   6,   7,   8,
+      8,   9,   9,  10,  11,  11,  12,  12,
+     13,  14,  14,  15,  15,  16,  17,  17,
+     18,  19,  19,  20,  20,  21,  22,  22,
+     23,  24,  24,  25,  26,  26,  27,  28,
+     28,  29,  30,  30,  31,  32,  33,  33,
+     34,  35,  35,  36,  37,  38,  38,  39,
+     40,  41,  41,  42,  43,  44,  44,  45,
+     46,  47,  48,  48,  49,  50,  51,  52,
+     52,  53,  54,  55,  56,  57,  58,  58,
+     59,  60,  61,  62,  63,  64,  65,  66,
+     67,  68,  69,  69,  70,  71,  72,  73,
+     74,  75,  77,  78,  79,  80,  81,  82,
+     83,  84,  85,  86,  87,  89,  90,  91,
+     92,  93,  94,  96,  97,  98,  99, 101,
+    102, 103, 105, 106, 107, 109, 110, 112,
+    113, 115, 116, 118, 119, 121, 122, 124,
+    125, 127, 129, 130, 132, 134, 136, 137,
+    139, 141, 143, 145, 147, 149, 151, 153,
+    155, 158, 160, 162, 164, 167, 169, 172,
+    174, 177, 180, 182, 185, 188, 191, 194,
+    197, 201, 204, 208, 211, 215, 219, 223,
+    227, 232, 236, 241, 246, 251, 257, 263,
+    269, 275, 283, 290, 298, 307, 317, 327,
+    339, 352, 367, 384, 404, 429, 458, 494,
+    522, 522, 522, 522, 522, 522, 522, 522, 522,
+};
+
+
+static const int32_t hd_quantize_intervals_MLF[33] = {
+      -21236,   21236,   63830,  106798,  150386,  194832,  240376,  287258,
+      335726,  386034,  438460,  493308,  550924,  611696,  676082,  744626,
+      817986,  896968,  982580, 1076118, 1179278, 1294344, 1424504, 1574386,
+     1751090, 1966260, 2240868, 2617662, 3196432, 4176450, 5658260, 7671068,
+    10380372,
+};
+static const int32_t hd_invert_quantize_dither_factors_MLF[33] = {
+    21236,  21236,  21360,  21608,  21978,  22468,  23076,   23806,
+    24660,  25648,  26778,  28070,  29544,  31228,  33158,   35386,
+    37974,  41008,  44606,  48934,  54226,  60840,  69320,   80564,
+    96140, 119032, 155576, 221218, 357552, 622468, 859344, 1153464, 1555840,
+};
+static const int32_t hd_quantize_dither_factors_MLF[32] = {
+       0,   31,    62,    93,   123,   152,   183,    214,
+     247,  283,   323,   369,   421,   483,   557,    647,
+     759,  900,  1082,  1323,  1654,  2120,  2811,   3894,
+    5723, 9136, 16411, 34084, 66229, 59219, 73530, 100594,
+};
+static const int16_t hd_quantize_factor_select_offset_MLF[33] = {
+      0, -21, -16, -12,  -7,  -2,   3,   8,
+     13,  19,  24,  30,  36,  43,  50,  57,
+     65,  74,  83,  93, 104, 117, 131, 147,
+    166, 189, 219, 259, 322, 427, 521, 521, 521,
+};
+
+
+static const int32_t hd_quantize_intervals_MHF[9] = {
+    -95044, 95044, 295844, 528780, 821332, 1226438, 1890540, 3344850, 6450664,
+};
+static const int32_t hd_invert_quantize_dither_factors_MHF[9] = {
+    95044, 95044, 105754, 127180, 165372, 39736, 424366, 1029946, 2075866,
+};
+static const int32_t hd_quantize_dither_factors_MHF[8] = {
+    0, 2678, 5357, 9548, -31409, 96158, 151395, 261480,
+};
+static const int16_t hd_quantize_factor_select_offset_MHF[9] = {
+    0, -17, 5, 30, 62, 105, 177, 334, 518,
+};
+
+
+static const int32_t hd_quantize_intervals_HF[17] = {
+     -45754,   45754,  138496,  234896,  337336,  448310,  570738,  708380,
+     866534, 1053262, 1281958, 1577438, 1993050, 2665984, 3900982, 5902844,
+    8897462,
+};
+static const int32_t hd_invert_quantize_dither_factors_HF[17] = {
+    45754,  45754,  46988,  49412,  53026,  57950,  64478,   73164,
+    84988, 101740, 126958, 168522, 247092, 425842, 809154, 1192708, 1801910,
+};
+static const int32_t hd_quantize_dither_factors_HF[16] = {
+       0,  309,   606,   904,  1231,  1632,  2172,   2956,
+    4188, 6305, 10391, 19643, 44688, 95828, 95889, 152301,
+};
+static const int16_t hd_quantize_factor_select_offset_HF[17] = {
+     0, -18,  -8,   2,  13,  25,  38,  53,
+    70,  90, 115, 147, 192, 264, 398, 521, 521,
+};
+
 typedef const struct {
     const int32_t *quantize_intervals;
     const int32_t *invert_quantize_dither_factors;
@@ -192,7 +393,8 @@  typedef const struct {
     int32_t prediction_order;
 } ConstTables;
 
-static ConstTables tables[NB_SUBBANDS] = {
+static ConstTables tables[2][NB_SUBBANDS] = {
+{
     [LF]  = { quantize_intervals_LF,
               invert_quantize_dither_factors_LF,
               quantize_dither_factors_LF,
@@ -217,6 +419,33 @@  static ConstTables tables[NB_SUBBANDS] = {
               quantize_factor_select_offset_HF,
               FF_ARRAY_ELEMS(quantize_intervals_HF),
               0x15FF, 12 },
+},
+{
+    [LF]  = { hd_quantize_intervals_LF,
+              hd_invert_quantize_dither_factors_LF,
+              hd_quantize_dither_factors_LF,
+              hd_quantize_factor_select_offset_LF,
+              FF_ARRAY_ELEMS(hd_quantize_intervals_LF),
+              0x11FF, 24 },
+    [MLF] = { hd_quantize_intervals_MLF,
+              hd_invert_quantize_dither_factors_MLF,
+              hd_quantize_dither_factors_MLF,
+              hd_quantize_factor_select_offset_MLF,
+              FF_ARRAY_ELEMS(hd_quantize_intervals_MLF),
+              0x14FF, 12 },
+    [MHF] = { hd_quantize_intervals_MHF,
+              hd_invert_quantize_dither_factors_MHF,
+              hd_quantize_dither_factors_MHF,
+              hd_quantize_factor_select_offset_MHF,
+              FF_ARRAY_ELEMS(hd_quantize_intervals_MHF),
+              0x16FF, 6 },
+    [HF]  = { hd_quantize_intervals_HF,
+              hd_invert_quantize_dither_factors_HF,
+              hd_quantize_dither_factors_HF,
+              hd_quantize_factor_select_offset_HF,
+              FF_ARRAY_ELEMS(hd_quantize_intervals_HF),
+              0x15FF, 12 },
+}
 };
 
 static const int16_t quantization_factors[32] = {
@@ -494,7 +723,7 @@  static void aptx_quantize_difference(Quantize *quantize,
     quantize->quantized_sample_parity_change = parity_change    ^ inv;
 }
 
-static void aptx_encode_channel(Channel *channel, int32_t samples[4])
+static void aptx_encode_channel(Channel *channel, int32_t samples[4], int hd)
 {
     int32_t subband_samples[4];
     int subband;
@@ -505,7 +734,7 @@  static void aptx_encode_channel(Channel *channel, int32_t samples[4])
         aptx_quantize_difference(&channel->quantize[subband], diff,
                                  channel->dither[subband],
                                  channel->invert_quantize[subband].quantization_factor,
-                                 &tables[subband]);
+                                 &tables[hd][subband]);
     }
 }
 
@@ -616,7 +845,7 @@  static void aptx_process_subband(InvertQuantize *invert_quantize,
                               tables->prediction_order);
 }
 
-static void aptx_invert_quantize_and_prediction(Channel *channel)
+static void aptx_invert_quantize_and_prediction(Channel *channel, int hd)
 {
     int subband;
     for (subband = 0; subband < NB_SUBBANDS; subband++)
@@ -624,7 +853,7 @@  static void aptx_invert_quantize_and_prediction(Channel *channel)
                              &channel->prediction[subband],
                              channel->quantize[subband].quantized_sample,
                              channel->dither[subband],
-                             &tables[subband]);
+                             &tables[hd][subband]);
 }
 
 static int32_t aptx_quantized_parity(Channel *channel)
@@ -678,6 +907,15 @@  static uint16_t aptx_pack_codeword(Channel *channel)
          | (((channel->quantize[0].quantized_sample & 0x7F)         ) <<  0);
 }
 
+static uint32_t aptxhd_pack_codeword(Channel *channel)
+{
+    int32_t parity = aptx_quantized_parity(channel);
+    return (((channel->quantize[3].quantized_sample & 0x01E) | parity) << 19)
+         | (((channel->quantize[2].quantized_sample & 0x00F)         ) << 15)
+         | (((channel->quantize[1].quantized_sample & 0x03F)         ) <<  9)
+         | (((channel->quantize[0].quantized_sample & 0x1FF)         ) <<  0);
+}
+
 static void aptx_unpack_codeword(Channel *channel, uint16_t codeword)
 {
     channel->quantize[0].quantized_sample = sign_extend(codeword >>  0, 7);
@@ -688,35 +926,53 @@  static void aptx_unpack_codeword(Channel *channel, uint16_t codeword)
                                           | aptx_quantized_parity(channel);
 }
 
+static void aptxhd_unpack_codeword(Channel *channel, uint32_t codeword)
+{
+    channel->quantize[0].quantized_sample = sign_extend(codeword >>  0, 9);
+    channel->quantize[1].quantized_sample = sign_extend(codeword >>  9, 6);
+    channel->quantize[2].quantized_sample = sign_extend(codeword >> 15, 4);
+    channel->quantize[3].quantized_sample = sign_extend(codeword >> 19, 5);
+    channel->quantize[3].quantized_sample = (channel->quantize[3].quantized_sample & ~1)
+                                          | aptx_quantized_parity(channel);
+}
+
 static void aptx_encode_samples(AptXContext *ctx,
                                 int32_t samples[NB_CHANNELS][4],
-                                uint8_t output[2*NB_CHANNELS])
+                                uint8_t *output)
 {
     int channel;
     for (channel = 0; channel < NB_CHANNELS; channel++)
-        aptx_encode_channel(&ctx->channels[channel], samples[channel]);
+        aptx_encode_channel(&ctx->channels[channel], samples[channel], ctx->hd);
 
     aptx_insert_sync(ctx->channels, &ctx->sync_idx);
 
     for (channel = 0; channel < NB_CHANNELS; channel++) {
-        aptx_invert_quantize_and_prediction(&ctx->channels[channel]);
-        AV_WB16(output + 2*channel, aptx_pack_codeword(&ctx->channels[channel]));
+        aptx_invert_quantize_and_prediction(&ctx->channels[channel], ctx->hd);
+        if (ctx->hd)
+            AV_WB24(output + 3*channel,
+                    aptxhd_pack_codeword(&ctx->channels[channel]));
+        else
+            AV_WB16(output + 2*channel,
+                    aptx_pack_codeword(&ctx->channels[channel]));
     }
 }
 
 static int aptx_decode_samples(AptXContext *ctx,
-                                const uint8_t input[2*NB_CHANNELS],
+                                const uint8_t *input,
                                 int32_t samples[NB_CHANNELS][4])
 {
     int channel, ret;
 
     for (channel = 0; channel < NB_CHANNELS; channel++) {
-        uint16_t codeword;
         aptx_generate_dither(&ctx->channels[channel]);
 
-        codeword = AV_RB16(input + 2*channel);
-        aptx_unpack_codeword(&ctx->channels[channel], codeword);
-        aptx_invert_quantize_and_prediction(&ctx->channels[channel]);
+        if (ctx->hd)
+            aptxhd_unpack_codeword(&ctx->channels[channel],
+                                   AV_RB24(input + 3*channel));
+        else
+            aptx_unpack_codeword(&ctx->channels[channel],
+                                 AV_RB16(input + 2*channel));
+        aptx_invert_quantize_and_prediction(&ctx->channels[channel], ctx->hd);
     }
 
     ret = aptx_check_parity(ctx->channels, &ctx->sync_idx);
@@ -733,11 +989,15 @@  static av_cold int aptx_init(AVCodecContext *avctx)
     AptXContext *s = avctx->priv_data;
     int chan, subband;
 
+    s->hd = avctx->codec->id == AV_CODEC_ID_APTX_HD;
+    s->block_size = s->hd ? 6 : 4;
+
     if (avctx->frame_size == 0)
-        avctx->frame_size = 1024;
+        avctx->frame_size = 256 * s->block_size;
 
-    if (avctx->frame_size & 3) {
-        av_log(avctx, AV_LOG_ERROR, "Frame size must be a multiple of 4 samples\n");
+    if (avctx->frame_size % s->block_size) {
+        av_log(avctx, AV_LOG_ERROR,
+               "Frame size must be a multiple of %d samples\n", s->block_size);
         return AVERROR(EINVAL);
     }
 
@@ -759,9 +1019,9 @@  static int aptx_decode_frame(AVCodecContext *avctx, void *data,
 {
     AptXContext *s = avctx->priv_data;
     AVFrame *frame = data;
-    int pos, channel, sample, ret;
+    int pos, opos, channel, sample, ret;
 
-    if (avpkt->size < 4) {
+    if (avpkt->size < s->block_size) {
         av_log(avctx, AV_LOG_ERROR, "Packet is too small\n");
         return AVERROR_INVALIDDATA;
     }
@@ -769,11 +1029,11 @@  static int aptx_decode_frame(AVCodecContext *avctx, void *data,
     /* get output buffer */
     frame->channels = NB_CHANNELS;
     frame->format = AV_SAMPLE_FMT_S32P;
-    frame->nb_samples = avpkt->size & ~3;
+    frame->nb_samples = 4 * avpkt->size / s->block_size;
     if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
         return ret;
 
-    for (pos = 0; pos < frame->nb_samples; pos += 4) {
+    for (pos = 0, opos = 0; opos < frame->nb_samples; pos += s->block_size, opos += 4) {
         int32_t samples[NB_CHANNELS][4];
 
         if (aptx_decode_samples(s, &avpkt->data[pos], samples)) {
@@ -783,32 +1043,33 @@  static int aptx_decode_frame(AVCodecContext *avctx, void *data,
 
         for (channel = 0; channel < NB_CHANNELS; channel++)
             for (sample = 0; sample < 4; sample++)
-                AV_WN32A(&frame->data[channel][4*(sample+pos)],
+                AV_WN32A(&frame->data[channel][4*(opos+sample)],
                          samples[channel][sample] << 8);
     }
 
     *got_frame_ptr = 1;
-    return frame->nb_samples;
+    return s->block_size * frame->nb_samples / 4;
 }
 
 static int aptx_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
                              const AVFrame *frame, int *got_packet_ptr)
 {
     AptXContext *s = avctx->priv_data;
-    int pos, channel, sample, ret;
+    int pos, ipos, channel, sample, output_size, ret;
 
     if ((ret = ff_af_queue_add(&s->afq, frame)) < 0)
         return ret;
 
-    if ((ret = ff_alloc_packet2(avctx, avpkt, frame->nb_samples, 0)) < 0)
+    output_size = s->block_size * frame->nb_samples/4;
+    if ((ret = ff_alloc_packet2(avctx, avpkt, output_size, 0)) < 0)
         return ret;
 
-    for (pos = 0; pos < frame->nb_samples; pos += 4) {
+    for (pos = 0, ipos = 0; pos < output_size; pos += s->block_size, ipos += 4) {
         int32_t samples[NB_CHANNELS][4];
 
         for (channel = 0; channel < NB_CHANNELS; channel++)
             for (sample = 0; sample < 4; sample++)
-                samples[channel][sample] = (int32_t)AV_RN32A(&frame->data[channel][4*(sample+pos)]) >> 8;
+                samples[channel][sample] = (int32_t)AV_RN32A(&frame->data[channel][4*(ipos+sample)]) >> 8;
 
         aptx_encode_samples(s, samples, avpkt->data + pos);
     }
@@ -844,6 +1105,24 @@  AVCodec ff_aptx_decoder = {
 };
 #endif
 
+#if CONFIG_APTX_HD_DECODER
+AVCodec ff_aptx_hd_decoder = {
+    .name                  = "aptx_hd",
+    .long_name             = NULL_IF_CONFIG_SMALL("aptX HD (Audio Processing Technology for Bluetooth)"),
+    .type                  = AVMEDIA_TYPE_AUDIO,
+    .id                    = AV_CODEC_ID_APTX_HD,
+    .priv_data_size        = sizeof(AptXContext),
+    .init                  = aptx_init,
+    .decode                = aptx_decode_frame,
+    .close                 = aptx_close,
+    .capabilities          = AV_CODEC_CAP_DR1,
+    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE,
+    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO, 0},
+    .sample_fmts           = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S32P,
+                                                             AV_SAMPLE_FMT_NONE },
+};
+#endif
+
 #if CONFIG_APTX_ENCODER
 AVCodec ff_aptx_encoder = {
     .name                  = "aptx",
@@ -862,3 +1141,22 @@  AVCodec ff_aptx_encoder = {
     .supported_samplerates = (const int[]) {8000, 16000, 24000, 32000, 44100, 48000, 0},
 };
 #endif
+
+#if CONFIG_APTX_HD_ENCODER
+AVCodec ff_aptx_hd_encoder = {
+    .name                  = "aptx_hd",
+    .long_name             = NULL_IF_CONFIG_SMALL("aptX HD (Audio Processing Technology for Bluetooth)"),
+    .type                  = AVMEDIA_TYPE_AUDIO,
+    .id                    = AV_CODEC_ID_APTX_HD,
+    .priv_data_size        = sizeof(AptXContext),
+    .init                  = aptx_init,
+    .encode2               = aptx_encode_frame,
+    .close                 = aptx_close,
+    .capabilities          = AV_CODEC_CAP_SMALL_LAST_FRAME,
+    .caps_internal         = FF_CODEC_CAP_INIT_THREADSAFE,
+    .channel_layouts       = (const uint64_t[]) { AV_CH_LAYOUT_STEREO, 0},
+    .sample_fmts           = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S32P,
+                                                             AV_SAMPLE_FMT_NONE },
+    .supported_samplerates = (const int[]) {8000, 16000, 24000, 32000, 44100, 48000, 0},
+};
+#endif
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index c13deb599f..95d164abc1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -634,6 +634,7 @@  enum AVCodecID {
     AV_CODEC_ID_ATRAC3PAL,
     AV_CODEC_ID_DOLBY_E,
     AV_CODEC_ID_APTX,
+    AV_CODEC_ID_APTX_HD,
 
     /* subtitle codecs */
     AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID pointing at the start of subtitle codecs.
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index c3688de1d6..ca18bb2b67 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -2866,6 +2866,13 @@  static const AVCodecDescriptor codec_descriptors[] = {
         .long_name = NULL_IF_CONFIG_SMALL("aptX (Audio Processing Technology for Bluetooth)"),
         .props     = AV_CODEC_PROP_LOSSY,
     },
+    {
+        .id        = AV_CODEC_ID_APTX_HD,
+        .type      = AVMEDIA_TYPE_AUDIO,
+        .name      = "aptx_hd",
+        .long_name = NULL_IF_CONFIG_SMALL("aptX HD (Audio Processing Technology for Bluetooth)"),
+        .props     = AV_CODEC_PROP_LOSSY,
+    },
 
     /* subtitle codecs */
     {