Discussion:
[PATCH 1/8] input.c: fix frame_size check in BUG_ON
Johannes Weißl
2011-03-31 17:20:27 UTC
Permalink
current check works only for frame_size = 2^i, but if multiple
channels are used this is no longer true. The new check is also much
easier to read!
---
input.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/input.c b/input.c
index d3bc9d4..223a5ee 100644
--- a/input.c
+++ b/input.c
@@ -636,7 +636,7 @@ int ip_read(struct input_plugin *ip, char *buffer, int count)
return rc;
}

- BUG_ON((rc & ~((unsigned int)sf_get_frame_size(ip->data.sf) - 1U)) != rc);
+ BUG_ON(rc % sf_get_frame_size(ip->data.sf) != 0);

sample_size = sf_get_sample_size(ip->data.sf);
if (ip->pcm_convert_in_place != NULL)
--
1.7.4.1
Johannes Weißl
2011-03-31 17:20:28 UTC
Permalink
---
oss.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/oss.c b/oss.c
index a9c2223..57dc3c9 100644
--- a/oss.c
+++ b/oss.c
@@ -63,9 +63,16 @@ static int oss_set_sf(sample_format_t sf)

oss_reset();
oss_sf = sf;
+
+#ifdef SNDCTL_DSP_CHANNELS
+ tmp = sf_get_channels(oss_sf);
+ if (ioctl(oss_fd, SNDCTL_DSP_CHANNELS, &tmp) == -1)
+ return -1;
+#else
tmp = sf_get_channels(oss_sf) - 1;
if (ioctl(oss_fd, SNDCTL_DSP_STEREO, &tmp) == -1)
return -1;
+#endif

if (sf_get_bits(oss_sf) == 16) {
if (sf_get_signed(oss_sf)) {
@@ -185,8 +192,7 @@ static int oss_write(const char *buffer, int count)
{
int rc;

- count /= 4;
- count *= 4;
+ count -= count % sf_get_frame_size(oss_sf);
rc = write(oss_fd, buffer, count);
return rc;
}
--
1.7.4.1
Johannes Weißl
2011-03-31 17:20:30 UTC
Permalink
Use bigendian byte order on big endian sytems. Otherwise cmus does up to
three endian conversions (!). Also use byte swapping functions from
utils.h.
---
pcm.c | 19 ++++++++++++-------
player.c | 3 +++
2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/pcm.c b/pcm.c
index 17d9232..0a1ab0a 100644
--- a/pcm.c
+++ b/pcm.c
@@ -1,4 +1,5 @@
#include "pcm.h"
+#include "utils.h"

#include <inttypes.h>
#include <stdlib.h>
@@ -100,21 +101,19 @@ static void convert_u16_be_to_s16_le(char *buf, int count)
uint16_t u = b[i];
int sample;

- u = (u << 8) | (u >> 8);
+ u = swap_uint16(u);
sample = (int)u - 32768;
b[i] = sample;
}
}

-static void convert_s16_be_to_s16_le(char *buf, int count)
+static void swap_s16_byte_order(char *buf, int count)
{
int16_t *b = (int16_t *)buf;
int i;

- for (i = 0; i < count; i++) {
- uint16_t sample = b[i];
- b[i] = (sample << 8) | (sample >> 8);
- }
+ for (i = 0; i < count; i++)
+ b[i] = swap_uint16(b[i]);
}

static void convert_16_1ch_to_16_2ch(char *dst, const char *src, int count)
@@ -151,6 +150,12 @@ pcm_conv_in_place_func pcm_conv_in_place[8] = {

convert_u16_le_to_s16_le,
convert_u16_be_to_s16_le,
+
+#ifdef WORDS_BIGENDIAN
+ swap_s16_byte_order,
+ NULL,
+#else
NULL,
- convert_s16_be_to_s16_le
+ swap_s16_byte_order,
+#endif
};
diff --git a/player.c b/player.c
index e633656..573a49a 100644
--- a/player.c
+++ b/player.c
@@ -141,6 +141,9 @@ static void set_buffer_sf(sample_format_t sf)
if (sf_get_channels(buffer_sf) <= 2 && sf_get_bits(buffer_sf) <= 16) {
buffer_sf &= SF_RATE_MASK;
buffer_sf |= sf_channels(2) | sf_bits(16) | sf_signed(1);
+#ifdef WORDS_BIGENDIAN
+ buffer_sf |= sf_bigendian(1);
+#endif
}
}
--
1.7.4.1
Johannes Weißl
2011-03-31 17:20:31 UTC
Permalink
Tremor always returns samples in native byte order. This leads to
non-playable vorbis on big-endian systems because the sample format is
marked as little-endian. This patch fixes this and synchronizes
the behaviour with libvorbisfile.
---
vorbis.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/vorbis.c b/vorbis.c
index 277acfe..87dded6 100644
--- a/vorbis.c
+++ b/vorbis.c
@@ -123,6 +123,9 @@ static int vorbis_open(struct input_plugin_data *ip_data)

