link: improve renegotiation0.1.0

Only suspend an idle node when we need to configure a different format.

20 files changed, 190 insertions, 38 deletions

diff --git a/meson.build b/meson.build
index 8fc4bb35..a6e08f50 100644
--- a/meson.build
+++ b/meson.build
@@ -1,5 +1,5 @@
 project('pipewire', 'c',
-  version : '0.0.1.1',
+  version : '0.1.0',
   meson_version : '>= 0.36.0',
   default_options : [ 'warning_level=1',
                       'c_std=gnu99',
diff --git a/pipewire/client/mem.c b/pipewire/client/mem.c
index fb8a2779..b4bd3d95 100644
--- a/pipewire/client/mem.c
+++ b/pipewire/client/mem.c
@@ -158,7 +158,7 @@ int pw_memblock_alloc(enum pw_memblock_flags flags, size_t size, struct pw_membl
 			return SPA_RESULT_ERRNO;
 		}
 #else
-		char filename[] = "/dev/shm/spa-tmpfile.XXXXXX";
+		char filename[] = "/dev/shm/pipewire-tmpfile.XXXXXX";
 		mem->fd = mkostemp(filename, O_CLOEXEC);
 		if (mem->fd == -1) {
 			pw_log_error("Failed to create temporary file: %s\n", strerror(errno));
diff --git a/pipewire/server/client-node.c b/pipewire/server/client-node.c
index 47db23ea..dc3956a3 100644
--- a/pipewire/server/client-node.c
+++ b/pipewire/server/client-node.c
@@ -30,6 +30,7 @@
 
 #include "spa/node.h"
 #include "spa/format-builder.h"
+#include "spa/lib/format.h"
 
 #include "pipewire/client/pipewire.h"
 #include "pipewire/client/interfaces.h"
diff --git a/pipewire/server/core.c b/pipewire/server/core.c
index 3d9e93af..35103f81 100644
--- a/pipewire/server/core.c
+++ b/pipewire/server/core.c
@@ -611,6 +611,7 @@ struct spa_format *pw_core_find_format(struct pw_core *core,
 
 	pw_log_debug("core %p: finding best format %d %d", core, out_state, in_state);
 
+	/* when a port is configured but the node is idle, we can reconfigure with a different format */
 	if (out_state > PW_PORT_STATE_CONFIGURE && output->node->info.state == PW_NODE_STATE_IDLE)
 		out_state = PW_PORT_STATE_CONFIGURE;
 	if (in_state > PW_PORT_STATE_CONFIGURE && input->node->info.state == PW_NODE_STATE_IDLE)
diff --git a/pipewire/server/core.h b/pipewire/server/core.h
index 683e91df..d21b7efd 100644
--- a/pipewire/server/core.h
+++ b/pipewire/server/core.h
@@ -51,6 +51,9 @@ struct pw_global;
  *
  * \subpage page_resource
  *
+ * \subpage page_node
+ *
+ * \subpage page_link
  */
 
 /** \page page_core Core
diff --git a/pipewire/server/link.c b/pipewire/server/link.c
index 0c862f4e..6ae61fb4 100644
--- a/pipewire/server/link.c
+++ b/pipewire/server/link.c
@@ -23,6 +23,7 @@
 #include <spa/video/format.h>
 #include <spa/pod-utils.h>
 
+#include <spa/lib/format.h>
 #include <spa/lib/props.h>
 
 #include "pipewire/client/pipewire.h"
@@ -108,7 +109,7 @@ static int do_negotiate(struct pw_link *this, uint32_t in_state, uint32_t out_st
 {
 	struct impl *impl = SPA_CONTAINER_OF(this, struct impl, this);
 	int res = SPA_RESULT_ERROR, res2;
-	struct spa_format *format;
+	struct spa_format *format, *current;
 	char *error = NULL;
 
 	if (in_state != PW_PORT_STATE_CONFIGURE && out_state != PW_PORT_STATE_CONFIGURE)
@@ -120,13 +121,39 @@ static int do_negotiate(struct pw_link *this, uint32_t in_state, uint32_t out_st
 	if (format == NULL)
 		goto error;
 
+	format = spa_format_copy(format);
+
 	if (out_state > PW_PORT_STATE_CONFIGURE && this->output->node->info.state == PW_NODE_STATE_IDLE) {
-		pw_node_set_state(this->output->node, PW_NODE_STATE_SUSPENDED);
-		out_state = PW_PORT_STATE_CONFIGURE;
+		if ((res = spa_node_port_get_format(this->output->node->node,
+						    SPA_DIRECTION_OUTPUT,
+						    this->output->port_id,
+						    (const struct spa_format **) &current)) < 0) {
+			asprintf(&error, "error get output format: %d", res);
+			goto error;
+		}
+		if (spa_format_compare(current, format) < 0) {
+			pw_log_debug("link %p: output format change, renegotiate", this);
+			pw_node_set_state(this->output->node, PW_NODE_STATE_SUSPENDED);
+			out_state = PW_PORT_STATE_CONFIGURE;
+		}
+		else
+			pw_node_update_state(this->output->node, PW_NODE_STATE_RUNNING, NULL);
 	}
 	if (in_state > PW_PORT_STATE_CONFIGURE && this->input->node->info.state == PW_NODE_STATE_IDLE) {
-		pw_node_set_state(this->input->node, PW_NODE_STATE_SUSPENDED);
-		in_state = PW_PORT_STATE_CONFIGURE;
+		if ((res = spa_node_port_get_format(this->input->node->node,
+						    SPA_DIRECTION_INPUT,
+						    this->input->port_id,
+						    (const struct spa_format **) &current)) < 0) {
+			asprintf(&error, "error get input format: %d", res);
+			goto error;
+		}
+		if (spa_format_compare(current, format) < 0) {
+			pw_log_debug("link %p: input format change, renegotiate", this);
+			pw_node_set_state(this->input->node, PW_NODE_STATE_SUSPENDED);
+			in_state = PW_PORT_STATE_CONFIGURE;
+		}
+		else
+			pw_node_update_state(this->input->node, PW_NODE_STATE_RUNNING, NULL);
 	}
 
 	pw_log_debug("link %p: doing set format", this);
@@ -157,13 +184,18 @@ static int do_negotiate(struct pw_link *this, uint32_t in_state, uint32_t out_st
 		pw_work_queue_add(impl->work, this->input->node, res2, complete_ready, this->input);
 		res = res2 != SPA_RESULT_OK ? res2 : res;
 	}
+
+	if (this->info.format)
+		free(this->info.format);
+	this->info.format = format;
+
 	return res;
 
       error:
-	{
-		pw_link_update_state(this, PW_LINK_STATE_ERROR, error);
-		return res;
-	}
+	pw_link_update_state(this, PW_LINK_STATE_ERROR, error);
+	if (format)
+		free(format);
+	return res;
 }
 
 static struct spa_param *find_param(struct spa_param **params, int n_params, uint32_t type)
@@ -637,16 +669,14 @@ static int do_allocation(struct pw_link *this, uint32_t in_state, uint32_t out_s
 	return res;
 
       error:
-	{
-		this->output->buffers = NULL;
-		this->output->n_buffers = 0;
-		this->output->allocated = false;
-		this->input->buffers = NULL;
-		this->input->n_buffers = 0;
-		this->input->allocated = false;
-		pw_link_update_state(this, PW_LINK_STATE_ERROR, error);
-		return res;
-	}
+	this->output->buffers = NULL;
+	this->output->n_buffers = 0;
+	this->output->allocated = false;
+	this->input->buffers = NULL;
+	this->input->n_buffers = 0;
+	this->input->allocated = false;
+	pw_link_update_state(this, PW_LINK_STATE_ERROR, error);
+	return res;
 }
 
 static int do_start(struct pw_link *this, uint32_t in_state, uint32_t out_state)
@@ -810,6 +840,9 @@ static void pw_link_free(struct pw_link *link)
 
 	pw_work_queue_destroy(impl->work);
 
+	if (link->info.format)
+		free(link->info.format);
+
 	if (impl->buffer_owner == link)
 		pw_memblock_free(&impl->buffer_mem);
 
@@ -997,14 +1030,10 @@ do_link_remove(struct spa_loop *loop,
 
 	if (this->rt.input) {
 		spa_list_remove(&this->rt.input_link);
-		if (spa_list_is_empty(&this->rt.input->rt.links))
-			pw_port_pause_rt(this->rt.input);
 		this->rt.input = NULL;
 	}
 	if (this->rt.output) {
 		spa_list_remove(&this->rt.output_link);
-		if (spa_list_is_empty(&this->rt.output->rt.links))
-			pw_port_pause_rt(this->rt.output);
 		this->rt.output = NULL;
 	}
 
diff --git a/pipewire/server/link.h b/pipewire/server/link.h
index 30e7e922..a1d98e66 100644
--- a/pipewire/server/link.h
+++ b/pipewire/server/link.h
@@ -32,6 +32,17 @@ extern "C" {
 #include <pipewire/server/port.h>
 #include <pipewire/server/main-loop.h>
 
+/** \page page_link Link
+ *
+ * \section page_link_overview Overview
+ *
+ * A link is the connection between 2 nodes (\ref page_node). Nodes are
+ * linked together on ports.
+ *
+ * The link is responsible for negotiating the format and buffers for
+ * the nodes.
+ */
+
 /** \class pw_link
  *
  * PipeWire link interface.
diff --git a/pipewire/server/node.h b/pipewire/server/node.h
index 66eaf08c..6b326ce7 100644
--- a/pipewire/server/node.h
+++ b/pipewire/server/node.h
@@ -39,6 +39,16 @@ extern "C" {
 #include <pipewire/server/client.h>
 #include <pipewire/server/data-loop.h>
 
+/** \page page_node Node
+ *
+ * \section page_node_overview Overview
+ *
+ * The node object processes data. The node has a list of
+ * input and output ports (\ref page_port) on which it
+ * will receive and send out buffers respectively.
+ *
+ * The node wraps an SPA node object.
+ */
 /** \class pw_node
  *
  * PipeWire node class.
diff --git a/pipewire/server/port.c b/pipewire/server/port.c
index 1e032165..ccea2bff 100644
--- a/pipewire/server/port.c
+++ b/pipewire/server/port.c
@@ -162,15 +162,11 @@ struct pw_link *pw_port_link(struct pw_port *output_port,
 	return link;
 
       same_node:
-	{
-		asprintf(error, "can't link a node to itself");
-		return NULL;
-	}
+	asprintf(error, "can't link a node to itself");
+	return NULL;
       was_linked:
-	{
-		asprintf(error, "input port was already linked");
-		return NULL;
-	}
+	asprintf(error, "input port was already linked");
+	return NULL;
       no_mem:
 	return NULL;
 }
diff --git a/spa/include/spa/format-builder.h b/spa/include/spa/format-builder.h
index fc430804..8a7a44ce 100644
--- a/spa/include/spa/format-builder.h
+++ b/spa/include/spa/format-builder.h
@@ -54,11 +54,6 @@ spa_pod_builder_push_format(struct spa_pod_builder *builder,
 		SPA_POD_TYPE_ID,media_subtype,					\
 		__VA_ARGS__)
 
-int
-spa_format_filter(const struct spa_format *format,
-		  const struct spa_format *filter,
-		  struct spa_pod_builder *result);
-
 #ifdef __cplusplus
 }  /* extern "C" */
 #endif
diff --git a/spa/lib/format.c b/spa/lib/format.c
index a760ce18..6577065c 100644
--- a/spa/lib/format.c
+++ b/spa/lib/format.c
@@ -59,3 +59,20 @@ spa_format_filter(const struct spa_format *format,
 
 	return res;
 }
+
+int
+spa_format_compare(const struct spa_format *format1,
+		   const struct spa_format *format2)
+{
+	if (format1 == NULL || format2 == NULL)
+		return SPA_RESULT_INVALID_ARGUMENTS;
+
+	if (SPA_FORMAT_MEDIA_TYPE(format1) != SPA_FORMAT_MEDIA_TYPE(format2) ||
+	    SPA_FORMAT_MEDIA_SUBTYPE(format1) != SPA_FORMAT_MEDIA_SUBTYPE(format2))
+		return SPA_RESULT_INVALID_MEDIA_TYPE;
+
+	return spa_props_compare(SPA_POD_CONTENTS(struct spa_format, format1),
+				 SPA_POD_CONTENTS_SIZE(struct spa_format, format1),
+				 SPA_POD_CONTENTS(struct spa_format, format2),
+				 SPA_POD_CONTENTS_SIZE(struct spa_format, format2));
+}
diff --git a/spa/lib/format.h b/spa/lib/format.h
new file mode 100644
index 00000000..0b6ddc42
--- /dev/null
+++ b/spa/lib/format.h
@@ -0,0 +1,40 @@
+/* Simple Plugin API
+ * Copyright (C) 2016 Wim Taymans <wim.taymans@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library 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 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
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library 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.
+ */
+
+#ifndef __SPA_LIBFORMAT_H__
+#define __SPA_LIBFORMAT_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <spa/props.h>
+
+int spa_format_filter(const struct spa_format *format,
+		      const struct spa_format *filter,
+		      struct spa_pod_builder *result);
+
+int spa_format_compare(const struct spa_format *format1,
+		       const struct spa_format *format2);
+
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif /* __SPA_LIBFORMAT_H__ */
diff --git a/spa/lib/meson.build b/spa/lib/meson.build
index b6b34033..9acd00f0 100644
--- a/spa/lib/meson.build
+++ b/spa/lib/meson.build
@@ -1,5 +1,6 @@
 spalib_headers = [
   'debug.h',
+  'format.h',
   'props.h',
 ]
 
diff --git a/spa/lib/props.c b/spa/lib/props.c
index 52412353..9f7cf86a 100644
--- a/spa/lib/props.c
+++ b/spa/lib/props.c
@@ -310,3 +310,40 @@ spa_props_filter(struct spa_pod_builder *b,
 	}
 	return SPA_RESULT_OK;
 }
+
+int spa_props_compare(const struct spa_pod *props1,
+                      uint32_t props1_size,
+                      const struct spa_pod *props2,
+                      uint32_t props2_size)
+{
+	const struct spa_pod *pr;
+
+	SPA_POD_FOREACH(props1, props1_size, pr) {
+		struct spa_pod_prop *p1, *p2;
+		void *a1, *a2;
+
+		if (pr->type != SPA_POD_TYPE_PROP)
+			continue;
+
+		p1 = (struct spa_pod_prop *) pr;
+
+		if ((p2 = find_prop(props2, props2_size, p1->body.key)) == NULL)
+			return SPA_RESULT_INCOMPATIBLE_PROPS;
+
+		/* incompatible property types */
+		if (p1->body.value.type != p2->body.value.type)
+			return SPA_RESULT_INCOMPATIBLE_PROPS;
+
+		if (p1->body.flags & SPA_POD_PROP_FLAG_UNSET ||
+		    p2->body.flags & SPA_POD_PROP_FLAG_UNSET)
+			return SPA_RESULT_INCOMPATIBLE_PROPS;
+
+		a1 = SPA_MEMBER(p1, sizeof(struct spa_pod_prop), void);
+		a2 = SPA_MEMBER(p2, sizeof(struct spa_pod_prop), void);
+
+		if (compare_value(p1->body.value.type, a1, a2) != 0)
+			return SPA_RESULT_INCOMPATIBLE_PROPS;
+	}
+	return SPA_RESULT_OK;
+
+}
diff --git a/spa/lib/props.h b/spa/lib/props.h
index 0fbe26a8..f4351221 100644
--- a/spa/lib/props.h
+++ b/spa/lib/props.h
@@ -32,6 +32,10 @@ int spa_props_filter(struct spa_pod_builder *b,
 		     const struct spa_pod *filter,
 		     uint32_t filter_size);
 
+int spa_props_compare(const struct spa_pod *props1,
+		      uint32_t props1_size,
+		      const struct spa_pod *props2,
+		      uint32_t props2_size);
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/spa/plugins/alsa/alsa-utils.c b/spa/plugins/alsa/alsa-utils.c
index 9e7a9d4f..032a2f0a 100644
--- a/spa/plugins/alsa/alsa-utils.c
+++ b/spa/plugins/alsa/alsa-utils.c
@@ -10,6 +10,8 @@
 #include <sys/timerfd.h>
 
 #include <lib/debug.h>
+#include <lib/format.h>
+
 #include "alsa-utils.h"
 
 #define CHECK(s,msg) if ((err = (s)) < 0) { spa_log_error(state->log, msg ": %s", snd_strerror(err)); return err; }
diff --git a/spa/plugins/audiomixer/audiomixer.c b/spa/plugins/audiomixer/audiomixer.c
index 4ded9813..3ab086e8 100644
--- a/spa/plugins/audiomixer/audiomixer.c
+++ b/spa/plugins/audiomixer/audiomixer.c
@@ -26,6 +26,7 @@
 #include <spa/node.h>
 #include <spa/audio/format-utils.h>
 #include <spa/format-builder.h>
+#include <lib/format.h>
 #include <lib/props.h>
 
 #define NAME "audiomixer"
diff --git a/spa/plugins/audiotestsrc/audiotestsrc.c b/spa/plugins/audiotestsrc/audiotestsrc.c
index 13767f26..10615678 100644
--- a/spa/plugins/audiotestsrc/audiotestsrc.c
+++ b/spa/plugins/audiotestsrc/audiotestsrc.c
@@ -32,6 +32,7 @@
 #include <spa/list.h>
 #include <spa/audio/format-utils.h>
 #include <spa/format-builder.h>
+#include <lib/format.h>
 #include <lib/props.h>
 
 #define NAME "audiotestsrc"
diff --git a/spa/plugins/videotestsrc/videotestsrc.c b/spa/plugins/videotestsrc/videotestsrc.c
index 5d0b3265..35838160 100644
--- a/spa/plugins/videotestsrc/videotestsrc.c
+++ b/spa/plugins/videotestsrc/videotestsrc.c
@@ -33,6 +33,8 @@
 #include <spa/list.h>
 #include <spa/video/format-utils.h>
 #include <spa/format-builder.h>
+
+#include <lib/format.h>
 #include <lib/props.h>
 
 #define NAME "videotestsrc"
diff --git a/spa/plugins/volume/volume.c b/spa/plugins/volume/volume.c
index 6cdea266..23a2f98f 100644
--- a/spa/plugins/volume/volume.c
+++ b/spa/plugins/volume/volume.c
@@ -28,6 +28,7 @@
 #include <spa/format-builder.h>
 #include <spa/param-alloc.h>
 #include <lib/props.h>
+#include <lib/format.h>
 
 #define NAME "volume"