On Sat, Jul 05, 2014 at 07:32:18PM +0100, bifferos wrote: > Here's a mixer applet (OSS). It gets the list of mixer devices, gets and > sets individual device values.
First, thanks! I'm interested, for one. Second, it would be much easier to try out with the kconfig data included. (A patch to add that to mixer.c is attached; apply with -p2 if you don't have mixer.c in miscutils/) Third, did you try checking size with "make bloatcheck"? This shows the breakdown in how much is getting added by what functions/data. On my current system (Alpine Linux, musl libc, GCC 4.8.2), I get this: function old new delta mixer_main - 503 +503 .rodata 135904 136174 +270 packed_usage 29585 29663 +78 pr_level_exit - 55 +55 applet_names 2451 2457 +6 applet_main 1428 1432 +4 applet_nameofs 714 716 +2 ------------------------------------------------------------------------------ (add/remove: 3/0 grow/shrink: 5/0 up/down: 918/0) Total: 918 bytes > / # mixer --help > BusyBox v1.22.1 (2014-06-23 15:19:41 BST) multi-call binary. > > Usage: mixer [-d DEVICE] [MIXER [LEVEL]] > > Sets the mixer volume at a specific level (0-100) > > -d DEVICE use given mixer device (default /dev/mixer) > > / # mixer > Mixer: speaker mic phout > > / # mixer speaker > Level (L/R): 99/99 > > / # mixer speaker 20 > Level (L/R): 20/20 > > It adds about about 2.6k to Busybox and I don't really understand why. Even > with the list of mixers it should only be ~500 bytes. If anyone has any > suggestions I can try to fix that. I'm seeing 918 bytes (gcc 4.8, musl libc). It can be reduced at least 7 bytes from that, and perhaps 11 more to 900 bytes. > regards, > Biff. > /* vi: set sw=4 ts=4: */ > /* > * Audio mixer for busybox (OSS emulation only) > * > * Copyright (c) Bifferos ([email protected]) > * > * Licensed under GPLv2 or later, see file LICENSE in this source tree. > * > * Setting L/R stereo channels independently is unsupported in order to keep > this applet small. > * > * > */ > > > #include <linux/soundcard.h> > #include "libbb.h" > > //usage:#define mixer_trivial_usage > //usage: "[-d DEVICE] [MIXER [LEVEL]]" > //usage:#define mixer_full_usage "\n\n" > //usage: "Sets the mixer volume at a specific level (0-100)\n" > //usage: "\n -d DEVICE use given mixer device (default > /dev/mixer)" > //usage:#define mixer_example_usage > //usage: "# mixer vol 50\n" > Here, add: static void pr_level_exit(int level) NORETURN; This will let you drop the "return 0;" at the end, and save ~3 bytes. > static void pr_level_exit(int level) > { > printf("Level (L/R): %d/%d\n", level&0xff, ((level&0xff00) >> 8)); > exit(0); > } > > > int mixer_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE; > int mixer_main(int argc, char **argv ) > { > int fd_mixer; > unsigned opts, extra_args; > char* device; > char* mixer = 0; > uint32_t write_level; > uint32_t read_level; You only use read_level or write_level, and both are unconditionally initialized when the code reaches them. So you can change use "uint32_t level;" instead. (-4 bytes) > int devmask, i; > const char *m_names[SOUND_MIXER_NRDEVICES] = SOUND_DEVICE_NAMES ; > int mixer_device=0; > > > /* Option handling */ > > opt_complementary = "?2"; /* must have no more than 2 */ > opts = getopt32(argv, "d:", &device); > > extra_args = argc - 1; I'm wondering why you don't do typical *argv++, argc--. Is this smaller? > if (opts & 1) > { > fd_mixer = xopen(device, O_RDWR); > extra_args -= 2; > } > else > { > fd_mixer = xopen("/dev/mixer", O_RDWR); > } > > if (extra_args) mixer = argv[argc - ((extra_args==1)?1:2)]; With argc and argv being manipulated, this is literally: if (argc--) mixer = argv[argc - (argc?0:1)]; But my understanding is that extra_args could not be larger than 2 here, due to the previous line: opt_complementary="?2"; Which suggests that you could do this: if (argc--) mixer = *argv; or, if you want to keep extra_args, if (extra_args) mixer = argv[argc - extra_args]; If this analysis is correct, the latter is 11 bytes saved. > > /* mixer device enumeration */ > > ioctl_or_perror_and_die(fd_mixer, SOUND_MIXER_READ_DEVMASK, &devmask, > "DEVMASK"); > > if (!mixer) printf("Mixer:"); > > for (i = 0; i < SOUND_MIXER_NRDEVICES; ++i) > { > if ((1 << i) & devmask) { > if (mixer) > { > if (strcmp(m_names[i], mixer) == 0) > mixer_device = i; > } > else > { > printf(" %s", m_names[i]); > } > } > } > > if (!mixer) > { > printf("\n"); > exit(0); > } > > /* mixer device reading */ > > ioctl_or_perror_and_die(fd_mixer, MIXER_READ(mixer_device), > &read_level, "MIXER_READ"); > > if (extra_args == 1) pr_level_exit(read_level); > > > /* mixer device setting */ > > write_level = xatou_range(argv[argc - 1], 0, 100); > write_level = (write_level<<8) + write_level; See the comment about write_level/read_level. > ioctl_or_perror_and_die(fd_mixer, MIXER_WRITE(mixer_device), > &write_level, "MIXER_WRITE"); > > pr_level_exit(write_level); > > > return 0; Can be removed with the NORETURN declaration. > } HTH, Isaac Dunham _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
