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