vi = ov_info(&priv->vf, -1);
ip_data->sf = sf_rate(vi->rate) | sf_channels(vi->channels) | sf_bits(16) | sf_signed(1);
+#ifdef WORDS_BIGENDIAN
+ ip_data->sf |= sf_bigendian(1);
+#endif
return 0;
}

@@ -142,6 +145,15 @@ static int vorbis_close(struct input_plugin_data *ip_data)
return 0;
}

+static inline int vorbis_endian(void)
+{
+#ifdef WORDS_BIGENDIAN
+ return 1;
+#else
+ return 0;
+#endif
+}
+
/*
* OV_HOLE
* indicates there was an interruption in the data.
@@ -168,7 +180,7 @@ static int vorbis_read(struct input_plugin_data *ip_data, char *buffer, int coun
/* Tremor can only handle signed 16 bit data */
rc = ov_read(&priv->vf, buffer, count, &current_section);
#else
- rc = ov_read(&priv->vf, buffer, count, 0, 2, 1, &current_section);
+ rc = ov_read(&priv->vf, buffer, count, vorbis_endian(), 2, 1, &current_section);
#endif

if (ip_data->remote && current_section != priv->current_section) {
--
1.7.4.1
Johannes Weißl
2011-03-31 17:20:29 UTC
Permalink
Non-blocking use of OSS is discouraged and doesn't work on systems like
OpenWRT.

http://manuals.opensound.com/developer/SNDCTL_DSP_GETOSPACE.html
---
oss.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/oss.c b/oss.c
index 57dc3c9..70d439c 100644
--- a/oss.c
+++ b/oss.c
@@ -216,7 +216,7 @@ static int oss_buffer_space(void)

if (ioctl(oss_fd, SNDCTL_DSP_GETOSPACE, &info) == -1)
return -1;
- space = info.bytes;
+ space = (info.fragments - 1) * info.fragsize;
return space;
}
--
1.7.4.1
Johannes Weißl
2011-03-31 17:20:32 UTC
Permalink
mikmod uses native byte order
---
mikmod.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/mikmod.c b/mikmod.c
index 4e956f5..1032944 100644
--- a/mikmod.c
+++ b/mikmod.c
@@ -74,6 +74,9 @@ static int mik_open(struct input_plugin_data *ip_data)

ip_data->private = priv;
ip_data->sf = sf_bits(16) | sf_rate(44100) | sf_channels(2) | sf_signed(1);
+#ifdef WORDS_BIGENDIAN
+ ip_data->sf |= sf_bigendian(1);
+#endif
return 0;
}
--
1.7.4.1
Johannes Weißl
2011-03-31 17:20:33 UTC
Permalink
modplug uses native byte order
---
modplug.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/modplug.c b/modplug.c
index f7266da..a520d01 100644
--- a/modplug.c
+++ b/modplug.c
@@ -89,6 +89,9 @@ static int mod_open(struct input_plugin_data *ip_data)

ip_data->private = priv;
ip_data->sf = sf_bits(16) | sf_rate(44100) | sf_channels(2) | sf_signed(1);
+#ifdef WORDS_BIGENDIAN
+ ip_data->sf |= sf_bigendian(1);
+#endif
return 0;
}
--
1.7.4.1
Johannes Weißl
2011-03-31 17:20:34 UTC
Permalink
ffmpeg uses native byte order
---
ffmpeg.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 6d16d20..811c884 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -250,6 +250,9 @@ static int ffmpeg_open(struct input_plugin_data *ip_data)
ip_data->sf |= sf_bits(16) | sf_signed(1);
break;
}
+#ifdef WORDS_BIGENDIAN
+ ip_data->sf |= sf_bigendian(1);
+#endif
return 0;
}
--
1.7.4.1
Johannes Weißl
2011-03-31 21:50:30 UTC
Permalink
E.g. in OpenWrt libiconv is an optional package.
---
configure | 1 +
convert.c | 13 +++++++++++++
scripts/checks.sh | 37 +++++++++++++++++++++++++++++++++----
ui_curses.c | 11 +++++++++++
4 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index daf6f10..96d0cdb 100755
--- a/configure
+++ b/configure
@@ -407,6 +407,7 @@ config_header config/mp4.h USE_MPEG4IP
config_header config/curses.h HAVE_RESIZETERM HAVE_USE_DEFAULT_COLORS
config_header config/ffmpeg.h USE_FALLBACK_IP
config_header config/utils.h HAVE_BYTESWAP_H
+config_header config/iconv.h HAVE_ICONV

