[pulseaudio-commits] 5 commits - src/modules src/pulse src/pulsecore
Arun Raghavan
arun at kemper.freedesktop.org
Fri Oct 28 06:49:35 PDT 2011
src/modules/bluetooth/a2dp-codecs.h | 18 ++++++++--------
src/modules/bluetooth/ipc.c | 1
src/modules/bluetooth/ipc.h | 1
src/modules/bluetooth/sbc/sbc.c | 37 +++++++++++++++++++--------------
src/modules/bluetooth/sbc/sbc_tables.h | 6 +++--
src/modules/module-filter-heuristics.c | 23 --------------------
src/modules/module-intended-roles.c | 19 ----------------
src/pulse/proplist.h | 2 -
src/pulsecore/core-util.c | 20 +++++++++++++++++
src/pulsecore/core-util.h | 1
10 files changed, 61 insertions(+), 67 deletions(-)
New commits:
commit 8c0cca79058c2e669a6b02ea503514926e795981
Author: Johan Hedberg <johan.hedberg at intel.com>
Date: Thu Oct 20 15:47:49 2011 +0300
bluetooth: sbc: Reduce for-loop induced indentation in sbc_unpack_frame
diff --git a/src/modules/bluetooth/sbc/sbc.c b/src/modules/bluetooth/sbc/sbc.c
index ad391bd..c5015ab 100644
--- a/src/modules/bluetooth/sbc/sbc.c
+++ b/src/modules/bluetooth/sbc/sbc.c
@@ -493,26 +493,30 @@ static int sbc_unpack_frame(const uint8_t *data, struct sbc_frame *frame,
for (blk = 0; blk < frame->blocks; blk++) {
for (ch = 0; ch < frame->channels; ch++) {
for (sb = 0; sb < frame->subbands; sb++) {
- if (levels[ch][sb] > 0) {
- uint32_t shift =
- frame->scale_factor[ch][sb] +
+ uint32_t shift;
+
+ if (levels[ch][sb] == 0) {
+ frame->sb_sample[blk][ch][sb] = 0;
+ continue;
+ }
+
+ shift = frame->scale_factor[ch][sb] +
1 + SBCDEC_FIXED_EXTRA_BITS;
- audio_sample = 0;
- for (bit = 0; bit < bits[ch][sb]; bit++) {
- if (consumed > len * 8)
- return -1;
- if ((data[consumed >> 3] >> (7 - (consumed & 0x7))) & 0x01)
- audio_sample |= 1 << (bits[ch][sb] - bit - 1);
+ audio_sample = 0;
+ for (bit = 0; bit < bits[ch][sb]; bit++) {
+ if (consumed > len * 8)
+ return -1;
- consumed++;
- }
+ if ((data[consumed >> 3] >> (7 - (consumed & 0x7))) & 0x01)
+ audio_sample |= 1 << (bits[ch][sb] - bit - 1);
- frame->sb_sample[blk][ch][sb] = (int32_t)
- (((((uint64_t) audio_sample << 1) | 1) << shift) /
- levels[ch][sb]) - (1 << shift);
- } else
- frame->sb_sample[blk][ch][sb] = 0;
+ consumed++;
+ }
+
+ frame->sb_sample[blk][ch][sb] = (int32_t)
+ (((((uint64_t) audio_sample << 1) | 1) << shift) /
+ levels[ch][sb]) - (1 << shift);
}
}
}
commit 00602537ba0fa7ab52e1ef6d93c606bc538de254
Author: Siarhei Siamashka <siarhei.siamashka at nokia.com>
Date: Thu Oct 20 15:47:48 2011 +0300
bluetooth: sbc: overflow bugfix and audio decoding quality improvement
The "(((audio_sample << 1) | 1) << frame->scale_factor[ch][sb])"
part of expression
"frame->sb_sample[blk][ch][sb] =
(((audio_sample << 1) | 1) << frame->scale_factor[ch][sb]) /
levels[ch][sb] - (1 << frame->scale_factor[ch][sb])"
in "sbc_unpack_frame" function can sometimes overflow 32-bit signed int.
This problem can be reproduced by first using bitpool 128 and encoding
some random noise data, and then feeding it to sbc decoder. The obvious
thing to do would be to change "audio_sample" variable type to uint32_t.
However the problem is a little bit more complicated. According
to the section "12.6.2 Scale Factors" of A2DP spec:
scalefactor[ch][sb] = pow(2.0, (scale_factor[ch][sb] + 1))
And according to "12.6.4 Reconstruction of the Subband Samples":
sb_sample[blk][ch][sb] = scalefactor[ch][sb] *
((audio_sample[blk][ch][sb]*2.0+1.0) / levels[ch][sb]-1.0);
Hence the current code for calculating "sb_sample[blk][ch][sb]" is
not quite correct, because it loses one least significant bit of
sample data and passes twice smaller sample values to the synthesis
filter (the filter also deviates from the spec to compensate this).
This all has quite a noticeable impact on audio quality. Moreover,
it makes sense to keep a few extra bits of precision here in order
to minimize rounding errors. So the proposed patch introduces a new
SBCDEC_FIXED_EXTRA_BITS constant and uses uint64_t data type
for intermediate calculations in order to safeguard against
overflows. This patch intentionally addresses only the quality
issue, but performance can be also improved later (like replacing
division with multiplication by reciprocal).
Test for the difference of sbc encoding/decoding roundtrip vs.
the original audio file for joint stereo, bitpool 128, 8 subbands
and http://media.xiph.org/sintel/sintel-master-st.flac sample
demonstrates some quality improvement:
=== before ===
--- comparing original / sbc_encoder.exe + sbcdec ---
stddev: 4.64 PSNR: 82.97 bytes:170495708/170496000
=== after ===
--- comparing original / sbc_encoder.exe + sbcdec ---
stddev: 1.95 PSNR: 90.50 bytes:170495708/170496000
diff --git a/src/modules/bluetooth/sbc/sbc.c b/src/modules/bluetooth/sbc/sbc.c
index 77fcc5d..ad391bd 100644
--- a/src/modules/bluetooth/sbc/sbc.c
+++ b/src/modules/bluetooth/sbc/sbc.c
@@ -383,7 +383,7 @@ static int sbc_unpack_frame(const uint8_t *data, struct sbc_frame *frame,
int crc_pos = 0;
int32_t temp;
- int audio_sample;
+ uint32_t audio_sample;
int ch, sb, blk, bit; /* channel, subband, block and bit standard
counters */
int bits[2][8]; /* bits distribution */
@@ -494,6 +494,9 @@ static int sbc_unpack_frame(const uint8_t *data, struct sbc_frame *frame,
for (ch = 0; ch < frame->channels; ch++) {
for (sb = 0; sb < frame->subbands; sb++) {
if (levels[ch][sb] > 0) {
+ uint32_t shift =
+ frame->scale_factor[ch][sb] +
+ 1 + SBCDEC_FIXED_EXTRA_BITS;
audio_sample = 0;
for (bit = 0; bit < bits[ch][sb]; bit++) {
if (consumed > len * 8)
@@ -505,9 +508,9 @@ static int sbc_unpack_frame(const uint8_t *data, struct sbc_frame *frame,
consumed++;
}
- frame->sb_sample[blk][ch][sb] =
- (((audio_sample << 1) | 1) << frame->scale_factor[ch][sb]) /
- levels[ch][sb] - (1 << frame->scale_factor[ch][sb]);
+ frame->sb_sample[blk][ch][sb] = (int32_t)
+ (((((uint64_t) audio_sample << 1) | 1) << shift) /
+ levels[ch][sb]) - (1 << shift);
} else
frame->sb_sample[blk][ch][sb] = 0;
}
diff --git a/src/modules/bluetooth/sbc/sbc_tables.h b/src/modules/bluetooth/sbc/sbc_tables.h
index 28c0d54..25e24e6 100644
--- a/src/modules/bluetooth/sbc/sbc_tables.h
+++ b/src/modules/bluetooth/sbc/sbc_tables.h
@@ -40,11 +40,13 @@ static const int sbc_offset8[4][8] = {
{ -4, 0, 0, 0, 0, 0, 1, 2 }
};
+/* extra bits of precision for the synthesis filter input data */
+#define SBCDEC_FIXED_EXTRA_BITS 2
#define SS4(val) ASR(val, SCALE_SPROTO4_TBL)
#define SS8(val) ASR(val, SCALE_SPROTO8_TBL)
-#define SN4(val) ASR(val, SCALE_NPROTO4_TBL)
-#define SN8(val) ASR(val, SCALE_NPROTO8_TBL)
+#define SN4(val) ASR(val, SCALE_NPROTO4_TBL + 1 + SBCDEC_FIXED_EXTRA_BITS)
+#define SN8(val) ASR(val, SCALE_NPROTO8_TBL + 1 + SBCDEC_FIXED_EXTRA_BITS)
static const int32_t sbc_proto_4_40m0[] = {
SS4(0x00000000), SS4(0xffa6982f), SS4(0xfba93848), SS4(0x0456c7b8),
commit 7aec17c3b619cb370b902157823fab7da0212c17
Author: Marcel Holtmann <marcel at holtmann.org>
Date: Fri Aug 26 11:18:54 2011 -0700
bluetooth: audio: Update license for shared header files
The header files with constants and structures for audio specific
interaction with Pulseaudio are suppose to be under LGPL license.
For some odd reason a2dp-codecs.h ended up being under GPL license
which is against the intention of this being shared and re-used by
non-GPL programs. Fix this now to avoid any future confusion.
diff --git a/src/modules/bluetooth/a2dp-codecs.h b/src/modules/bluetooth/a2dp-codecs.h
index e44634e..51c796a 100644
--- a/src/modules/bluetooth/a2dp-codecs.h
+++ b/src/modules/bluetooth/a2dp-codecs.h
@@ -6,18 +6,18 @@
* Copyright (C) 2004-2010 Marcel Holtmann <marcel at holtmann.org>
*
*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
*
- * This program is distributed in the hope that it will be useful,
+ * This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
*
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*
*/
diff --git a/src/modules/bluetooth/ipc.c b/src/modules/bluetooth/ipc.c
index 1bdad78..669eeef 100644
--- a/src/modules/bluetooth/ipc.c
+++ b/src/modules/bluetooth/ipc.c
@@ -4,6 +4,7 @@
*
* Copyright (C) 2004-2010 Marcel Holtmann <marcel at holtmann.org>
*
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
diff --git a/src/modules/bluetooth/ipc.h b/src/modules/bluetooth/ipc.h
index d69b97e..77c57d3 100644
--- a/src/modules/bluetooth/ipc.h
+++ b/src/modules/bluetooth/ipc.h
@@ -4,6 +4,7 @@
*
* Copyright (C) 2004-2010 Marcel Holtmann <marcel at holtmann.org>
*
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
commit 06fc121eef235d6102793fb69d6b8f36f0162b4a
Author: Arun Raghavan <arun.raghavan at collabora.co.uk>
Date: Thu Oct 27 12:49:18 2011 +0200
core: Add a string list membership check function
This adds a pa_str_in_list() to check for a given string in a
space-separated list of strings. For now, this is merely present to
avoid duplication of role matching code (intended roles can be a
space-separate list) across modules.
diff --git a/src/modules/module-filter-heuristics.c b/src/modules/module-filter-heuristics.c
index 5bb0945..e158b95 100644
--- a/src/modules/module-filter-heuristics.c
+++ b/src/modules/module-filter-heuristics.c
@@ -55,27 +55,6 @@ struct userdata {
*source_output_move_finish_slot;
};
-static pa_bool_t role_match(pa_proplist *proplist, const char *role) {
- const char *ir;
- char *r;
- const char *state = NULL;
-
- if (!(ir = pa_proplist_gets(proplist, PA_PROP_DEVICE_INTENDED_ROLES)))
- return FALSE;
-
- while ((r = pa_split_spaces(ir, &state))) {
-
- if (pa_streq(role, r)) {
- pa_xfree(r);
- return TRUE;
- }
-
- pa_xfree(r);
- }
-
- return FALSE;
-}
-
static pa_hook_result_t process(struct userdata *u, pa_object *o, pa_bool_t is_sink_input) {
const char *want;
pa_proplist *pl, *parent_pl;
@@ -93,7 +72,7 @@ static pa_hook_result_t process(struct userdata *u, pa_object *o, pa_bool_t is_s
return PA_HOOK_OK;
/* On phone sinks, make sure we're not applying echo cancellation */
- if (role_match(parent_pl, "phone")) {
+ if (pa_str_in_list_spaces(pa_proplist_gets(parent_pl, PA_PROP_DEVICE_INTENDED_ROLES), "phone")) {
const char *apply = pa_proplist_gets(pl, PA_PROP_FILTER_APPLY);
if (apply && pa_streq(apply, "echo-cancel")) {
diff --git a/src/modules/module-intended-roles.c b/src/modules/module-intended-roles.c
index 9ba893b..d1e9b81 100644
--- a/src/modules/module-intended-roles.c
+++ b/src/modules/module-intended-roles.c
@@ -66,24 +66,7 @@ struct userdata {
};
static pa_bool_t role_match(pa_proplist *proplist, const char *role) {
- const char *ir;
- char *r;
- const char *state = NULL;
-
- if (!(ir = pa_proplist_gets(proplist, PA_PROP_DEVICE_INTENDED_ROLES)))
- return FALSE;
-
- while ((r = pa_split_spaces(ir, &state))) {
-
- if (pa_streq(role, r)) {
- pa_xfree(r);
- return TRUE;
- }
-
- pa_xfree(r);
- }
-
- return FALSE;
+ return pa_str_in_list_spaces(pa_proplist_gets(proplist, PA_PROP_DEVICE_INTENDED_ROLES), role);
}
static pa_hook_result_t sink_input_new_hook_callback(pa_core *c, pa_sink_input_new_data *new_data, struct userdata *u) {
diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
index 69986a3..79c8e08 100644
--- a/src/pulsecore/core-util.c
+++ b/src/pulsecore/core-util.c
@@ -2627,6 +2627,26 @@ pa_bool_t pa_in_system_mode(void) {
return !!atoi(e);
}
+/* Checks a whitespace-separated list of words in haystack for needle */
+pa_bool_t pa_str_in_list_spaces(const char *haystack, const char *needle) {
+ char *s;
+ const char *state = NULL;
+
+ if (!haystack || !needle)
+ return FALSE;
+
+ while ((s = pa_split_spaces(haystack, &state))) {
+ if (pa_streq(needle, s)) {
+ pa_xfree(s);
+ return TRUE;
+ }
+
+ pa_xfree(s);
+ }
+
+ return FALSE;
+}
+
char *pa_get_user_name_malloc(void) {
ssize_t k;
char *u;
diff --git a/src/pulsecore/core-util.h b/src/pulsecore/core-util.h
index 2b5fe9c..4a1c096 100644
--- a/src/pulsecore/core-util.h
+++ b/src/pulsecore/core-util.h
@@ -204,6 +204,7 @@ void pa_unset_env_recorded(void);
pa_bool_t pa_in_system_mode(void);
#define pa_streq(a,b) (!strcmp((a),(b)))
+pa_bool_t pa_str_in_list_spaces(const char *needle, const char *haystack);
char *pa_get_host_name_malloc(void);
char *pa_get_user_name_malloc(void);
commit 667289679f0ee9ed5ff8562d1e5fea9aca8e21fc
Author: Arun Raghavan <arun.raghavan at collabora.co.uk>
Date: Thu Oct 27 12:54:17 2011 +0200
doc: Correct intended roles property documentation
The documentation says we expect a comma-separate list of intended
roles, but the code splits the string on whitespaces, so this corrects
the documentation to match the implementation.
diff --git a/src/pulse/proplist.h b/src/pulse/proplist.h
index 50b1806..bc143bc 100644
--- a/src/pulse/proplist.h
+++ b/src/pulse/proplist.h
@@ -236,7 +236,7 @@ PA_C_DECL_BEGIN
/** For devices: profile identifier for the profile this devices is in. e.g. "analog-stereo", "analog-surround-40", "iec958-stereo", ...*/
#define PA_PROP_DEVICE_PROFILE_NAME "device.profile.name"
-/** For devices: intended use. A comma separated list of roles (see PA_PROP_MEDIA_ROLE) this device is particularly well suited for, due to latency, quality or form factor. \since 0.9.16 */
+/** For devices: intended use. A space separated list of roles (see PA_PROP_MEDIA_ROLE) this device is particularly well suited for, due to latency, quality or form factor. \since 0.9.16 */
#define PA_PROP_DEVICE_INTENDED_ROLES "device.intended_roles"
/** For devices: human readable one-line description of the profile this device is in. e.g. "Analog Stereo", ... */
More information about the pulseaudio-commits
mailing list