New applet proposal: minimp3
Denys Vlasenko
vda.linux at googlemail.com
Fri Sep 10 10:43:43 UTC 2010
with CONFIG_WERROR=y:
cc1: warnings being treated as errors
miscutils/minimp3.c: In function 'get_bits1':
miscutils/minimp3.c:1003: error: declaration of 'index' shadows a
global declaration
/usr/cross/i486-linux-uclibc/usr/include/string.h:317: error: shadowed
declaration is here
miscutils/minimp3.c: In function 'alloc_table':
miscutils/minimp3.c:1035: error: declaration of 'index' shadows a
global declaration
/usr/cross/i486-linux-uclibc/usr/include/string.h:317: error: shadowed
declaration is here
miscutils/minimp3.c: In function 'build_table':
miscutils/minimp3.c:1054: error: declaration of 'index' shadows a
global declaration
/usr/cross/i486-linux-uclibc/usr/include/string.h:317: error: shadowed
declaration is here
miscutils/minimp3.c: In function 'get_vlc2':
miscutils/minimp3.c:1173: error: declaration of 'index' shadows a
global declaration
/usr/cross/i486-linux-uclibc/usr/include/string.h:317: error: shadowed
declaration is here
miscutils/minimp3.c: In function 'compute_antialias':
miscutils/minimp3.c:1326: error: unused parameter 's'
miscutils/minimp3.c: In function 'huffman_decode':
miscutils/minimp3.c:1608: error: declaration of 'pos' shadows a previous local
miscutils/minimp3.c:1584: error: shadowed declaration is here
miscutils/minimp3.c: In function 'compute_imdct':
miscutils/minimp3.c:1754: error: unused parameter 's'
miscutils/minimp3.c: In function 'mp3_synth_filter':
miscutils/minimp3.c:2105: error: declaration of 'window' shadows a
global declaration
miscutils/minimp3.c:275: error: shadowed declaration is here
miscutils/minimp3.c:2111: error: declaration of 'w' shadows a global declaration
miscutils/minimp3.c:44: error: shadowed declaration is here
miscutils/minimp3.c:2152: error: declaration of 'tmp' shadows a previous local
miscutils/minimp3.c:2109: error: shadowed declaration is here
miscutils/minimp3.c:2154: error: declaration of 'tmp' shadows a previous local
miscutils/minimp3.c:2109: error: shadowed declaration is here
miscutils/minimp3.c: In function 'mp3_decode_init':
miscutils/minimp3.c:2636: error: declaration of 'k' shadows a previous local
miscutils/minimp3.c:2537: error: shadowed declaration is here
miscutils/minimp3.c:2535: error: unused parameter 's'
make[1]: *** [miscutils/minimp3.o] Error 1
make bloatcheck:
function old new delta
expval_table - 32768 +32768
static.granules - 9680 +9680
mp_decode_layer3 - 9230 +9230
mp3_decode - 5843 +5843
exp_table - 2048 +2048
mp3_decode_init - 1951 +1951
static.exponents - 1152 +1152
mdct_win - 1152 +1152
mp3_enwindow - 1028 +1028
window - 1024 +1024
minimp3_main - 710 +710
build_table - 582 +582
mp3_huffcodes_24 - 512 +512
mp3_huffcodes_16 - 512 +512
mp3_huffcodes_15 - 512 +512
mp3_huffcodes_13 - 512 +512
__kernel_tan - 455 +455
band_index_long - 414 +414
imdct12 - 361 +361
mp3_huffbits_24 - 256 +256
mp3_huffbits_16 - 256 +256
mp3_huffbits_15 - 256 +256
mp3_huffbits_13 - 256 +256
is_table_lsf - 256 +256
huff_vlc - 256 +256
reorder_block - 211 +211
get_vlc2 - 207 +207
band_size_long - 198 +198
mp3_huff_tables - 192 +192
mp3_huffcodes_12 - 128 +128
mp3_huffcodes_11 - 128 +128
mp3_huffcodes_10 - 128 +128
is_table - 128 +128
csa_table_float - 128 +128
csa_table - 128 +128
frexp - 127 +127
__GI_frexp - 127 +127
switch_buffer - 125 +125
band_size_short - 117 +117
tan - 116 +116
__GI_tan - 116 +116
mp3_huffcodes_9 - 72 +72
mp3_huffcodes_8 - 72 +72
mp3_huffcodes_7 - 72 +72
lsf_nsf_table - 72 +72
lsf_sf_expand - 67 +67
static.idxtab - 64 +64
mp3_huffbits_12 - 64 +64
mp3_huffbits_11 - 64 +64
mp3_huffbits_10 - 64 +64
mp3_huff_data - 64 +64
init_vlc - 63 +63
mp3_bitrate_tab - 60 +60
l3_unscale - 60 +60
mp3_pretab - 44 +44
round_sample - 42 +42
init_get_bits - 41 +41
mp3_huffbits_9 - 36 +36
mp3_huffbits_8 - 36 +36
mp3_huffbits_7 - 36 +36
icos36h - 36 +36
icos36 - 36 +36
get_bits1 - 36 +36
unaligned32_be - 35 +35
slen_table - 32 +32
mp3_quad_codes - 32 +32
mp3_quad_bits - 32 +32
mp3_huffcodes_6 - 32 +32
mp3_huffcodes_5 - 32 +32
mp3_create - 32 +32
huff_quad_vlc - 32 +32
ci_table - 32 +32
align_get_bits - 27 +27
mp3_huffcodes_3 - 18 +18
mp3_huffcodes_2 - 18 +18
mp3_huffbits_6 - 16 +16
mp3_huffbits_5 - 16 +16
applet_names 2306 2322 +16
get_bitsz - 12 +12
mp3_huffbits_3 - 9 +9
mp3_huffbits_2 - 9 +9
mp3_huffcodes_1 - 8 +8
applet_main 1356 1364 +8
mp3_freq_tab - 6 +6
w - 4 +4
table_4_3_value - 4 +4
table_4_3_exp - 4 +4
skip_bits_long - 4 +4
mp3_huffbits_1 - 4 +4
get_bits_count - 4 +4
applet_nameofs 678 682 +4
applet_install_loc 170 171 +1
packed_usage 27311 27288 -23
------------------------------------------------------------------------------
(add/remove: 92/0 grow/shrink: 4/1 up/down: 75970/-23) Total: 75947 bytes
text data bss dec hex filename
857188 438 7540 865166 d338e busybox_old
884282 438 56724 941444 e5d84 busybox_unstripped
This kind of growth is definitely not ok. Especially in bss.
Code:
#define out(text) write(1, (const void *) text, strlen(text))
This should be a function, or use printf.
+ out("minimp3 -- a small MPEG-1 Audio Layer III player based on
ffmpeg\n\n");
Bloat.
+ fd = open(argv[w], O_RDONLY);
+ if (fd < 0) {
+ out("Error: cannot open `");
+ out(argv[w]);
+ out("'!\n");
+ return 1;
+ }
We have xopen() to replace this entire block.
+ mp3 = mp3_create();
+ frame_size = mp3_decode(mp3, stream_pos, bytes_left, sample_buf, &info);
+ if (!frame_size) {
+ out("\nError: not a valid MP3 audio file!\n");
+ return 1;
+ }
bbox has bb_[p]error_msg_and_die, and uses terser messages:
bb_error_msg_and_die("bad MP3 file format")
+ if (ioctl(pcm, SNDCTL_DSP_SETFMT, &value) < 0)
+ FAIL("cannot set audio format");
xioctl().
+struct _granule;
+
+typedef struct _bitstream {
Why leading underscores?
+static vlc_t huff_vlc[16];
+static vlc_t huff_quad_vlc[2];
+static uint16_t band_index_long[9][23];
+#define TABLE_4_3_SIZE (8191 + 16)*4
+static int8_t *table_4_3_exp;
+static uint32_t *table_4_3_value;
+static uint32_t exp_table[512];
+static uint32_t expval_table[512][16];
+static int32_t is_table[2][16];
+static int32_t is_table_lsf[2][2][16];
+static int32_t csa_table[8][4];
+static float csa_table_float[8][4];
+static int32_t mdct_win[8][36];
+static int16_t window[512];
This needs to be malloced. See "G trick" in many files in the tree:
#define G (*ptr_to_globals)
#define INIT_G() do { \
G.foo = 16; \
G.bar = 14; \
} while (0)
...
int baz_main()
{
INIT_G();
+static const uint16_t mp3_huffcodes_9[36] = {
+ 0x0007, 0x0005, 0x0009, 0x000e, 0x000f, 0x0007, 0x0006, 0x0004,
+ 0x0005, 0x0005, 0x0006, 0x0007, 0x0007, 0x0006, 0x0008, 0x0008,
+ 0x0008, 0x0005, 0x000f, 0x0006, 0x0009, 0x000a, 0x0005, 0x0001,
+ 0x000b, 0x0007, 0x0009, 0x0006, 0x0004, 0x0001, 0x000e, 0x0004,
+ 0x0006, 0x0002, 0x0006, 0x0000,
+};
Looks like these can be uint8_t.
+static INLINE unsigned int get_bits(bitstream_t *s, int n){
+ register int tmp;
+ OPEN_READER(re, s)
+ UPDATE_CACHE(re, s)
+ tmp= SHOW_UBITS(re, s, n);
+ LAST_SKIP_BITS(re, s, n)
+ CLOSE_READER(re, s)
+ return tmp;
+}
Isn't it a bit big for inlining?
+ if (dec) free(dec);
No need to check for NULL here.
The indentation and style needs adjusting to bbox style everywhere.
--
vda
More information about the busybox
mailing list