[FFmpeg-devel] Review request - ra288.{c,h} ra144.{c,h}
Michael Niedermayer
michaelni
Sun Sep 7 00:22:39 CEST 2008
On Fri, Sep 05, 2008 at 12:23:58AM +0200, Vitor Sessak wrote:
> Vitor Sessak wrote:
> > Hi,
> >
> > Those four files never passed a review. I've just finished cleaning them
> > up, so if anyone wants to review them (Michael already said he will),
> > now is time.
>
> I think now they can go through another review cycle.
ok ra144 reveiw below, i also think that this is my last ra144
review pass, i dont think id find much more ...
[...]
> typedef struct {
> unsigned int old_energy; ///< previous frame energy
>
> unsigned int lpc_tables[2][10];
>
> /** LPC coefficients: lpc_coef[0] is the coefficients of the current frame
> * and lpc_coef[1] of the previous one */
> unsigned int *lpc_coef[2];
>
> unsigned int lpc_refl_rms[2];
>
> /** the current subblock padded by the last 10 values of the previous one*/
> int16_t curr_sblock[50];
>
> /** adaptive codebook. Its size is two units bigger to avoid a
> * buffer overflow */
> uint16_t adapt_cb[148];
this could be written as 146+2
> } RA144Context;
>
> static int ra144_decode_init(AVCodecContext * avctx)
av_cold
[...]
> /**
> * Copy the last offset values of *source to *target. If those values are not
> * enough to fill the target buffer, fill it with another copy of those values.
> */
> static void copy_and_dup(int16_t *target, const int16_t *source, int offset)
> {
> source += BUFFERSIZE - offset;
>
> if (offset > BLOCKSIZE) {
> memcpy(target, source, BLOCKSIZE*sizeof(*target));
> } else {
> memcpy(target, source, offset*sizeof(*target));
> memcpy(target + offset, source, (BLOCKSIZE - offset)*sizeof(*target));
> }
> }
iam not sure which is best but there are a few alternative ways to write this
possibly the current one is best (fastest) though
memcpy(target, source, FFMIN(BLOCKSIZE, offset)*sizeof(*target));
if (offset < BLOCKSIZE)
memcpy(target + offset, source, (BLOCKSIZE - offset)*sizeof(*target));
or
size= FFMIN(BLOCKSIZE, offset)
memcpy(target , source, size *sizeof(*target));
memcpy(target + offset, source, (BLOCKSIZE - size)*sizeof(*target));
[...]
> static int interp(RA144Context *ractx, int16_t *out, int block_num,
> int copyold, int energy)
it is slightly ugly that the 2 input lpc coeff vectors are not passed as
aruments but taken from the context though iam not sure if it would be a
good idea to add 2 more arguments ...
> {
> int work[10];
> int a = block_num + 1;
if a is passed as argument then block_num is unneeded.
--
> /* 14.4 data tables */
> static const int16_t gain_val_tab[256][3] = {
doxygen
> {541, 956, 768}, {877, 581, 568}, {675, 787, 635}, {624, 732, 668},
> {623, 839, 697}, {640, 693, 991}, {925, 687, 608}, {552, 797, 572},
> {535, 832, 799}, {762, 605, 577}, {832, 561, 1003}, {590, 687, 588},
> {646, 901, 732}, {828, 689, 896}, {875, 624, 848}, {571, 942, 1022},
> {824, 736, 643}, {517, 765, 512}, {562, 908, 761}, {694, 913, 675},
> {704, 524, 672}, {721, 757, 558}, {884, 551, 633}, {558, 1007, 846},
> {932, 746, 777}, {566, 822, 926}, {613, 771, 611}, {737, 671, 1008},
> {651, 594, 579}, {801, 636, 564}, {852, 910, 719}, {998, 614, 575},
> {665, 935, 628}, {631, 596, 829}, {644, 926, 526}, {879, 988, 613},
> {941, 692, 693}, {565, 672, 576}, {547, 628, 740}, {639, 532, 537},
> {955, 604, 598}, {562, 580, 900}, {603, 899, 621}, {746, 533, 624},
> {729, 514, 735}, {853, 551, 692}, {949, 1018, 1004}, {544, 988, 735},
> {789, 782, 821}, {897, 516, 754}, {517, 702, 828}, {586, 818, 763},
> {907, 652, 592}, {528, 652, 642}, {531, 708, 780}, {666, 625, 727},
> {947, 727, 554}, {549, 657, 981}, {605, 920, 852}, {624, 619, 983},
> {605, 909, 547}, {690, 935, 516}, {700, 612, 853}, {767, 832, 574},
> {523, 898, 923}, {722, 958, 691}, {613, 771, 928}, {758, 757, 584},
> {512, 567, 577}, {615, 638, 698}, {574, 642, 589}, {993, 682, 878},
> {539, 890, 913}, {694, 928, 544}, {805, 600, 680}, {540, 951, 782},
> {816, 950, 590}, {955, 847, 811}, {547, 883, 556}, {652, 888, 604},
> {863, 585, 855}, {1023, 997, 516}, {932, 614, 640}, {627, 564, 573},
> {876, 900, 724}, {515, 857, 896}, {647, 953, 879}, {806, 854, 857},
> {545, 583, 631}, {657, 601, 751}, {740, 905, 795}, {841, 1016, 568},
> {747, 589, 983}, {878, 613, 526}, {864, 723, 779}, {534, 674, 774},
> {950, 649, 939}, {590, 703, 899}, {618, 527, 579}, {725, 647, 972},
> {641, 647, 707}, {730, 663, 644}, {807, 572, 578}, {879, 611, 821},
> {667, 729, 841}, {782, 585, 751}, {802, 733, 976}, {850, 871, 708},
> {870, 743, 704}, {941, 899, 585}, {943, 632, 875}, {1023, 732, 638},
> {778, 753, 655}, {843, 945, 945}, {942, 969, 572}, {1008, 559, 854},
> {868, 729, 787}, {970, 686, 547}, {535, 635, 674}, {560, 636, 828},
> {994, 592, 833}, {548, 621, 694}, {550, 801, 955}, {582, 522, 646},
> {606, 625, 818}, {623, 591, 874}, {669, 535, 1001}, {701, 938, 592},
> {925, 820, 738}, {735, 790, 544}, {575, 788, 674}, {655, 783, 528},
> {527, 513, 677}, {782, 852, 940}, {578, 910, 513}, {692, 882, 734},
> {586, 683, 715}, {739, 609, 717}, {778, 773, 697}, {922, 785, 813},
> {766, 651, 984}, {978, 596, 515}, {535, 757, 540}, {662, 687, 589},
> {554, 536, 979}, {723, 982, 690}, {936, 956, 527}, {590, 1002, 547},
> {517, 653, 825}, {832, 592, 974}, {512, 957, 903}, {631, 545, 906},
> {514, 720, 649}, {596, 679, 694}, {617, 740, 979}, {711, 685, 877},
> {655, 835, 848}, {754, 839, 698}, {871, 515, 769}, {955, 852, 573},
> {640, 859, 587}, {792, 863, 554}, {843, 708, 682}, {971, 768, 552},
> {891, 536, 690}, {1016, 560, 663}, {543, 870, 674}, {601, 999, 585},
> {945, 966, 889}, {529, 912, 777}, {574, 1020, 714}, {609, 922, 932},
> {598, 778, 929}, {651, 772, 744}, {691, 957, 722}, {729, 766, 984},
> {547, 519, 632}, {583, 532, 922}, {633, 995, 603}, {677, 571, 874},
> {602, 545, 666}, {627, 542, 875}, {672, 983, 598}, {692, 979, 730},
> {668, 634, 872}, {711, 706, 674}, {739, 977, 595}, {759, 905, 763},
> {756, 582, 763}, {748, 1013, 908}, {804, 937, 950}, {785, 543, 998},
> {999, 684, 942}, {626, 633, 996}, {626, 567, 835}, {739, 571, 973},
> {655, 769, 707}, {702, 952, 571}, {727, 712, 514}, {744, 686, 741},
> {731, 552, 714}, {824, 991, 726}, {795, 615, 544}, {870, 575, 824},
> {803, 832, 923}, {819, 839, 531}, {887, 786, 852}, {933, 764, 570},
> {716, 906, 654}, {784, 804, 563}, {774, 535, 876}, {807, 598, 649},
> {817, 759, 718}, {831, 993, 846}, {858, 567, 605}, {876, 1012, 651},
> {852, 548, 549}, {895, 1008, 871}, {892, 1000, 591}, {935, 516, 836},
> {931, 612, 776}, {968, 614, 816}, {524, 777, 719}, {549, 694, 786},
> {882, 754, 534}, {597, 837, 766}, {635, 954, 704}, {803, 550, 798},
> {699, 654, 798}, {924, 767, 738}, {970, 675, 608}, {632, 706, 684},
> {858, 767, 563}, {527, 765, 702}, {559, 924, 1003}, {618, 524, 611},
> {999, 942, 963}, {547, 857, 935}, {734, 926, 569}, {967, 746, 551},
> {834, 633, 881}, {941, 701, 727}, {945, 564, 636}, {512, 563, 793},
> {984, 556, 570}, {984, 540, 740}, {527, 764, 874}, {530, 664, 1014},
> {546, 515, 521}, {554, 934, 672}, {598, 945, 556}, {627, 531, 733},
> {576, 1020, 1014}, {623, 924, 594}, {678, 909, 603}, {814, 744, 543}
> };
>
> static const uint8_t gain_exp_tab[256][3] = {
> {14, 14, 14}, {14, 14, 14}, {14, 13, 14}, {13, 13, 14},
> {13, 14, 13}, {13, 14, 15}, {13, 13, 13}, {12, 14, 13},
> {13, 13, 13}, {13, 13, 12}, {13, 12, 13}, {12, 13, 12},
> {12, 13, 13}, {12, 13, 13}, {12, 12, 13}, {11, 13, 13},
> {13, 12, 13}, {12, 12, 12}, {13, 12, 12}, {13, 12, 11},
> {12, 12, 12}, {12, 13, 11}, {12, 12, 11}, {11, 13, 12},
> {12, 12, 12}, {11, 12, 12}, {11, 12, 12}, {11, 12, 13},
> {11, 12, 11}, {11, 12, 11}, {11, 13, 12}, {11, 12, 12},
> {12, 12, 12}, {12, 11, 12}, {12, 12, 11}, {12, 12, 11},
> {13, 11, 11}, {12, 11, 10}, {11, 11, 11}, {11, 11, 10},
> {12, 11, 12}, {11, 11, 12}, {11, 12, 11}, {11, 11, 11},
> {11, 11, 12}, {11, 11, 12}, {11, 12, 12}, {10, 12, 12},
> {11, 12, 11}, {11, 11, 11}, {10, 12, 11}, {10, 12, 11},
> {11, 11, 11}, {10, 11, 11}, {10, 11, 12}, {10, 11, 12},
> {11, 12, 11}, {10, 12, 12}, {10, 13, 12}, {10, 12, 13},
> {10, 12, 11}, {10, 12, 11}, {10, 12, 12}, {10, 12, 12},
> {12, 11, 12}, {12, 11, 11}, {11, 11, 12}, {11, 11, 11},
> {11, 10, 11}, {11, 10, 11}, {12, 10, 10}, {12, 10, 10},
> {11, 11, 11}, {11, 11, 10}, {11, 11, 10}, {10, 12, 10},
> {11, 11, 11}, {11, 11, 11}, {10, 11, 11}, {10, 11, 11},
> {11, 10, 11}, {11, 11, 10}, {11, 10, 10}, {10, 10, 10},
> {11, 11, 10}, {10, 11, 10}, {10, 11, 10}, {10, 11, 10},
> {10, 10, 11}, {10, 10, 11}, {10, 11, 11}, {10, 11, 11},
> {10, 10, 11}, {10, 10, 10}, {10, 10, 11}, { 9, 10, 11},
> {11, 11, 11}, {10, 11, 11}, {10, 11, 10}, {10, 11, 11},
> {10, 11, 11}, {10, 11, 11}, {10, 11, 11}, {10, 11, 12},
> {10, 12, 11}, {10, 12, 11}, {10, 12, 12}, {10, 13, 12},
> {10, 12, 11}, {10, 12, 11}, {10, 12, 12}, {10, 12, 12},
> {10, 12, 10}, {10, 12, 11}, {10, 12, 10}, {10, 11, 11},
> {10, 11, 11}, {10, 11, 11}, { 9, 11, 11}, { 9, 11, 12},
> {10, 12, 11}, { 9, 12, 11}, { 9, 12, 12}, { 9, 12, 12},
> { 9, 12, 11}, { 9, 12, 12}, { 9, 12, 12}, { 9, 13, 12},
> {12, 10, 11}, {11, 10, 10}, {10, 10, 11}, {10, 10, 10},
> {11, 9, 10}, {11, 10, 10}, {10, 10, 9}, {10, 10, 9},
> {10, 10, 10}, {10, 10, 10}, {10, 10, 10}, {10, 10, 10},
> {10, 10, 10}, {10, 10, 9}, { 9, 10, 9}, { 9, 10, 9},
> {10, 9, 11}, {10, 10, 10}, {10, 10, 10}, { 9, 10, 10},
> {10, 9, 10}, {10, 9, 10}, { 9, 10, 10}, { 9, 9, 10},
> { 9, 10, 10}, { 9, 10, 10}, { 9, 10, 11}, { 9, 10, 11},
> { 9, 10, 10}, { 9, 10, 10}, { 9, 9, 10}, { 9, 10, 10},
> {10, 11, 10}, {10, 11, 10}, {10, 11, 10}, {10, 11, 10},
> {10, 10, 10}, {10, 10, 10}, { 9, 11, 10}, { 9, 11, 10},
> {10, 11, 11}, { 9, 11, 11}, { 9, 11, 11}, { 9, 11, 12},
> { 9, 11, 11}, { 9, 11, 11}, { 9, 11, 11}, { 9, 11, 12},
> { 9, 11, 10}, { 9, 11, 11}, { 9, 12, 10}, { 9, 11, 11},
> { 9, 11, 11}, { 9, 11, 12}, { 9, 12, 11}, { 9, 12, 12},
> { 9, 12, 11}, { 9, 12, 11}, { 9, 12, 11}, { 9, 12, 12},
> { 9, 12, 11}, { 9, 13, 12}, { 9, 13, 12}, { 9, 12, 13},
> {10, 11, 10}, { 9, 11, 10}, { 9, 10, 10}, { 9, 10, 10},
> { 9, 11, 10}, { 9, 11, 10}, { 9, 11, 10}, { 9, 11, 11},
> { 9, 10, 10}, { 9, 11, 10}, { 9, 10, 10}, { 9, 10, 11},
> { 9, 11, 11}, { 9, 11, 11}, { 9, 11, 11}, { 9, 11, 11},
> { 9, 12, 10}, { 9, 12, 10}, { 9, 11, 11}, { 9, 11, 11},
> { 9, 12, 11}, { 9, 12, 12}, { 9, 12, 11}, { 9, 13, 12},
> { 9, 11, 10}, { 9, 12, 11}, { 9, 12, 11}, { 9, 11, 12},
> { 9, 12, 11}, { 9, 12, 12}, { 8, 12, 11}, { 8, 12, 12},
> {10, 9, 9}, { 9, 9, 9}, { 9, 10, 9}, { 9, 9, 9},
> { 9, 9, 10}, { 9, 9, 10}, { 9, 9, 9}, { 8, 9, 9},
> { 9, 10, 9}, { 8, 10, 9}, { 8, 10, 10}, { 8, 9, 10},
> { 9, 9, 9}, { 7, 8, 8}, { 8, 10, 9}, { 8, 9, 9},
> { 9, 11, 10}, { 9, 11, 10}, { 9, 10, 10}, { 8, 10, 11},
> { 9, 11, 10}, { 9, 11, 11}, { 8, 11, 11}, { 8, 11, 12},
> { 8, 10, 9}, { 8, 11, 10}, { 8, 11, 10}, { 8, 10, 11},
> { 8, 12, 11}, { 8, 12, 11}, { 8, 11, 10}, { 8, 11, 10}
> };
maybe it would be possible to normalize the tripples so that
each just needs one shift value
that would safe 512 byte and simplify the code to
v[i] = (gain_val_tab[n][i] * m[i]) >> gain_exp_tab[n];
though ive not verified that all would fit in 16bit.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080907/458ea500/attachment.pgp>
More information about the ffmpeg-devel
mailing list