makefile_vars bindir datadir libdir mandir exampledir
makefile_vars CONFIG_FLAC CONFIG_MAD CONFIG_MIKMOD CONFIG_MODPLUG CONFIG_MPC CONFIG_VORBIS CONFIG_WAVPACK CONFIG_WAV CONFIG_MP4 CONFIG_AAC CONFIG_FFMPEG
diff --git a/convert.c b/convert.c
index 27a266e..f372fb5 100644
--- a/convert.c
+++ b/convert.c
@@ -19,8 +19,11 @@

#include "convert.h"
#include "xmalloc.h"
+#include "config/iconv.h"

+#ifdef HAVE_ICONV
#include <iconv.h>
+#endif
#include <string.h>
#include <errno.h>

@@ -28,6 +31,7 @@ ssize_t convert(const char *inbuf, ssize_t inbuf_size,
char **outbuf, ssize_t outbuf_estimate,
const char *tocode, const char *fromcode)
{
+#ifdef HAVE_ICONV
const char *in;
char *out;
size_t outbuf_size, inbytesleft, outbytesleft;
@@ -78,6 +82,15 @@ error:
iconv_close(cd);
errno = err_save;
return -1;
+
+#else
+ if (inbuf_size < 0)
+ inbuf_size = strlen(inbuf);
+ *outbuf = xnew(char, inbuf_size + 1);
+ memcpy(*outbuf, inbuf, inbuf_size);
+ (*outbuf)[inbuf_size] = '\0';
+ return inbuf_size;
+#endif
}

int utf8_encode(const char *inbuf, const char *encoding, char **outbuf)
diff --git a/scripts/checks.sh b/scripts/checks.sh
index e6e1d67..c2f048f 100644
--- a/scripts/checks.sh
+++ b/scripts/checks.sh
@@ -670,10 +670,39 @@ check_dl()
# adds ICONV_CFLAGS and ICONV_LIBS to config.mk
check_iconv()
{
- check_library ICONV "" "-liconv" && return 0
+ HAVE_ICONV=n
+ if check_library ICONV "" "-liconv"
+ then
+ echo "taking iconv from libiconv"
+ else
+ echo "assuming libc contains iconv"
+ makefile_var ICONV_CFLAGS ""
+ makefile_var ICONV_LIBS ""
+ fi
+ msg_checking "for working iconv"
+ if try_compile_link '
+#include <stdio.h>
+#include <string.h>
+#include <iconv.h>
+int main(int argc, char *argv[]) {
+ char buf[128], *out = buf, *in = argv[1];
+ size_t outleft = 127, inleft = strlen(in);
+ iconv_t cd = iconv_open("UTF-8", "ISO-8859-1");
+ iconv(cd, &in, &inleft, &out, &outleft);
+ *out = 0;
+ printf("%s", buf);
+ iconv_close(cd);
+ return 0;
+}' $ICONV_CFLAGS $ICONV_LIBS
+ then
+ msg_result "yes"
+ HAVE_ICONV=y
+ else
+ msg_result "no"
+ msg_error "Your system doesn't have iconv!"
+ msg_error "This means that no charset conversion can be done, so all"
+ msg_error "your tracks need to be encoded in your system charset!"
+ fi

- echo "assuming libc contains iconv"
- makefile_var ICONV_CFLAGS ""
- makefile_var ICONV_LIBS ""
return 0
}
diff --git a/ui_curses.c b/ui_curses.c
index 217ed41..50a0df5 100644
--- a/ui_curses.c
+++ b/ui_curses.c
@@ -47,6 +47,7 @@
#include "input.h"
#include "file.h"
#include "config/curses.h"
+#include "config/iconv.h"

#include <unistd.h>
#include <stdlib.h>
@@ -57,7 +58,9 @@
#include <dirent.h>
#include <locale.h>
#include <langinfo.h>
+#ifdef HAVE_ICONV
#include <iconv.h>
+#endif
#include <signal.h>
#include <stdarg.h>
#include <math.h>
@@ -288,6 +291,7 @@ int track_format_valid(const char *format)

static void utf8_encode(const char *buffer)
{
+#ifdef HAVE_ICONV
static iconv_t cd = (iconv_t)-1;
size_t is, os;
const char *i;
@@ -312,10 +316,14 @@ static void utf8_encode(const char *buffer)
d_print("iconv failed: %s\n", strerror(errno));
return;
}
+#else
+ strcpy(conv_buffer, buffer);
+#endif
}

static void utf8_decode(const char *buffer)
{
+#ifdef HAVE_ICONV
static iconv_t cd = (iconv_t)-1;
size_t is, os;
const char *i;
@@ -340,6 +348,9 @@ static void utf8_decode(const char *buffer)
d_print("iconv failed: %s\n", strerror(errno));
return;
}
+#else
+ strcpy(conv_buffer, buffer);
+#endif
}

/* screen updates {{{ */
--
1.7.4.1
Johannes Weißl
2011-03-31 23:01:44 UTC
Permalink
app_config is dangerous because unlike pkg-config, the program from the
host system is used when cross-compiling.
---
configure | 22 +++++++++++++++++++---
1 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index daf6f10..63d8704 100755
--- a/configure
+++ b/configure
@@ -152,11 +152,20 @@ check_mad()
return $?
}

+mikmod_code="
+#include <mikmod.h>
+int main() {
+ MikMod_RegisterAllDrivers();
+ return 0;
+}
+"
check_mikmod()
{
- app_config MIKMOD libmikmod-config && return 0
# mikmod is linked against pthread
- check_library MIKMOD "$PTHREAD_CFLAGS" "-lmikmod $PTHREAD_LIBS"
+ app_config MIKMOD libmikmod-config || \
+ check_library MIKMOD "$PTHREAD_CFLAGS" "-lmikmod $PTHREAD_LIBS" || \
+ return 1
+ try_compile_link "$mikmod_code" $MIKMOD_CFLAGS $MIKMOD_LIBS
return $?
}

@@ -203,9 +212,16 @@ check_ao()
return $?
}

+arts_code="
+#include <artsc.h>
+int main() {
+ return arts_init();
+}
+"
check_arts()
{
- app_config ARTS artsc-config
+ app_config ARTS artsc-config || return 1
+ try_compile_link "$arts_code" $ARTS_CFLAGS $ARTS_LIBS
return $?
}
--
1.7.4.1
Johannes Weißl
2011-04-01 07:56:03 UTC
Permalink
Leads to build error on SunOS 5.11
---
compiler.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/compiler.h b/compiler.h
index 279ed4a..3730fcd 100644
--- a/compiler.h
+++ b/compiler.h
@@ -12,8 +12,10 @@
*/
#if defined(__GNUC__)

+#if __GNUC__ > 3
#undef offsetof
#define offsetof(type, member) __builtin_offsetof(type, member)
+#endif

/* Optimization: Condition @x is likely */
#define likely(x) __builtin_expect(!!(x), 1)
--
1.7.4.1
Johannes Weißl
2011-04-01 23:59:58 UTC
Permalink
---
ffmpeg.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index 811c884..b5b9932 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -44,8 +44,10 @@
#define AV_SAMPLE_FMT_S16 SAMPLE_FMT_S16
#define AV_SAMPLE_FMT_S32 SAMPLE_FMT_S32
#define AV_SAMPLE_FMT_FLT SAMPLE_FMT_FLT
+#if (LIBAVCODEC_VERSION_INT > ((51<<16)+(64<<8)+0))
#define AV_SAMPLE_FMT_DBL SAMPLE_FMT_DBL
#endif
+#endif

struct ffmpeg_input {
AVPacket pkt;
@@ -209,7 +211,11 @@ static int ffmpeg_open(struct input_plugin_data *ip_data)
break;
}

+#if (LIBAVCODEC_VERSION_INT > ((51<<16)+(64<<8)+0))
if (cc->sample_fmt == AV_SAMPLE_FMT_FLT || cc->sample_fmt == AV_SAMPLE_FMT_DBL) {
+#else
+ if (cc->sample_fmt == AV_SAMPLE_FMT_FLT) {
+#endif
err = -IP_ERROR_SAMPLE_FORMAT;
break;
}
--
1.7.4.1
Gregory Petrosyan
2011-04-02 08:33:06 UTC
Permalink
Post by Johannes Weißl
---
ffmpeg.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
Thanks, merged to master and maint!

Gregory

Loading...