* [PATCH v2 3/5] adapter: Convert storage trusts
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>
All entry in trusts file are set to [all] (if a device is not trusted
it does not have entry in this file).
So, we do not need to check entry value and set device (entry key) as
trusted.
---
src/adapter.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/adapter.c b/src/adapter.c
index 7ba821e..bdea378 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2543,6 +2543,11 @@ static void convert_aliases_entry(GKeyFile *key_file, void *value)
g_key_file_set_string(key_file, "General", "Alias", value);
}
+static void convert_trusts_entry(GKeyFile *key_file, void *value)
+{
+ g_key_file_set_boolean(key_file, "General", "Trusted", TRUE);
+}
+
static void convert_entry(char *key, char *value, void *user_data)
{
struct device_converter *converter = user_data;
@@ -2619,6 +2624,9 @@ static void convert_device_storage(struct btd_adapter *adapter)
/* Convert aliases */
convert_file("aliases", address, convert_aliases_entry);
+
+ /* Convert trusts */
+ convert_file("trusts", address, convert_trusts_entry);
}
static void convert_config(struct btd_adapter *adapter, const char *filename,
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 4/5] device: Use new storage for device trust
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>
As set_trust() changes value in device structure and write to
storage is deferred, property set could not return failure.
---
src/device.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/device.c b/src/device.c
index c508d18..792fe3d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -226,6 +226,9 @@ static gboolean store_device_info_cb(gpointer user_data)
g_key_file_set_string(key_file, "General", "Alias",
device->alias);
+ g_key_file_set_boolean(key_file, "General", "Trusted",
+ device->trusted);
+
ba2str(adapter_get_address(device->adapter), adapter_addr);
ba2str(&device->bdaddr, device_addr);
snprintf(filename, PATH_MAX, INFO_PATH, adapter_addr, device_addr);
@@ -713,23 +716,14 @@ static gboolean dev_property_get_trusted(const GDBusPropertyTable *property,
static void set_trust(GDBusPendingPropertySet id, gboolean value, void *data)
{
struct btd_device *device = data;
- struct btd_adapter *adapter = device->adapter;
- char srcaddr[18], dstaddr[18];
- int err;
if (device->trusted == value)
return g_dbus_pending_property_success(id);
- ba2str(adapter_get_address(adapter), srcaddr);
- ba2str(&device->bdaddr, dstaddr);
-
- err = write_trust(srcaddr, dstaddr, device->bdaddr_type, value);
- if (err < 0)
- return g_dbus_pending_property_error(id,
- ERROR_INTERFACE ".Failed", strerror(-err));
-
device->trusted = value;
+ store_device_info(device);
+
g_dbus_emit_property_changed(btd_get_dbus_connection(),
device->path, DEVICE_INTERFACE, "Trusted");
@@ -1743,6 +1737,10 @@ static void load_info(struct btd_device *device, const gchar *local,
device->alias = g_key_file_get_string(key_file, "General", "Alias",
NULL);
+ /* Load trust */
+ device->trusted = g_key_file_get_boolean(key_file, "General",
+ "Trusted", NULL);
+
if (store_needed)
store_device_info(device);
@@ -1787,8 +1785,6 @@ struct btd_device *device_create(struct btd_adapter *adapter,
load_info(device, srcaddr, address);
- device->trusted = read_trust(src, address, device->bdaddr_type);
-
if (read_blocked(src, &device->bdaddr, device->bdaddr_type))
device_block(device, FALSE);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 5/5] adapter: Remove storage path #defines
From: Frédéric Danis @ 2012-11-14 11:28 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>
SETTINGS_PATH and CACHE_PATH will be static for the
entire 5.x series
---
src/adapter.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/adapter.c b/src/adapter.c
index bdea378..3f09295 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -84,9 +84,6 @@
#define REMOVE_TEMP_TIMEOUT (3 * 60)
#define PENDING_FOUND_MAX 5
-#define SETTINGS_PATH STORAGEDIR "/%s/settings"
-#define CACHE_PATH STORAGEDIR "/%s/cache/%s"
-
static GSList *adapter_drivers = NULL;
enum session_req_type {
@@ -244,7 +241,7 @@ static void store_adapter_info(struct btd_adapter *adapter)
adapter->discov_timeout);
ba2str(&adapter->bdaddr, address);
- snprintf(filename, PATH_MAX, SETTINGS_PATH, address);
+ snprintf(filename, PATH_MAX, STORAGEDIR "/%s/settings", address);
filename[PATH_MAX] = '\0';
create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
@@ -267,7 +264,7 @@ static void store_cached_name(const bdaddr_t *local, const bdaddr_t *peer,
ba2str(local, s_addr);
ba2str(peer, d_addr);
- snprintf(filename, PATH_MAX, CACHE_PATH, s_addr, d_addr);
+ snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s", s_addr, d_addr);
filename[PATH_MAX] = '\0';
create_file(filename, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
@@ -2703,7 +2700,7 @@ static void load_config(struct btd_adapter *adapter)
key_file = g_key_file_new();
- snprintf(filename, PATH_MAX, SETTINGS_PATH, address);
+ snprintf(filename, PATH_MAX, STORAGEDIR "/%s/settings", address);
filename[PATH_MAX] = '\0';
if (!g_key_file_load_from_file(key_file, filename, 0, NULL))
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v2 1/5] adapter: Convert storage aliases
From: Johan Hedberg @ 2012-11-14 11:54 UTC (permalink / raw)
To: Frédéric Danis; +Cc: linux-bluetooth
In-Reply-To: <1352892533-15154-1-git-send-email-frederic.danis@linux.intel.com>
Hi Frederic,
On Wed, Nov 14, 2012, Frédéric Danis wrote:
> ---
> src/adapter.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
All patches in this set have been applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCHv1 11/12] Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt
From: Andrei Emeltchenko @ 2012-11-14 12:19 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth, marcel
In-Reply-To: <alpine.DEB.2.02.1211130911200.27104@mathewm-linux>
Hi Mat,
On Tue, Nov 13, 2012 at 09:29:38AM -0800, Mat Martineau wrote:
> >>>>>+void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
> >>>>>+{
> >>>>>+ struct hci_dev *bredr_hdev = hci_dev_hold(bredr_hcon->hdev);
> >>>>>+ struct amp_mgr *mgr = hs_hcon->amp_mgr;
> >>>>>+ struct l2cap_chan *bredr_chan;
> >>>>>+
> >>>>>+ BT_DBG("bredr_hcon %p hs_hcon %p mgr %p", bredr_hcon, hs_hcon, mgr);
> >>>>>+
> >>>>>+ if (!bredr_hdev || !mgr || !mgr->bredr_chan)
> >>>>>+ return;
> >>>>>+
> >>>>>+ bredr_chan = mgr->bredr_chan;
> >>>>>+
> >>>>>+ set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
> >>>>>+ bredr_chan->ctrl_id = hs_hcon->remote_id;
> >>>>>+ bredr_chan->hs_hcon = hs_hcon;
> >>>>>+ bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
> >>>>>+ bredr_chan->fcs = L2CAP_FCS_NONE;
> >>
> >>Changing FCS requires L2CAP reconfiguration for the channel, and
> >>chan->fcs shouldn't be modified until reconfiguration happens.
> >>While it doesn't make much sense to do so, the remote device may
> >>want to keep FCS enabled. The move may also fail and you don't want
> >>to forget the original FCS setting in that case.
> >
> >So we agree that FCS shall not be used for High Speed channels. I was
> >thinking more about the case where we start sending data right over HS
> >channel. The configuration should be just started.
>
> No matter what the BlueZ implementation prefers, it must be able to
> handle a remote device that requests FCS during L2CAP config. If
> one side requests FCS, then it must be used.
The idea here is to disable FCS on our side, we would still respect FCS
requests from remote side. This code needs to be moved to l2cap_do_create.
> Do we want BlueZ to always ignore the L2CAP FCS socket option on AMP
> controllers? (Marcel?) This checksum is always redundant with
> 802.11 AMP controllers.
>
> >What would be the better place to init FCS? l2cap_physical_cfm after
> >checking channel moving status?
>
> I think it should be as late as possible before L2CAP configuration
> begins in order to be sure the channel is really on AMP. For the
> "create channel" case, set_default_fcs could see if an AMP link is
> being used and turn the FCS off.
set_default_fcs sets FCS when configuration is finished, this way we
cannot respect other side FCS settings.
Best regards
Andrei Emeltchenko
^ permalink raw reply
* Re: [PATCH BlueZ 1/2] build: Require D-Bus 1.5 or later
From: Syam Sidhardhan @ 2012-11-14 13:03 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <CAFBvHif+xS12ywGEUdpycTtU--TVZSTgAb4mJuN_O5NKJSz36w@mail.gmail.com>
Hi,
On Tue, Nov 13, 2012 at 10:01 PM, Syam Sidhardhan
<syamsidhardh@gmail.com> wrote:
> Hi Marcel,
>
> On Fri, Nov 9, 2012 at 4:14 PM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
>> Hi Syam,
>>
>>> The gdbus require D-Bus 1.5 version.
>>>
>>> Log:
>>> CC gdbus/object.o
>>> gdbus/object.c: In function =E2=80=98properties_set=E2=80=99:
>>> gdbus/object.c:876:7: error: =E2=80=98DBUS_ERROR_UNKNOWN_PROPERTY=E2=80=
=99 undeclared
>>> (first use in this function)
>>> gdbus/object.c:876:7: note: each undeclared identifier is reported
>>> only once for each function it appears in
>>> gdbus/object.c:881:6: error: =E2=80=98DBUS_ERROR_PROPERTY_READ_ONLY=E2=
=80=99
>>> undeclared (first use in this function)
>>> make[1]: *** [gdbus/object.o] Error 1
>>> make: *** [all] Error 2
>>
>> if they are still simple defines, then lets just #ifndef them and define
>> them in gdbus/object.c
>>
>
> Yes, those are simple defines. I'll send a patch according to your sugges=
tion.
>
Missing the mailing list in my earlier reply. CC-ing here.
Regards,
Syam.
^ permalink raw reply
* [PATCH BlueZ 1/1] gdbus: Fix compilation error due to missing #defines
From: Syam Sidhardhan @ 2012-11-14 13:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Syam Sidhardhan
Since these are simple #define strings, we are defining it here
instead of upgrading to D-Bus 1.5 or later.
Log:
CC gdbus/object.o
gdbus/object.c: In function ‘properties_set’:
gdbus/object.c:876:7: error: ‘DBUS_ERROR_UNKNOWN_PROPERTY’ undeclared
(first use in this function)
gdbus/object.c:876:7: note: each undeclared identifier is reported
only once for each function it appears in
gdbus/object.c:881:6: error: ‘DBUS_ERROR_PROPERTY_READ_ONLY’
undeclared (first use in this function)
make[1]: *** [gdbus/object.o] Error 1
make: *** [all] Error 2
---
gdbus/object.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/gdbus/object.c b/gdbus/object.c
index e3ad067..3101ca6 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -39,6 +39,14 @@
#define DBUS_INTERFACE_OBJECT_MANAGER "org.freedesktop.DBus.ObjectManager"
+#ifndef DBUS_ERROR_UNKNOWN_PROPERTY
+#define DBUS_ERROR_UNKNOWN_PROPERTY "org.freedesktop.DBus.Error.UnknownProperty"
+#endif
+
+#ifndef DBUS_ERROR_PROPERTY_READ_ONLY
+#define DBUS_ERROR_PROPERTY_READ_ONLY "org.freedesktop.DBus.Error.PropertyReadOnly"
+#endif
+
struct generic_data {
unsigned int refcount;
DBusConnection *conn;
--
1.7.4.1
^ permalink raw reply related
* Re: [RFC] Bluetooth: Add BT_DEFER_SETUP option to sco socket
From: Marcel Holtmann @ 2012-11-14 14:39 UTC (permalink / raw)
To: frederic.dalleau; +Cc: Michael Knudsen, linux-bluetooth
In-Reply-To: <50A36E06.2060206@linux.intel.com>
Hi Fred,
> > My idea is to define (I'm not sure if this is exactly the same as what you
> > suggested yourself) a bit, LM_DEFER, that e.g. sco_connect_ind() may return
> > and propagate this bit to hci_conn_request_evt(). This way all policing of
> > accepting SCO connections is handled from a single entry point into sco.c.
>
> You detailed it much further than I did, but yes it is the same idea. I
> fully support it! If everybody agrees, then I can update the patch.
>
> hci.h would receive the folowing line:
>
> + #define HCI_LM_DEFER 0x4000
can we actually not that that. This is an old public API and I rather
not extend it.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v4 11/16] sbc: Add SBC_MSBC flag to enable 15 block encoding
From: Marcel Holtmann @ 2012-11-14 14:49 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1351589975-22640-12-git-send-email-frederic.dalleau@linux.intel.com>
Hi Fred,
> ---
> sbc/sbc.c | 23 +++++++++++++++--------
> sbc/sbc.h | 3 +++
> 2 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/sbc/sbc.c b/sbc/sbc.c
> index ffdf05d..22fa898 100644
> --- a/sbc/sbc.c
> +++ b/sbc/sbc.c
> @@ -52,6 +52,9 @@
>
> #define SBC_SYNCWORD 0x9C
>
> +#define MSBC_SYNCWORD 0xAD
> +#define MSBC_BLOCKS 15
> +
> /* This structure contains an unpacked SBC frame.
> Yes, there is probably quite some unused space herein */
> struct sbc_frame {
> @@ -903,12 +906,12 @@ static ssize_t sbc_pack_frame(uint8_t *data, struct sbc_frame *frame, size_t len
> }
> }
>
> -static void sbc_encoder_init(struct sbc_encoder_state *state,
> - const struct sbc_frame *frame)
> +static void sbc_encoder_init(unsigned long flags,
> + struct sbc_encoder_state *state, const struct sbc_frame *frame)
> {
> memset(&state->X, 0, sizeof(state->X));
> state->position = (SBC_X_BUFFER_SIZE - frame->subbands * 9) & ~7;
> - state->increment = 4;
> + state->increment = flags & SBC_MSBC ? 1 : 4;
just for pure cosmetics, use an if statement.
>
> sbc_init_primitives(state);
> }
> @@ -922,6 +925,7 @@ struct sbc_priv {
>
> static void sbc_set_defaults(sbc_t *sbc, unsigned long flags)
> {
> + sbc->flags = flags;
> sbc->frequency = SBC_FREQ_44100;
> sbc->mode = SBC_MODE_STEREO;
> sbc->subbands = SBC_SB_8;
> @@ -1057,12 +1061,13 @@ SBC_EXPORT ssize_t sbc_encode(sbc_t *sbc, const void *input, size_t input_len,
> priv->frame.subband_mode = sbc->subbands;
> priv->frame.subbands = sbc->subbands ? 8 : 4;
> priv->frame.block_mode = sbc->blocks;
> - priv->frame.blocks = 4 + (sbc->blocks * 4);
> + priv->frame.blocks = sbc->flags & SBC_MSBC ?
> + MSBC_BLOCKS : 4 + (sbc->blocks * 4);
Same here.
> priv->frame.bitpool = sbc->bitpool;
> priv->frame.codesize = sbc_get_codesize(sbc);
> priv->frame.length = sbc_get_frame_length(sbc);
>
> - sbc_encoder_init(&priv->enc_state, &priv->frame);
> + sbc_encoder_init(sbc->flags, &priv->enc_state, &priv->frame);
> priv->init = 1;
> } else if (priv->frame.bitpool != sbc->bitpool) {
> priv->frame.length = sbc_get_frame_length(sbc);
> @@ -1141,7 +1146,7 @@ SBC_EXPORT size_t sbc_get_frame_length(sbc_t *sbc)
> return priv->frame.length;
>
> subbands = sbc->subbands ? 8 : 4;
> - blocks = 4 + (sbc->blocks * 4);
> + blocks = sbc->flags & SBC_MSBC ? MSBC_BLOCKS : 4 + (sbc->blocks * 4);
And for this one as well.
> channels = sbc->mode == SBC_MODE_MONO ? 1 : 2;
> joint = sbc->mode == SBC_MODE_JOINT_STEREO ? 1 : 0;
> bitpool = sbc->bitpool;
> @@ -1165,7 +1170,8 @@ SBC_EXPORT unsigned sbc_get_frame_duration(sbc_t *sbc)
> priv = sbc->priv;
> if (!priv->init) {
> subbands = sbc->subbands ? 8 : 4;
> - blocks = 4 + (sbc->blocks * 4);
> + blocks = sbc->flags & SBC_MSBC ?
> + MSBC_BLOCKS : 4 + (sbc->blocks * 4);
Even here it is better cosmetics.
> } else {
> subbands = priv->frame.subbands;
> blocks = priv->frame.blocks;
> @@ -1202,7 +1208,8 @@ SBC_EXPORT size_t sbc_get_codesize(sbc_t *sbc)
> priv = sbc->priv;
> if (!priv->init) {
> subbands = sbc->subbands ? 8 : 4;
> - blocks = 4 + (sbc->blocks * 4);
> + blocks = sbc->flags & SBC_MSBC ?
> + MSBC_BLOCKS : 4 + (sbc->blocks * 4);
Here as well.
> channels = sbc->mode == SBC_MODE_MONO ? 1 : 2;
> } else {
> subbands = priv->frame.subbands;
> diff --git a/sbc/sbc.h b/sbc/sbc.h
> index bbd45da..3511119 100644
> --- a/sbc/sbc.h
> +++ b/sbc/sbc.h
> @@ -64,6 +64,9 @@ extern "C" {
> #define SBC_LE 0x00
> #define SBC_BE 0x01
>
> +/* Additional features */
> +#define SBC_MSBC 0x01
> +
I am debating to actually call this SBC_FLAG_MSBC instead of just
SBC_MSBC. Anyone any comments?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v4 00/16] mSBC tests
From: Marcel Holtmann @ 2012-11-14 14:50 UTC (permalink / raw)
To: frederic.dalleau; +Cc: linux-bluetooth
In-Reply-To: <50A36BAC.4090604@linux.intel.com>
Hi Fred,
> > v4 integrate latest comments from Siarhei:
> > - remove state->pending field from sbc_encoder_state
> > - add armv6 and iwmmxt primitives
> > - use simd primitive instead of neon
> > - reorder patches so that the SBC_MSBC flag is exposed to users only when
> > implementation is complete.
>
> Any feedback ?
beside a tiny cosmetic comment, I have nothing. However I am not the
expert here. If Siarhei looks over this and is fine with it, I will be
as well.
Regards
Marcel
^ permalink raw reply
* Re: CSA2: User space aspect
From: Michael Knudsen @ 2012-11-14 14:55 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <509BB56A.80609@samsung.com>
On 2012-11-08 14:36, Michael Knudsen wrote:
> If anyone else has an ideas or opinions about this, please speak up,
> otherwise I'll try coming up with an interface specification with
> more details.
This diff shows the direction I'm heading:
diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 1e35c43..a565a4d 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -51,6 +51,42 @@ struct sco_conninfo {
__u8 dev_class[3];
};
+/* Audio format setting */
+#define SCO_HOST_FORMAT 0x04
+#define SCO_AIR_FORMAT 0x05
+
+#define SCO_FORMAT_ULAW 0x00
+#define SCO_FORMAT_ALAW 0x01
+#define SCO_FORMAT_CVSD 0x02
+#define SCO_FORMAT_TRANSPARENT 0x03
+#define SCO_FORMAT_PCM 0x05
+#define SCO_FORMAT_MSBC 0x05
+#define SCO_FORMAT_VENDOR 0xff
+struct sco_format_vendor {
+ __u16 vendor;
+ __u16 codec;
+};
+
+struct sco_format {
+ __u8 in_format;
+ struct sco_format_vendor in_vendor;
+
+ __u8 out_format;
+ struct sco_format_vendor out_vendor;
+};
+
+#define SCO_CODECS 0x06
+struct sco_codecs {
+ __u8 count;
+ __u8 *codec;
+};
+
+#define SCO_CODECS_VENDOR 0x07
+struct sco_codecs_vendor {
+ __u8 count;
+ struct sco_format_vendor *format;
+};
+
/* ---- SCO connections ---- */
struct sco_conn {
struct hci_conn *hcon;
@@ -74,6 +110,8 @@ struct sco_pinfo {
struct bt_sock bt;
__u32 flags;
struct sco_conn *conn;
+ struct sco_format host_format;
+ struct sco_format air_format;
};
#endif /* __SCO_H */
Basically, this adds four socket options (I'll do the audio path
stuff as well once this is done):
SCO_AIR_FORMAT
SCO_HOST_FORMAT
SCO_CODECS (ro)
SCO_CODECS_VENDOR (ro)
The SCO_CODECS ones provide the application with a list of codecs
supported by the hdev as indicated in the HCI_Read_Local_Supported_Codecs
command response, and if the hdev does not support this command a
default of linear PCM, CVSD, and transparent will be provided.
Because the result length is variable, the idea is that the application-
provided structure is modified by the kernel to hold the actual number
of results so the application can allocate a buffer accordingly, e.g.:
struct sco_codecs sc;
memset(&sc, 0, sizeof(sc));
getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);
sk.codecs = malloc(sk.count);
getsockopt(sk, SOL_SCO, SCO_CODECS, &sk);
The SCO_*_FORMAT ones allows the application to set the parameters that
are to be used on host-controller and controller-controller paths. While
the spec requires the pairs (host input/output, air input/output) to be
identical, I don't see a reason to enforce this in the API, thus all are
set independently.
So, before I spend any more time on this.. comments?
-m.
^ permalink raw reply related
* Re: [PATCH BlueZ 1/1] gdbus: Fix compilation error due to missing #defines
From: Lucas De Marchi @ 2012-11-14 15:13 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth
In-Reply-To: <1352898430-3383-1-git-send-email-s.syam@samsung.com>
On Wed, Nov 14, 2012 at 11:07 AM, Syam Sidhardhan <s.syam@samsung.com> wrote:
> Since these are simple #define strings, we are defining it here
> instead of upgrading to D-Bus 1.5 or later.
>
> Log:
> CC gdbus/object.o
> gdbus/object.c: In function ‘properties_set’:
> gdbus/object.c:876:7: error: ‘DBUS_ERROR_UNKNOWN_PROPERTY’ undeclared
> (first use in this function)
> gdbus/object.c:876:7: note: each undeclared identifier is reported
> only once for each function it appears in
> gdbus/object.c:881:6: error: ‘DBUS_ERROR_PROPERTY_READ_ONLY’
> undeclared (first use in this function)
> make[1]: *** [gdbus/object.o] Error 1
> make: *** [all] Error 2
> ---
> gdbus/object.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/gdbus/object.c b/gdbus/object.c
> index e3ad067..3101ca6 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -39,6 +39,14 @@
>
> #define DBUS_INTERFACE_OBJECT_MANAGER "org.freedesktop.DBus.ObjectManager"
>
> +#ifndef DBUS_ERROR_UNKNOWN_PROPERTY
> +#define DBUS_ERROR_UNKNOWN_PROPERTY "org.freedesktop.DBus.Error.UnknownProperty"
> +#endif
> +
> +#ifndef DBUS_ERROR_PROPERTY_READ_ONLY
> +#define DBUS_ERROR_PROPERTY_READ_ONLY "org.freedesktop.DBus.Error.PropertyReadOnly"
> +#endif
> +
> struct generic_data {
> unsigned int refcount;
> DBusConnection *conn;
> --
> 1.7.4.1
Ack
Lucas De Marchi
^ permalink raw reply
* Re: [PATCH v4 11/16] sbc: Add SBC_MSBC flag to enable 15 block encoding
From: Frédéric Dalleau @ 2012-11-14 15:34 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1352904577.20338.10.camel@aeonflux>
Hi Marcel,
On 11/14/2012 03:49 PM, Marcel Holtmann wrote:
> I am debating to actually call this SBC_FLAG_MSBC instead of just
> SBC_MSBC. Anyone any comments?
The use of the flag may not be the most elegant API. As an alternative,
maybe we could have a specific function for msbc initialization. Then,
instead of sbc_init(), sbc_encode() sbc_decode(), we could have
msbc_init(), sbc_encode(), sbc_decode().
The biggest advantage is that parameters like bitpool, subbands,
allocation, and other parts could be specified inside this function
instead of being given from the application.
Regards,
Frédéric
^ permalink raw reply
* [RFCv1 1/3] Bluetooth: Refactor locking in amp_physical_cfm
From: Andrei Emeltchenko @ 2012-11-14 15:39 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Remove locking from l2cap_physical_cfm and lock chan inside
amp_physical_cfm.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
include/net/bluetooth/l2cap.h | 2 +-
net/bluetooth/amp.c | 6 +++++-
net/bluetooth/l2cap_core.c | 7 ++-----
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index d65db45..f57fab0 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -811,6 +811,6 @@ void l2cap_send_conn_req(struct l2cap_chan *chan);
void l2cap_move_start(struct l2cap_chan *chan);
void l2cap_logical_cfm(struct l2cap_chan *chan, struct hci_chan *hchan,
u8 status);
-void l2cap_physical_cfm(struct l2cap_chan *chan, int result);
+void __l2cap_physical_cfm(struct l2cap_chan *chan, int result);
#endif /* __L2CAP_H */
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index 4b2fea6..eaf473f 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -386,13 +386,17 @@ void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
bredr_chan = mgr->bredr_chan;
+ l2cap_chan_lock(bredr_chan);
+
set_bit(FLAG_EFS_ENABLE, &bredr_chan->flags);
bredr_chan->remote_amp_id = hs_hcon->remote_id;
bredr_chan->hs_hcon = hs_hcon;
bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
bredr_chan->fcs = L2CAP_FCS_NONE;
- l2cap_physical_cfm(bredr_chan, 0);
+ __l2cap_physical_cfm(bredr_chan, 0);
+
+ l2cap_chan_unlock(bredr_chan);
hci_dev_put(bredr_hdev);
}
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a1faaab..138d505 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4612,7 +4612,8 @@ static void l2cap_do_move_cancel(struct l2cap_chan *chan, int result)
l2cap_ertm_send(chan);
}
-void l2cap_physical_cfm(struct l2cap_chan *chan, int result)
+/* Invoke with locked chan */
+void __l2cap_physical_cfm(struct l2cap_chan *chan, int result)
{
u8 local_amp_id = chan->local_amp_id;
u8 remote_amp_id = chan->remote_amp_id;
@@ -4620,8 +4621,6 @@ void l2cap_physical_cfm(struct l2cap_chan *chan, int result)
BT_DBG("chan %p, result %d, local_amp_id %d, remote_amp_id %d",
chan, result, local_amp_id, remote_amp_id);
- l2cap_chan_lock(chan);
-
if (chan->state == BT_DISCONN || chan->state == BT_CLOSED) {
l2cap_chan_unlock(chan);
return;
@@ -4645,8 +4644,6 @@ void l2cap_physical_cfm(struct l2cap_chan *chan, int result)
break;
}
}
-
- l2cap_chan_unlock(chan);
}
static inline int l2cap_move_channel_req(struct l2cap_conn *conn,
--
1.7.10.4
^ permalink raw reply related
* [RFCv1 2/3] Bluetooth: Disable FCS only for new HS channels
From: Andrei Emeltchenko @ 2012-11-14 15:39 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1352907572-7113-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Set chan->fcs to L2CAP_FCS_NONE only for new L2CAP channels
(not moved). Other side can still request to use FCS.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
net/bluetooth/amp.c | 1 -
net/bluetooth/l2cap_core.c | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
index eaf473f..0258b26 100644
--- a/net/bluetooth/amp.c
+++ b/net/bluetooth/amp.c
@@ -392,7 +392,6 @@ void amp_physical_cfm(struct hci_conn *bredr_hcon, struct hci_conn *hs_hcon)
bredr_chan->remote_amp_id = hs_hcon->remote_id;
bredr_chan->hs_hcon = hs_hcon;
bredr_chan->conn->mtu = hs_hcon->hdev->block_mtu;
- bredr_chan->fcs = L2CAP_FCS_NONE;
__l2cap_physical_cfm(bredr_chan, 0);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 138d505..10b3062 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4518,6 +4518,8 @@ void l2cap_move_start(struct l2cap_chan *chan)
static void l2cap_do_create(struct l2cap_chan *chan, int result,
u8 local_amp_id, u8 remote_amp_id)
{
+ chan->fcs = L2CAP_FCS_NONE;
+
if (!test_bit(CONF_CONNECT_PEND, &chan->conf_state)) {
struct l2cap_conn_rsp rsp;
char buf[128];
--
1.7.10.4
^ permalink raw reply related
* [RFCv1 3/3] Bluetooth: trivial: Use __constant for constants
From: Andrei Emeltchenko @ 2012-11-14 15:39 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1352907572-7113-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
net/bluetooth/l2cap_core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 10b3062..4a635f1 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4529,12 +4529,12 @@ static void l2cap_do_create(struct l2cap_chan *chan, int result,
/* Incoming channel on AMP */
if (result == L2CAP_CR_SUCCESS) {
/* Send successful response */
- rsp.result = cpu_to_le16(L2CAP_CR_SUCCESS);
- rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
+ rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
+ rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
} else {
/* Send negative response */
- rsp.result = cpu_to_le16(L2CAP_CR_NO_MEM);
- rsp.status = cpu_to_le16(L2CAP_CS_NO_INFO);
+ rsp.result = __constant_cpu_to_le16(L2CAP_CR_NO_MEM);
+ rsp.status = __constant_cpu_to_le16(L2CAP_CS_NO_INFO);
}
l2cap_send_cmd(chan->conn, chan->ident, L2CAP_CREATE_CHAN_RSP,
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH v4 05/16] sbc: Add mmx primitive for 1b 8s analysis
From: Frédéric Dalleau @ 2012-11-14 15:43 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1351589975-22640-6-git-send-email-frederic.dalleau@linux.intel.com>
Hi,
Since I'm gonna resend a new series, I'll comment myself ;)
On 10/30/2012 10:39 AM, Frédéric Dalleau wrote:
> +static inline void sbc_analyze_1b_8s_mmx(struct sbc_encoder_state *state,
> + int16_t *x, int32_t *out, int out_stride)
> +{
> + if (state->odd)
> + sbc_analyze_eight_mmx(x, out, analysis_consts_fixed8_simd_odd);
> + else
> + sbc_analyze_eight_mmx(x, out, analysis_consts_fixed8_simd_even);
> +
> + state->odd = !state->odd;
> +
> + __asm__ volatile ("emms\n");
> +}
> +
One thing bother me about this patch : the emms instruction is called
after every block, instead of every four blocks until now. I have very
little knowledge about this, but I read that emms instruction is
somewhat expensive.
Some quick tests haven't shown differences, but it is possible to add a
post analyze callback to overcome this. In this case, emms instruction
could be run every 15 blocks or whatever is defined.
Is it worth it?
Thanks,
Frédéric
^ permalink raw reply
* [RFC 1/2] scotest: Add deferred setup option
From: Frédéric Dalleau @ 2012-11-14 18:17 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Frédéric Dalleau
---
test/scotest.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 55 insertions(+), 5 deletions(-)
diff --git a/test/scotest.c b/test/scotest.c
index de65edf..a40e395 100644
--- a/test/scotest.c
+++ b/test/scotest.c
@@ -57,6 +57,8 @@ static long data_size = 672;
static bdaddr_t bdaddr;
+static int defer_setup = 0;
+
static float tv2fl(struct timeval tv)
{
return (float)tv.tv_sec + (float)(tv.tv_usec/1000000.0);
@@ -147,6 +149,14 @@ static void do_listen(void (*handler)(int sk))
goto error;
}
+ /* Enable deferred setup */
+ if (defer_setup && setsockopt(sk, SOL_BLUETOOTH, BT_DEFER_SETUP,
+ &defer_setup, sizeof(defer_setup)) < 0) {
+ syslog(LOG_ERR, "Can't enable deferred setup : %s (%d)",
+ strerror(errno), errno);
+ goto error;
+ }
+
/* Listen for connections */
if (listen(sk, 10)) {
syslog(LOG_ERR,"Can not listen on the socket: %s (%d)",
@@ -181,8 +191,10 @@ static void do_listen(void (*handler)(int sk))
if (getsockopt(nsk, SOL_SCO, SCO_CONNINFO, &conn, &optlen) < 0) {
syslog(LOG_ERR, "Can't get SCO connection information: %s (%d)",
strerror(errno), errno);
- close(nsk);
- goto error;
+ if (!defer_setup) {
+ close(nsk);
+ goto error;
+ }
}
ba2str(&addr.sco_bdaddr, ba);
@@ -190,6 +202,18 @@ static void do_listen(void (*handler)(int sk))
ba, conn.hci_handle,
conn.dev_class[2], conn.dev_class[1], conn.dev_class[0]);
+ /* Handle deferred setup */
+ if (defer_setup) {
+ syslog(LOG_INFO, "Waiting for %d seconds",
+ abs(defer_setup) - 1);
+ sleep(abs(defer_setup) - 1);
+
+ if (defer_setup < 0) {
+ close(nsk);
+ goto error;
+ }
+ }
+
handler(nsk);
syslog(LOG_INFO, "Disconnect");
@@ -207,6 +231,15 @@ static void dump_mode(int sk)
{
int len;
+ if (defer_setup) {
+ len = read(sk, buf, sizeof(buf));
+ if (len < 0)
+ syslog(LOG_ERR, "Initial read error: %s (%d)",
+ strerror(errno), errno);
+ else
+ syslog(LOG_INFO, "Initial bytes %d", len);
+ }
+
syslog(LOG_INFO,"Receiving ...");
while ((len = read(sk, buf, data_size)) > 0)
syslog(LOG_INFO, "Recevied %d bytes", len);
@@ -216,6 +249,16 @@ static void recv_mode(int sk)
{
struct timeval tv_beg,tv_end,tv_diff;
long total;
+ int len;
+
+ if (defer_setup) {
+ len = read(sk, buf, sizeof(buf));
+ if (len < 0)
+ syslog(LOG_ERR, "Initial read error: %s (%d)",
+ strerror(errno), errno);
+ else
+ syslog(LOG_INFO, "Initial bytes %d", len);
+ }
syslog(LOG_INFO, "Receiving ...");
@@ -328,14 +371,17 @@ static void usage(void)
{
printf("scotest - SCO testing\n"
"Usage:\n");
- printf("\tscotest <mode> [-b bytes] [bd_addr]\n");
+ printf("\tscotest <mode> [options] [bd_addr]\n");
printf("Modes:\n"
"\t-d dump (server)\n"
"\t-c reconnect (client)\n"
"\t-m multiple connects (client)\n"
"\t-r receive (server)\n"
"\t-s connect and send (client)\n"
- "\t-n connect and be silent (client)\n");
+ "\t-n connect and be silent (client)\n"
+ "Options:\n"
+ "\t[-b bytes]\n"
+ "\t[-W seconds] enable deferred setup\n");
}
int main(int argc ,char *argv[])
@@ -343,7 +389,7 @@ int main(int argc ,char *argv[])
struct sigaction sa;
int opt, sk, mode = RECV;
- while ((opt=getopt(argc,argv,"rdscmnb:")) != EOF) {
+ while ((opt = getopt(argc, argv, "rdscmnb:W:")) != EOF) {
switch(opt) {
case 'r':
mode = RECV;
@@ -373,6 +419,10 @@ int main(int argc ,char *argv[])
data_size = atoi(optarg);
break;
+ case 'W':
+ defer_setup = atoi(optarg);
+ break;
+
default:
usage();
exit(1);
--
1.7.9.5
^ permalink raw reply related
* [RFC 2/2] btiotest: Enable deferred setup for sco sockets
From: Frédéric Dalleau @ 2012-11-14 18:18 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Frédéric Dalleau
In-Reply-To: <1352917080-18509-1-git-send-email-frederic.dalleau@linux.intel.com>
---
test/btiotest.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/test/btiotest.c b/test/btiotest.c
index f090dd9..f157286 100644
--- a/test/btiotest.c
+++ b/test/btiotest.c
@@ -459,24 +459,34 @@ static void sco_connect(const char *src, const char *dst, gint disconn)
}
}
-static void sco_listen(const char *src, gint disconn)
+static void sco_listen(const char *src, gint disconn, gboolean defer)
{
struct io_data *data;
+ BtIOConnect conn;
+ BtIOConfirm cfm;
GIOChannel *sco_srv;
GError *err = NULL;
printf("Listening for SCO connections\n");
+ if (defer) {
+ conn = NULL;
+ cfm = confirm_cb;
+ } else {
+ conn = connect_cb;
+ cfm = NULL;
+ }
+
data = io_data_new(NULL, -1, disconn, -1);
if (src)
- sco_srv = bt_io_listen(connect_cb, NULL, data,
+ sco_srv = bt_io_listen(conn, cfm, data,
(GDestroyNotify) io_data_unref,
&err,
BT_IO_OPT_SOURCE, src,
BT_IO_OPT_INVALID);
else
- sco_srv = bt_io_listen(connect_cb, NULL, data,
+ sco_srv = bt_io_listen(conn, cfm, data,
(GDestroyNotify) io_data_unref,
&err, BT_IO_OPT_INVALID);
@@ -581,7 +591,7 @@ int main(int argc, char *argv[])
if (argc > 1)
sco_connect(opt_dev, argv[1], opt_disconn);
else
- sco_listen(opt_dev, opt_disconn);
+ sco_listen(opt_dev, opt_disconn, opt_defer);
}
signal(SIGTERM, sig_term);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH BlueZ 1/1] gdbus: Fix compilation error due to missing #defines
From: Johan Hedberg @ 2012-11-14 18:24 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth
In-Reply-To: <1352898430-3383-1-git-send-email-s.syam@samsung.com>
Hi Syam,
On Wed, Nov 14, 2012, Syam Sidhardhan wrote:
> Since these are simple #define strings, we are defining it here
> instead of upgrading to D-Bus 1.5 or later.
>
> Log:
> CC gdbus/object.o
> gdbus/object.c: In function ‘properties_set’:
> gdbus/object.c:876:7: error: ‘DBUS_ERROR_UNKNOWN_PROPERTY’ undeclared
> (first use in this function)
> gdbus/object.c:876:7: note: each undeclared identifier is reported
> only once for each function it appears in
> gdbus/object.c:881:6: error: ‘DBUS_ERROR_PROPERTY_READ_ONLY’
> undeclared (first use in this function)
> make[1]: *** [gdbus/object.o] Error 1
> make: *** [all] Error 2
> ---
> gdbus/object.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
Applied. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH v4 10/16] sbc: simd support for 8 multiples block size
From: Siarhei Siamashka @ 2012-11-14 19:09 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1351589975-22640-11-git-send-email-frederic.dalleau@linux.intel.com>
On Tue, 30 Oct 2012 10:39:29 +0100
Frédéric Dalleau <frederic.dalleau@linux.intel.com> wrote:
This looks good. The only potential problem is here:
http://git.kernel.org/?p=bluetooth/sbc.git;a=blob;f=sbc/sbc_primitives.h;h=17ad4f74da4c#l31
31 #define SBC_X_BUFFER_SIZE 328
http://git.kernel.org/?p=bluetooth/sbc.git;a=blob;f=sbc/sbc.c;h=f0c77c7cb546#l908
908 state->position = (SBC_X_BUFFER_SIZE - frame->subbands * 9) & ~7;
http://git.kernel.org/?p=bluetooth/sbc.git;a=blob;f=sbc/sbc_primitives.c;h=ad780d0800de#l285
285 /* handle X buffer wraparound */
286 if (position < nsamples) {
287 if (nchannels > 0)
288 memcpy(&X[0][SBC_X_BUFFER_SIZE - 72], &X[0][position],
289 72 * sizeof(int16_t));
290 if (nchannels > 1)
291 memcpy(&X[1][SBC_X_BUFFER_SIZE - 72], &X[1][position],
292 72 * sizeof(int16_t));
293 position = SBC_X_BUFFER_SIZE - 72;
294 }
As we are going to use divisibility by 16 of position variable as a
way to distinguish between "even" and "odd" blocks (chunks of 8
samples) in the buffer, we need to be sure that the following
conditions are true:
1. assert((SBC_X_BUFFER_SIZE - frame->subbands * 9) & ~7 ==
SBC_X_BUFFER_SIZE - 72);
Buffer wraparound is handled by copying a portion of still
needed old data to the initial position.
2. assert((SBC_X_BUFFER_SIZE - 72) % 16 == 0);
The initial position is divisible by 16.
3. assert((SBC_X_BUFFER_SIZE - 72) % (15 * 8) % 16 == 0);
When we handle buffer wraparound, the position before memcpy has the
same divisibility by 16 as the new updated position after memcpy.
4. assert((SBC_X_BUFFER_SIZE - 72) % (15 * 8) >= 8);
Just to be sure that "x[-7] = PCM(0 + 7 * nchannels)" does not
do invalid memory writes below the X array. This would be not
necessary if we adjust the "if (position < nsamples)" check.
If anybody decides to change SBC_X_BUFFER_SIZE constant, there is a
risk of doing it wrong. But right now the code seems to fulfil these
requirements :)
And some cosmetic nitpicks.
First the text of the summary:
1. Why "simd"? It's not totally wrong, but still way too generic.
You could use "input permutation" or "input processing" or
"sbc_encoder_process_input_s8 function" instead.
2. That's a multiple of 1 block (or alternatively a multiple of 8
samples).
> ---
> sbc/sbc_primitives.c | 37 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/sbc/sbc_primitives.c b/sbc/sbc_primitives.c
> index 9311848..b189320 100644
> --- a/sbc/sbc_primitives.c
> +++ b/sbc/sbc_primitives.c
> @@ -309,8 +309,26 @@ static SBC_ALWAYS_INLINE int sbc_encoder_process_input_s8_internal(
> #define PCM(i) (big_endian ? \
> unaligned16_be(pcm + (i) * 2) : unaligned16_le(pcm + (i) * 2))
>
> + if (position % 16 == 8) {
> + position -= 8;
> + nsamples -= 8;
> + if (nchannels > 0) {
That's a somewhat redundant "if" here. Yes, it is also redundant in
the code where both "if (nchannels > 0)" and "if (nchannels > 1)" are
used, but it was there just for cosmetic reasons in order to keep the
same level of indentation.
> + int16_t *x = &X[0][position];
> + x[0] = PCM(0 + (15-8) * nchannels);
> + x[2] = PCM(0 + (14-8) * nchannels);
> + x[3] = PCM(0 + (8-8) * nchannels);
> + x[4] = PCM(0 + (13-8) * nchannels);
> + x[5] = PCM(0 + (9-8) * nchannels);
> + x[6] = PCM(0 + (12-8) * nchannels);
> + x[7] = PCM(0 + (10-8) * nchannels);
> + x[8] = PCM(0 + (11-8) * nchannels);
> + }
> + /* mSBC is designed for 1 channel */
Thanks for adding this comment. But IMHO it still feels a bit
incomplete. It could be a extended to:
/* mSBC codec is the only case when the number of
* samples to process may be not multiple of 16.
* mSBC only supports 1 channel */
Or alternatively just add back the omitted "if (nchannels > 1)" branch
too. Yes, it is not needed for mSBC now, but what if somebody introduces
something like a stereo variant of mSBC later? This way the code is
more orthogonal, "futureproof" and does not need lengthy excuses and
explanations in comments for the parts which are cutting the corners.
Either way is fine.
> + pcm += 16 * nchannels;
> + }
> +
> /* copy/permutate audio samples */
> - while ((nsamples -= 16) >= 0) {
> + while (nsamples >= 16) {
> position -= 16;
> if (nchannels > 0) {
> int16_t *x = &X[0][position];
> @@ -351,6 +369,23 @@ static SBC_ALWAYS_INLINE int sbc_encoder_process_input_s8_internal(
> x[15] = PCM(1 + 2 * nchannels);
> }
> pcm += 32 * nchannels;
> + nsamples -= 16;
> + }
> +
> + if (nsamples == 8) {
> + position -= 8;
> + if (nchannels > 0) {
> + int16_t *x = &X[0][position];
> + x[-7] = PCM(0 + 7 * nchannels);
> + x[1] = PCM(0 + 3 * nchannels);
> + x[2] = PCM(0 + 6 * nchannels);
> + x[3] = PCM(0 + 0 * nchannels);
> + x[4] = PCM(0 + 5 * nchannels);
> + x[5] = PCM(0 + 1 * nchannels);
> + x[6] = PCM(0 + 4 * nchannels);
> + x[7] = PCM(0 + 2 * nchannels);
> + }
> + /* mSBC is designed for 1 channel */
Same here.
> }
> #undef PCM
Well, none of these are really serious issues which could
immediately affect end users. But I would definitely prefer more
verbose commit messages, showing that you know what you are
doing, that the corner cases are under control, the rationale for
your decisions, etc.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply
* Re: pull request: bluetooth 2012-11-09
From: John W. Linville @ 2012-11-14 19:18 UTC (permalink / raw)
To: Gustavo Padovan, linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <20121110182427.GA7523@joana>
On Sat, Nov 10, 2012 at 07:24:27PM +0100, Gustavo Padovan wrote:
> Hi John,
>
> A few important fixes to go into 3.7. There is a new hw support by Marcos
> Chaparro. Johan added a memory leak fix and hci device index list fix.
> Also Marcel fixed a race condition in the device set up that was prevent the
> bt monitor to work properly. Last, Paulo Sérgio added a fix to the error
> status when pairing for LE fails. This was prevent userspace to work to handle
> the failure properly.
>
> Please pull! or let me know of any issues. Thanks.
>
> Gustavo
>
> ---
> The following changes since commit 6fe7cc71bbf3a0bc28c9cec3c00bc11e81344412:
>
> ath9k: Test for TID only in BlockAcks while checking tx status (2012-10-30 15:58:54 -0400)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth master
Pulled...
> for you to fetch changes up to 482049f75750d73358e65236b933417b69f9cc25:
>
> Bluetooth: Fix memory leak when removing a UUID (2012-11-09 16:45:37 +0100)
>
> ----------------------------------------------------------------
> Johan Hedberg (2):
> Bluetooth: Fix having bogus entries in mgmt_read_index_list reply
> Bluetooth: Fix memory leak when removing a UUID
>
> Marcel Holtmann (1):
> Bluetooth: Notify about device registration before power on
>
> Marcos Chaparro (1):
> Bluetooth: ath3k: Add support for VAIO VPCEH [0489:e027]
>
> Paulo Sérgio (1):
> Bluetooth: Fix error status when pairing fails
>
> drivers/bluetooth/ath3k.c | 1 +
> drivers/bluetooth/btusb.c | 1 +
> net/bluetooth/hci_core.c | 4 ++--
> net/bluetooth/mgmt.c | 12 +++++++-----
> net/bluetooth/smp.c | 2 +-
> 5 files changed, 12 insertions(+), 8 deletions(-)
>
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH v4 09/16] sbc: Use simd primitive if doing msbc on neon
From: Siarhei Siamashka @ 2012-11-14 19:27 UTC (permalink / raw)
To: Frédéric Dalleau; +Cc: linux-bluetooth
In-Reply-To: <1351589975-22640-10-git-send-email-frederic.dalleau@linux.intel.com>
On Tue, 30 Oct 2012 10:39:28 +0100
Frédéric Dalleau <frederic.dalleau@linux.intel.com> wrote:
> ---
> sbc/sbc_primitives.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/sbc/sbc_primitives.c b/sbc/sbc_primitives.c
> index a4767ef..9311848 100644
> --- a/sbc/sbc_primitives.c
> +++ b/sbc/sbc_primitives.c
> @@ -568,5 +568,8 @@ void sbc_init_primitives(struct sbc_encoder_state *state)
> #endif
> #ifdef SBC_BUILD_WITH_NEON_SUPPORT
> sbc_init_primitives_neon(state);
> +
> + if (state->increment == 1)
> + state->sbc_analyze_8s = sbc_analyze_1b_8s_simd;
> #endif
> }
This is not enough. As I commented earlier in
http://permalink.gmane.org/gmane.linux.bluez.kernel/31567
"neon code also provides optimized "sbc_enc_process_input_*" functions,
which are not going to work correctly for mSBC:
state->sbc_enc_process_input_8s_le = sbc_enc_process_input_8s_le_neon;
state->sbc_enc_process_input_8s_be = sbc_enc_process_input_8s_be_neon;"
And if we had, for example, an SSSE3 implementation of
"sbc_enc_process_input_*" functions for x86 (via PSHUFB instruction),
then it would also have the same problem.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply
* Re: pull request: bluetooth-next 2012-11-01
From: John W. Linville @ 2012-11-14 19:51 UTC (permalink / raw)
To: Gustavo Padovan, linux-wireless, linux-bluetooth, linux-kernel
In-Reply-To: <20121101223750.GA1915@joana>
On Thu, Nov 01, 2012 at 08:37:50PM -0200, Gustavo Padovan wrote:
> Hi John,
>
> Another set of patches for integration in wireless-next. There are two big set
> of changes in it: Andrei Emeltchenko and Mat Martineau added more patches
> towards a full Bluetooth High Speed support and Johan Hedberg improve the
> single mode support for Bluetooth dongles. Apart from that we have small fixes
> and improvements.
>
> Please pull. Thanks a lot.
>
> Gustavo
> ---
>
> The following changes since commit 9917c85b06c2eb9d61c0f2dadd2d5d8788f7e563:
>
> brcm80211: remove some truely barftastic code (2012-10-19 16:20:56 -0400)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next for-upstream
Pulled...
>
> for you to fetch changes up to 0c0afedf55ff409be9db0b0aeeaa1c6fe0f3cd3c:
>
> Bluetooth: Fix parameter order of hci_get_route (2012-11-01 20:27:11 -0200)
>
> ----------------------------------------------------------------
> Andrei Emeltchenko (18):
> Bluetooth: trivial: Remove unneeded assignment
> Bluetooth: Use helper function sending EFS conf rsp
> Bluetooth: AMP: Process Physical Link Complete evt
> Bluetooth: AMP: Process Logical Link complete evt
> Bluetooth: Add put(hcon) when deleting hchan
> Bluetooth: trivial: Fix braces style and remove empty line
> Bluetooth: Save hs_hchan instead of hs_hcon in loglink complete
> Bluetooth: Return correct L2CAP response type
> Bluetooth: Derive remote and local amp id from chan struct
> Bluetooth: AMP: Add Logical Link Create function
> Bluetooth: AMP: Process Disc Logical Link
> Bluetooth: AMP: Process Disc Physical Link Complete evt
> Bluetooth: AMP: Remove hci_conn receiving error command status
> Bluetooth: Disconnect logical link when deleting chan
> Bluetooth: AMP: Check for hs_hcon instead of ctrl_id
> Bluetooth: AMP: Use l2cap_physical_cfm in phylink complete evt
> Bluetooth: Process Create Chan Request
> Bluetooth: Rename ctrl_id to remote_amp_id
>
> Denis Kirjanov (1):
> Bluetooth:Replace list_for_each with list_for_each_entry() helper
>
> Gustavo Padovan (1):
> Bluetooth: Replace *_init() for *_setup()
>
> Johan Hedberg (16):
> Bluetooth: Add initial support for LE-only controllers
> Bluetooth: Fix LE MTU reporting for HCIGETDEVINFO
> Bluetooth: Add setting of the LE event mask
> Bluetooth: Read adversiting channel TX power during init sequence
> Bluetooth: Fix HCI command sending when powering on LE-only adapters
> Bluetooth: mgmt: Restrict BR/EDR settings to BR/EDR-only adapters
> Bluetooth: Fix updating host feature bits for LE
> Bluetooth: Add missing feature test macros
> Bluetooth: Make use feature test macros
> Bluetooth: Add flag for LE GAP Peripheral role
> Bluetooth: Disallow LE scanning and connecting in peripheral role
> Bluetooth: Fix setting host feature bits for SSP
> Bluetooth: Fix sending unnecessary HCI_Write_SSP_Mode command
> Bluetooth: Fix unnecessary EIR update during powering on
> Bluetooth: Fix sending unnecessary HCI_LE_Host_Enable
> Bluetooth: Fix parameter order of hci_get_route
>
> Mat Martineau (18):
> Bluetooth: Add new l2cap_chan struct members for high speed channels
> Bluetooth: Add L2CAP create channel request handling
> Bluetooth: Remove unnecessary intermediate function
> Bluetooth: Lookup channel structure based on DCID
> Bluetooth: Channel move request handling
> Bluetooth: Add new ERTM receive states for channel move
> Bluetooth: Add move channel confirm handling
> Bluetooth: Add state to hci_chan
> Bluetooth: Move channel response
> Bluetooth: Add logical link confirm
> Bluetooth: Add move confirm response handling
> Bluetooth: Handle physical link completion
> Bluetooth: Flag ACL frames as complete for AMP controllers
> Bluetooth: Do not send data during channel move
> Bluetooth: Configure appropriate timeouts for AMP controllers
> Bluetooth: Ignore BR/EDR packet size constraints when fragmenting for AMP
> Bluetooth: Do not retransmit data during a channel move
> Bluetooth: Start channel move when socket option is changed
>
> Syam Sidhardhan (4):
> Bluetooth: trivial: Remove newline before EOF
> Bluetooth: Replace include linux/module.h with linux/export.h
> Bluetooth: Remove unnecessary include export.h
> Bluetooth: mgmt: Use __constant when dealing with constants
>
> include/net/bluetooth/amp.h | 4 +
> include/net/bluetooth/hci.h | 7 +
> include/net/bluetooth/hci_core.h | 44 +-
> include/net/bluetooth/l2cap.h | 38 +-
> net/bluetooth/Kconfig | 1 -
> net/bluetooth/a2mp.c | 4 +-
> net/bluetooth/amp.c | 93 ++++
> net/bluetooth/bnep/netdev.c | 1 -
> net/bluetooth/cmtp/capi.c | 2 +-
> net/bluetooth/cmtp/sock.c | 2 +-
> net/bluetooth/hci_conn.c | 6 +
> net/bluetooth/hci_core.c | 65 +--
> net/bluetooth/hci_event.c | 309 +++++++++++--
> net/bluetooth/l2cap_core.c | 1000 +++++++++++++++++++++++++++++++++++++---
> net/bluetooth/l2cap_sock.c | 5 +
> net/bluetooth/mgmt.c | 98 ++--
> 16 files changed, 1486 insertions(+), 193 deletions(-)
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: [PATCH v4 00/16] mSBC tests
From: Siarhei Siamashka @ 2012-11-14 19:57 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: frederic.dalleau
In-Reply-To: <1352904655.20338.11.camel@aeonflux>
On Wed, 14 Nov 2012 23:50:55 +0900
Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Fred,
>
> > > v4 integrate latest comments from Siarhei:
> > > - remove state->pending field from sbc_encoder_state
> > > - add armv6 and iwmmxt primitives
> > > - use simd primitive instead of neon
> > > - reorder patches so that the SBC_MSBC flag is exposed to users only when
> > > implementation is complete.
> >
> > Any feedback ?
>
> beside a tiny cosmetic comment, I have nothing. However I am not the
> expert here. If Siarhei looks over this and is fine with it, I will be
> as well.
Hi,
My biggest concern is the way how we handle the mSBC API/ABI extension.
Because once the new version of sbc library is out, we can't
(easily/painlessly) change it any more. Everything else is fixable.
Anyway, as far as I can see, there are no regressions in the existing
SBC functionality, which is the most important requirement.
I personally would prefer a bit more verbose commit messages. But let
me know if these demands are unreasonable :)
--
Best regards,
Siarhei Siamashka
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox