Fix sbc negotiation and improves buffer handling by using GstAdapter.

7 files changed, 213 insertions, 67 deletions

diff --git a/audio/gsta2dpsink.c b/audio/gsta2dpsink.c
index be81efe8..e605158f 100644
--- a/audio/gsta2dpsink.c
+++ b/audio/gsta2dpsink.c
@@ -37,6 +37,7 @@
 
 #include "ipc.h"
 #include "rtp.h"
+#include "gstsbcutil.h"
 
 #include "gsta2dpsink.h"
 
@@ -143,10 +144,17 @@ static gboolean gst_a2dp_sink_stop(GstBaseSink *basesink)
 		self->watch_id = 0;
 	}
 
+	if (self->stream) {
+		g_io_channel_flush(self->stream, NULL);
+		g_io_channel_close(self->stream);
+		g_io_channel_unref(self->stream);
+		self->stream = NULL;
+	}
+
 	if (self->server) {
 		g_io_channel_close(self->server);
 		g_io_channel_unref(self->server);
-		self->stream = NULL;
+		self->server = NULL;
 	}
 
 	if (self->data) {
@@ -154,6 +162,16 @@ static gboolean gst_a2dp_sink_stop(GstBaseSink *basesink)
 		self->data = NULL;
 	}
 
+	if (self->sbc) {
+		g_free(self->sbc);
+		self->sbc = NULL;
+	}
+
+	if (self->dev_caps) {
+		gst_caps_unref(self->dev_caps);
+		self->dev_caps = NULL;
+	}
+
 	return TRUE;
 }
 
@@ -250,6 +268,25 @@ static gint gst_a2dp_sink_bluetooth_recvmsg_fd(GstA2dpSink *sink)
 	return -EINVAL;
 }
 
+static void gst_a2dp_sink_check_dev_caps(GstA2dpSink *self)
+{
+	GstStructure *structure;
+	GstCaps *dev_caps;
+	gint channels;
+
+	structure = gst_caps_get_structure(self->dev_caps, 0);
+	if (!gst_structure_get_int(structure, "channels", &channels))
+		channels = 2; /* FIXME how to get channels */
+	dev_caps = gst_sbc_caps_from_sbc(&(self->data->cfg), self->sbc,
+			channels);
+
+	self->new_dev_caps = TRUE;
+	gst_caps_unref(self->dev_caps);
+	self->dev_caps = gst_caps_ref(dev_caps);
+
+
+}
+
 static int gst_a2dp_sink_bluetooth_a2dp_init(GstA2dpSink *self,
 			struct ipc_codec_sbc *sbc)
 {
@@ -406,6 +443,9 @@ static gboolean gst_a2dp_sink_conf_resp(GstA2dpSink *sink)
 	}
 
 	memcpy(&sink->data->cfg, cfg, sizeof(*cfg));
+	memcpy(sink->sbc, sbc, sizeof(struct ipc_codec_sbc));
+
+	gst_a2dp_sink_check_dev_caps(sink);
 
 	GST_DEBUG_OBJECT(sink, "Device configuration:\n\tchannel=%p\n\t"
 			"fd_opt=%u\n\tpkt_len=%u\n\tsample_size=%u\n\trate=%u",
@@ -414,6 +454,7 @@ static gboolean gst_a2dp_sink_conf_resp(GstA2dpSink *sink)
 			sink->data->cfg.rate);
 
 	if (sink->data->cfg.codec == CFG_CODEC_SBC) {
+		/* FIXME is this necessary? */
 		ret = gst_a2dp_sink_bluetooth_a2dp_init(sink, sbc);
 		if (ret < 0)
 			return FALSE;
@@ -577,8 +618,12 @@ static gboolean gst_a2dp_sink_start(GstBaseSink *basesink)
 	self->data = g_new0(struct bluetooth_data, 1);
 	memset(self->data, 0, sizeof(struct bluetooth_data));
 
+	self->sbc = g_new0(struct ipc_codec_sbc, 1);
+
 	self->stream = NULL;
 	self->con_state = NOT_CONFIGURED;
+	self->new_dev_caps = FALSE;
+	self->dev_caps = NULL;
 
 	self->waiting_con_conf = FALSE;
 
@@ -718,8 +763,16 @@ static gboolean gst_a2dp_sink_set_caps(GstBaseSink *basesink, GstCaps *caps)
 	GstA2dpSink *self = GST_A2DP_SINK(basesink);
 
 	GST_A2DP_SINK_MUTEX_LOCK(self);
-	if (self->con_state == NOT_CONFIGURED)
+	if (self->con_state == NOT_CONFIGURED) {
 		gst_a2dp_sink_start_dev_conf(self, caps);
+
+		if (self->dev_caps)
+			gst_caps_unref(self->dev_caps);
+		self->dev_caps = gst_caps_ref(caps);
+
+		/* we suppose the device will accept this caps */
+		self->new_dev_caps = FALSE;
+	}
 	GST_A2DP_SINK_MUTEX_UNLOCK(self);
 
 	return TRUE;
@@ -735,6 +788,30 @@ static gboolean gst_a2dp_sink_unlock(GstBaseSink *basesink)
 	return TRUE;
 }
 
+static GstFlowReturn gst_a2dp_sink_buffer_alloc(GstBaseSink *basesink,
+				guint64 offset, guint size, GstCaps* caps,
+				GstBuffer **buf)
+{
+	GstA2dpSink *self = GST_A2DP_SINK(basesink);
+
+	*buf = gst_buffer_new_and_alloc(size);
+	if (!(*buf)) {
+		GST_ERROR_OBJECT(self, "buffer allocation failed");
+		return GST_FLOW_ERROR;
+	}
+
+	if (self->new_dev_caps && self->dev_caps) {
+		GST_INFO_OBJECT(self, "new caps from device");
+		gst_buffer_set_caps(*buf, self->dev_caps);
+		self->new_dev_caps = FALSE;
+	} else
+		gst_buffer_set_caps(*buf, caps);
+
+	GST_BUFFER_OFFSET(*buf) = offset;
+
+	return GST_FLOW_OK;
+}
+
 static void gst_a2dp_sink_class_init(GstA2dpSinkClass *klass)
 {
 	GObjectClass *object_class = G_OBJECT_CLASS(klass);
@@ -754,6 +831,8 @@ static void gst_a2dp_sink_class_init(GstA2dpSinkClass *klass)
 	basesink_class->preroll = GST_DEBUG_FUNCPTR(gst_a2dp_sink_preroll);
 	basesink_class->set_caps = GST_DEBUG_FUNCPTR(gst_a2dp_sink_set_caps);
 	basesink_class->unlock = GST_DEBUG_FUNCPTR(gst_a2dp_sink_unlock);
+	basesink_class->buffer_alloc =
+		GST_DEBUG_FUNCPTR(gst_a2dp_sink_buffer_alloc);
 
 	g_object_class_install_property(object_class, PROP_DEVICE,
 					g_param_spec_string("device", "Device",
@@ -768,6 +847,7 @@ static void gst_a2dp_sink_init(GstA2dpSink *self, GstA2dpSinkClass *klass)
 {
 	self->device = NULL;
 	self->data = NULL;
+	self->sbc = NULL;
 
 	self->stream = NULL;
 	self->con_state = NOT_CONFIGURED;
diff --git a/audio/gsta2dpsink.h b/audio/gsta2dpsink.h
index 76d512b9..ea750406 100644
--- a/audio/gsta2dpsink.h
+++ b/audio/gsta2dpsink.h
@@ -49,9 +49,12 @@ struct _GstA2dpSink {
 	GIOChannel *stream;
 
 	struct bluetooth_data *data;
+	struct ipc_codec_sbc *sbc;
 	GIOChannel *server;
 
 	gint con_state;
+	GstCaps *dev_caps;
+	gboolean new_dev_caps;
 
 	GCond *con_conf_end;
 	gboolean waiting_con_conf;
diff --git a/audio/gstsbcenc.c b/audio/gstsbcenc.c
index 681a2291..0e9daed1 100644
--- a/audio/gstsbcenc.c
+++ b/audio/gstsbcenc.c
@@ -115,7 +115,6 @@ static GstStaticPadTemplate sbc_enc_src_factory =
 				"allocation = (string) { snr, loudness },"
 				"bitpool = (int) [ 2, 64 ]"));
 
-
 static GstCaps* sbc_enc_generate_srcpad_caps(GstSbcEnc *enc, GstCaps *caps)
 {
 	gint rate;
@@ -188,24 +187,83 @@ error:
 	return FALSE;
 }
 
+gboolean gst_sbc_enc_fill_sbc_params(GstSbcEnc *enc, GstCaps *caps)
+{
+	GstStructure *structure;
+	gint rate, channels, subbands, blocks, bitpool;
+	const gchar* mode;
+	const gchar* allocation;
+
+	g_assert(gst_caps_is_fixed(caps));
+
+	structure = gst_caps_get_structure(caps, 0);
+
+	if (!gst_structure_get_int(structure, "rate", &rate))
+		return FALSE;
+	if (!gst_structure_get_int(structure, "channels", &channels))
+		return FALSE;
+	if (!gst_structure_get_int(structure, "subbands", &subbands))
+		return FALSE;
+	if (!gst_structure_get_int(structure, "blocks", &blocks))
+		return FALSE;
+	if (!gst_structure_get_int(structure, "bitpool", &bitpool))
+		return FALSE;
+
+	if (!(mode = gst_structure_get_string(structure, "mode")))
+		return FALSE;
+	if (!(allocation = gst_structure_get_string(structure, "allocation")))
+		return FALSE;
+
+	enc->sbc.rate = rate;
+	enc->sbc.channels = channels;
+	enc->blocks = blocks;
+	enc->sbc.subbands = subbands;
+	enc->sbc.bitpool = bitpool;
+	enc->mode = gst_sbc_get_mode_int(mode);
+	enc->allocation = gst_sbc_get_allocation_mode_int(allocation);
+
+	return TRUE;
+}
+
+static gboolean gst_sbc_enc_change_caps(GstSbcEnc *enc, GstCaps *caps)
+{
+	GST_INFO_OBJECT(enc, "Changing srcpad caps (renegotiation)");
+
+	if (!gst_pad_accept_caps(enc->srcpad, caps)) {
+		GST_WARNING_OBJECT(enc, "Src pad refused caps");
+		return FALSE;
+	}
+
+	if (!gst_sbc_enc_fill_sbc_params(enc, caps)) {
+		GST_ERROR_OBJECT(enc, "couldn't get sbc parameters from caps");
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static GstFlowReturn sbc_enc_chain(GstPad *pad, GstBuffer *buffer)
 {
 	GstSbcEnc *enc = GST_SBC_ENC(gst_pad_get_parent(pad));
+	GstAdapter *adapter = enc->adapter;
 	GstFlowReturn res = GST_FLOW_OK;
-	guint size, offset = 0;
-	guint8 *data;
+	gint codesize = enc->sbc.subbands * enc->sbc.blocks * enc->sbc.channels * 2;
 
-	data = GST_BUFFER_DATA(buffer);
-	size = GST_BUFFER_SIZE(buffer);
+	gst_adapter_push(adapter, buffer);
 
-	while (offset < size) {
+	while (gst_adapter_available(adapter) >= codesize && res == GST_FLOW_OK) {
 		GstBuffer *output;
 		GstCaps *caps;
+		const guint8 *data;
 		int consumed;
 
-		consumed = sbc_encode(&enc->sbc, data + offset, size - offset);
-		if (consumed <= 0)
+		data = gst_adapter_peek(adapter, codesize);
+		consumed = sbc_encode(&enc->sbc, (gpointer) data, codesize);
+		if (consumed <= 0) {
+			GST_ERROR ("comsumed < 0, codesize: %d", codesize);
 			break;
+		}
+		gst_adapter_flush(adapter, consumed);
 
 		caps = GST_PAD_CAPS(enc->srcpad);
 
@@ -216,21 +274,26 @@ static GstFlowReturn sbc_enc_chain(GstPad *pad, GstBuffer *buffer)
 		if (res != GST_FLOW_OK)
 			goto done;
 
+		if (!gst_caps_is_equal(caps, GST_BUFFER_CAPS(output)))
+			if (!gst_sbc_enc_change_caps(enc,
+					GST_BUFFER_CAPS(output))) {
+				res = GST_FLOW_ERROR;
+				GST_ERROR_OBJECT(enc, "couldn't renegotiate caps");
+				goto done;
+		}
+
 		memcpy(GST_BUFFER_DATA(output), enc->sbc.data, enc->sbc.len);
 		GST_BUFFER_TIMESTAMP(output) = GST_BUFFER_TIMESTAMP(buffer);
 
 		res = gst_pad_push(enc->srcpad, output);
-		if (res != GST_FLOW_OK)
+		if (res != GST_FLOW_OK) {
+			GST_ERROR_OBJECT(enc, "pad pushing failed");
 			goto done;
+		}
 
-		offset += consumed;
 	}
 
-	if (offset < size)
-		res = GST_FLOW_ERROR;
-
 done:
-	gst_buffer_unref(buffer);
 	gst_object_unref(enc);
 
 	return res;
@@ -259,6 +322,16 @@ static GstStateChangeReturn sbc_enc_change_state(GstElement *element,
 	return parent_class->change_state(element, transition);
 }
 
+static void gst_sbc_enc_dispose(GObject *object)
+{
+	GstSbcEnc *enc = GST_SBC_ENC(object);
+
+	if (enc->adapter != NULL)
+		g_object_unref (G_OBJECT (enc->adapter));
+
+	enc->adapter = NULL;
+}
+
 static void gst_sbc_enc_base_init(gpointer g_class)
 {
 	GstElementClass *element_class = GST_ELEMENT_CLASS(g_class);
@@ -333,6 +406,7 @@ static void gst_sbc_enc_class_init(GstSbcEncClass *klass)
 
 	object_class->set_property = GST_DEBUG_FUNCPTR(gst_sbc_enc_set_property);
 	object_class->get_property = GST_DEBUG_FUNCPTR(gst_sbc_enc_get_property);
+	object_class->dispose = GST_DEBUG_FUNCPTR(gst_sbc_enc_dispose);
 
 	element_class->change_state = GST_DEBUG_FUNCPTR(sbc_enc_change_state);
 
@@ -375,4 +449,6 @@ static void gst_sbc_enc_init(GstSbcEnc *self, GstSbcEncClass *klass)
 	self->blocks = SBC_ENC_DEFAULT_BLOCKS;
 	self->mode = SBC_ENC_DEFAULT_MODE;
 	self->allocation = SBC_ENC_DEFAULT_ALLOCATION;
+
+	self->adapter = gst_adapter_new();
 }
diff --git a/audio/gstsbcenc.h b/audio/gstsbcenc.h
index 0a95bcef..c5fc6bcc 100644
--- a/audio/gstsbcenc.h
+++ b/audio/gstsbcenc.h
@@ -22,6 +22,7 @@
  */
 
 #include <gst/gst.h>
+#include <gst/base/gstadapter.h>
 
 #include "sbc.h"
 #include "ipc.h"
@@ -48,6 +49,7 @@ struct _GstSbcEnc {
 
 	GstPad *sinkpad;
 	GstPad *srcpad;
+	GstAdapter *adapter;
 
 	gint mode;
 	gint blocks;
diff --git a/audio/gstsbcparse.c b/audio/gstsbcparse.c
index 42ae9550..185cda03 100644
--- a/audio/gstsbcparse.c
+++ b/audio/gstsbcparse.c
@@ -78,8 +78,6 @@ static GstCaps* sbc_parse_select_caps(GstSbcParse *parse, GstCaps *caps)
 		value = gst_structure_get_value(structure, "rate");
 		if (GST_VALUE_HOLDS_LIST(value)) {
 			temp = gst_sbc_select_rate_from_list(value);
-		} else if (GST_VALUE_HOLDS_INT_RANGE(value)) {
-			temp = gst_sbc_select_rate_from_range(value);
 		} else {
 			temp = g_value_get_int(value);
 		}
@@ -92,9 +90,7 @@ static GstCaps* sbc_parse_select_caps(GstSbcParse *parse, GstCaps *caps)
 		goto error;
 	} else {
 		value = gst_structure_get_value(structure, "channels");
-		if (GST_VALUE_HOLDS_LIST(value)) {
-			temp = gst_sbc_select_channels_from_list(value);
-		} else if (GST_VALUE_HOLDS_INT_RANGE(value)) {
+		if (GST_VALUE_HOLDS_INT_RANGE(value)) {
 			temp = gst_sbc_select_channels_from_range(value);
 		} else {
 			temp = g_value_get_int(value);
@@ -110,8 +106,6 @@ static GstCaps* sbc_parse_select_caps(GstSbcParse *parse, GstCaps *caps)
 		value = gst_structure_get_value(structure, "blocks");
 		if (GST_VALUE_HOLDS_LIST(value)) {
 			temp = gst_sbc_select_blocks_from_list(value);
-		} else if (GST_VALUE_HOLDS_INT_RANGE(value)) {
-			temp = gst_sbc_select_blocks_from_range(value);
 		} else {
 			temp = g_value_get_int(value);
 		}
@@ -126,8 +120,6 @@ static GstCaps* sbc_parse_select_caps(GstSbcParse *parse, GstCaps *caps)
 		value = gst_structure_get_value(structure, "subbands");
 		if (GST_VALUE_HOLDS_LIST(value)) {
 			temp = gst_sbc_select_subbands_from_list(value);
-		} else if (GST_VALUE_HOLDS_INT_RANGE(value)) {
-			temp = gst_sbc_select_subbands_from_range(value);
 		} else {
 			temp = g_value_get_int(value);
 		}
diff --git a/audio/gstsbcutil.c b/audio/gstsbcutil.c
index e55220c3..1d7a1227 100644
--- a/audio/gstsbcutil.c
+++ b/audio/gstsbcutil.c
@@ -39,25 +39,6 @@ gint gst_sbc_select_rate_from_list(const GValue *value)
 }
 
 /*
- * Selects one rate from a range of possible rates
- * TODO - use a better approach to this (it is selecting the maximum value)
- */
-gint gst_sbc_select_rate_from_range(const GValue *value)
-{
-	return gst_value_get_int_range_max(value);
-}
-
-/*
- * Selects one number of channels from a list of possible numbers
- * TODO - use a better approach to this (it is selecting the last element)
- */
-gint gst_sbc_select_channels_from_list(const GValue *value)
-{
-	guint size = gst_value_list_get_size(value);
-	return g_value_get_int(gst_value_list_get_value(value, size-1));
-}
-
-/*
  * Selects one number of channels option from a range of possible numbers
  * TODO - use a better approach to this (it is selecting the maximum value)
  */
@@ -77,15 +58,6 @@ gint gst_sbc_select_blocks_from_list(const GValue *value)
 }
 
 /*
- * Selects one blocks option from a range of possible blocks
- * TODO - use a better approach to this (it is selecting the maximum value)
- */
-gint gst_sbc_select_blocks_from_range(const GValue *value)
-{
-	return gst_value_get_int_range_max(value);
-}
-
-/*
  * Selects one number of subbands from a list
  * TODO - use a better approach to this (it is selecting the last element)
  */
@@ -96,15 +68,6 @@ gint gst_sbc_select_subbands_from_list(const GValue *value)
 }
 
 /*
- * Selects one subbands option from a range
- * TODO - use a better approach to this (it is selecting the maximum value)
- */
-gint gst_sbc_select_subbands_from_range(const GValue *value)
-{
-	return gst_value_get_int_range_max(value);
-}
-
-/*
  * Selects one bitpool option from a range
  * TODO - use a better approach to this (it is selecting the maximum value)
  */
@@ -187,8 +150,32 @@ const gchar *gst_sbc_get_allocation_string(int alloc)
 	case CFG_ALLOCATION_SNR:
 		return "snr";
 	case CFG_ALLOCATION_AUTO:
-		return NULL; /* TODO what should be selected here? */
+		return "loudness"; /* TODO what should be selected here? */
 	default:
 		return NULL;
 	}
 }
+
+GstCaps* gst_sbc_caps_from_sbc(struct ipc_data_cfg *cfg,
+		struct ipc_codec_sbc *sbc, gint channels)
+{
+	GstCaps *caps;
+	const gchar *mode_str;
+	const gchar *allocation_str;
+
+	mode_str = gst_sbc_get_mode_string(cfg->mode);
+	allocation_str = gst_sbc_get_allocation_string(sbc->allocation);
+
+	caps = gst_caps_new_simple("audio/x-sbc",
+				"rate", G_TYPE_INT, cfg->rate,
+				"channels", G_TYPE_INT, channels,
+				"mode", G_TYPE_STRING, mode_str,
+				"subbands", G_TYPE_INT, sbc->subbands,
+				"blocks", G_TYPE_INT, sbc->blocks,
+				"allocation", G_TYPE_STRING, allocation_str,
+				"bitpool", G_TYPE_INT, sbc->bitpool,
+				NULL);
+
+	return caps;
+}
+
diff --git a/audio/gstsbcutil.h b/audio/gstsbcutil.h
index 98f202f0..0c91fe82 100644
--- a/audio/gstsbcutil.h
+++ b/audio/gstsbcutil.h
@@ -22,18 +22,20 @@
  */
 
 #include <gst/gst.h>
+#include "sbc.h"
+
+struct ipc_data_cfg; /* FIXME can't include ipc.h */
+struct ipc_codec_sbc;
 
 gint gst_sbc_select_rate_from_list(const GValue *value);
-gint gst_sbc_select_rate_from_range(const GValue *value);
 
-gint gst_sbc_select_channels_from_list(const GValue *value);
 gint gst_sbc_select_channels_from_range(const GValue *value);
 
 gint gst_sbc_select_blocks_from_list(const GValue *value);
-gint gst_sbc_select_blocks_from_range(const GValue *value);
 
 gint gst_sbc_select_subbands_from_list(const GValue *value);
-gint gst_sbc_select_subbands_from_range(const GValue *value);
+
+gint gst_sbc_select_bitpool_from_range(const GValue *value);
 
 gint gst_sbc_select_bitpool_from_range(const GValue *value);
 
@@ -44,3 +46,7 @@ const gchar *gst_sbc_get_allocation_string(int alloc);
 const gchar *gst_sbc_get_mode_from_list(const GValue *value);
 gint gst_sbc_get_mode_int(const gchar *mode);
 const gchar *gst_sbc_get_mode_string(int joint);
+
+GstCaps* gst_sbc_caps_from_sbc(struct ipc_data_cfg *cfg, struct ipc_codec_sbc *sbc,
+		gint channels);
+