output-capture: Allow multiple formats and add formats_done event

The writeback output capture source may allow clients to select from
multiple possible formats. Update the protocol and its users
accordingly, and add formats_done event, making the implementation
easier and following common protocol practice.

Signed-off-by: Robert Mader <robert.mader@collabora.com>

clients/screenshot.c

@@ -66,6 +66,7 @@ struct screenshooter_buffer {
 
 struct screenshooter_output {
 	struct screenshooter_app *app;
+	uint32_t name;
 	struct wl_list link; /* struct screenshooter_app::output_list */
 
 	struct wl_output *wl_output;
@@ -75,7 +76,8 @@ struct screenshooter_output {
 
 	int buffer_width;
 	int buffer_height;
-	const struct pixel_format_info *fmt;
+	struct wl_array formats;
+	bool formats_done;
 	struct screenshooter_buffer *buffer;
 };
 
@@ -131,10 +133,28 @@ capture_source_handle_format(void *data,
 			     uint32_t drm_format)
 {
 	struct screenshooter_output *output = data;
+	uint32_t *fmt;
 
 	assert(output->source == proxy);
 
-	output->fmt = pixel_format_get_info(drm_format);
+	if (output->formats_done) {
+		wl_array_release(&output->formats);
+		wl_array_init(&output->formats);
+		output->formats_done = false;
+	}
+
+	fmt = wl_array_add(&output->formats, sizeof(uint32_t));
+	assert(fmt);
+	*fmt = drm_format;
+}
+
+static void
+capture_source_handle_formats_done(void *data,
+				   struct weston_capture_source_v1 *proxy)
+{
+	struct screenshooter_output *output = data;
+
+	output->formats_done = true;
 }
 
 static void
@@ -188,6 +208,7 @@ capture_source_handle_failed(void *data,
 
 static const struct weston_capture_source_v1_listener capture_source_handlers = {
 	.format = capture_source_handle_format,
+	.formats_done = capture_source_handle_formats_done,
 	.size = capture_source_handle_size,
 	.complete = capture_source_handle_complete,
 	.retry = capture_source_handle_retry,
@@ -202,6 +223,7 @@ create_output(struct screenshooter_app *app, uint32_t output_name, uint32_t vers
 	version = MIN(version, 4);
 	output = xzalloc(sizeof *output);
 	output->app = app;
+	output->name = output_name;
 	output->wl_output = wl_registry_bind(app->registry, output_name,
 					     &wl_output_interface, version);
 	abort_oom_if_null(output->wl_output);
@@ -213,6 +235,8 @@ create_output(struct screenshooter_app *app, uint32_t output_name, uint32_t vers
 	weston_capture_source_v1_add_listener(output->source,
 					      &capture_source_handlers, output);
 
+	wl_array_init(&output->formats);
+
 	wl_list_insert(&app->output_list, &output->link);
 }
 
@@ -221,6 +245,8 @@ destroy_output(struct screenshooter_output *output)
 {
 	weston_capture_source_v1_destroy(output->source);
 
+	wl_array_release(&output->formats);
+
 	if (wl_output_get_version(output->wl_output) >= WL_OUTPUT_RELEASE_SINCE_VERSION)
 		wl_output_release(output->wl_output);
 	else
@@ -248,7 +274,7 @@ handle_global(void *data, struct wl_registry *registry,
 	} else if (strcmp(interface, weston_capture_v1_interface.name) == 0) {
 		app->capture_factory = wl_registry_bind(registry, name,
 							&weston_capture_v1_interface,
-							1);
+							2);
 	}
 }
 
@@ -266,11 +292,25 @@ static const struct wl_registry_listener registry_listener = {
 static void
 screenshooter_output_capture(struct screenshooter_output *output)
 {
+	const struct pixel_format_info *fmt_info = NULL;
+	uint32_t *fmt;
+
 	screenshooter_buffer_destroy(output->buffer);
+
+	wl_array_for_each(fmt, &output->formats) {
+		fmt_info = pixel_format_get_info(*fmt);
+		assert(fmt_info);
+		break;
+	}
+	if (!fmt_info) {
+		fprintf(stderr, "No supported format found\n");
+		exit(1);
+	}
+
 	output->buffer = screenshot_create_shm_buffer(output->app,
 						      output->buffer_width,
 						      output->buffer_height,
-						      output->fmt);
+						      fmt_info);
 	abort_oom_if_null(output->buffer);
 
 	weston_capture_source_v1_capture(output->source,
@@ -353,6 +393,18 @@ screenshot_set_buffer_size(struct buffer_size *buff_size,
 	return 0;
 }
 
+static bool
+received_formats_for_all_outputs(struct screenshooter_app *app)
+{
+	struct screenshooter_output *output;
+
+	wl_list_for_each(output, &app->output_list, link) {
+		if (!output->formats_done)
+			return false;
+	}
+	return true;
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -389,6 +441,13 @@ main(int argc, char *argv[])
 	/* Process initial events for wl_output and weston_capture_source_v1 */
 	wl_display_roundtrip(display);
 
+	while (!received_formats_for_all_outputs(&app)) {
+		if (wl_display_dispatch(display) < 0) {
+			fprintf(stderr, "Error: connection terminated\n");
+			return -1;
+		}
+	}
+
 	do {
 		app.retry = false;
 

libweston/output-capture.c

@@ -214,6 +214,10 @@ capture_info_send_source_info(struct weston_output_capture_info *ci,
 
 		weston_capture_source_v1_send_format(csrc->resource,
 						     csi->drm_format);
+		if (wl_resource_get_version(csrc->resource) >=
+		    WESTON_CAPTURE_SOURCE_V1_FORMATS_DONE_SINCE_VERSION)
+			weston_capture_source_v1_send_formats_done(csrc->resource);
+
 		weston_capture_source_v1_send_size(csrc->resource,
 						   csi->width, csi->height);
 	}
@@ -671,7 +675,7 @@ weston_compositor_install_capture_protocol(struct weston_compositor *compositor)
 	compositor->output_capture.weston_capture_v1 =
 		wl_global_create(compositor->wl_display,
 				 &weston_capture_v1_interface,
-				 1, NULL, bind_weston_capture);
+				 2, NULL, bind_weston_capture);
 	abort_oom_if_null(compositor->output_capture.weston_capture_v1);
 }
 

protocol/weston-output-capture.xml

@@ -24,7 +24,7 @@
     DEALINGS IN THE SOFTWARE.
   </copyright>
 
-  <interface name="weston_capture_v1" version="1">
+  <interface name="weston_capture_v1" version="2">
     <description summary="image capture factory">
       The global interface exposing Weston screenshooting functionality
       intended for single shots.
@@ -91,7 +91,7 @@
     </request>
   </interface>
 
-  <interface name="weston_capture_source_v1" version="1">
+  <interface name="weston_capture_source_v1" version="2">
     <description summary="image capturing source">
       An object representing image capturing functionality for a single
       source. When created, it sends the initial events if and only if the
@@ -151,18 +151,26 @@
 
     <event name="format">
       <description summary="pixel format for a buffer">
-        This event delivers the pixel format that should be used for the
+        This event delivers one pixel format that can be used for the
         image buffer. Any buffer is incompatible if it does not have
-        this pixel format.
+        a pixel format delivered by one of this events.
 
         The format modifier is linear (DRM_FORMAT_MOD_LINEAR).
 
-        This is an initial event, and sent whenever the required format
+        This is an initial event, and sent whenever the supported formats
-        changes.
+        change.
+
+        This event may be send multiple times, followed by a format_done event.
       </description>
       <arg name="drm_format" type="uint" summary="DRM pixel format code"/>
     </event>
 
+    <event name="formats_done" since="2">
+      <description summary="sending formats is complete">
+        This event is sent after all formats have been sent.
+      </description>
+    </event>
+
     <event name="size">
       <description summary="dimensions for a buffer">
         This event delivers the size that should be used for the

tests/output-capture-protocol-test.c

@@ -89,6 +89,7 @@ struct capturer {
 	struct {
 		bool size;
 		bool format;
+		bool formats_done;
 		bool reply;
 	} events;
 
@@ -104,10 +105,27 @@ capture_source_handle_format(void *data,
 
 	test_assert_ptr_eq(capt->source, proxy);
 
+	if (capt->events.formats_done) {
+		capt->drm_format = DRM_FORMAT_INVALID;
+		capt->events.formats_done = false;
+	}
+
 	capt->events.format = true;
+	if (capt->drm_format == DRM_FORMAT_INVALID)
 		capt->drm_format = drm_format;
 }
 
+static void
+capture_source_handle_formats_done(void *data,
+				   struct weston_capture_source_v1 *proxy)
+{
+	struct capturer *capt = data;
+
+	test_assert_ptr_eq(capt->source, proxy);
+
+	capt->events.formats_done = true;
+}
+
 static void
 capture_source_handle_size(void *data,
 			   struct weston_capture_source_v1 *proxy,
@@ -164,6 +182,7 @@ capture_source_handle_failed(void *data,
 
 static const struct weston_capture_source_v1_listener capture_source_handlers = {
 	.format = capture_source_handle_format,
+	.formats_done = capture_source_handle_formats_done,
 	.size = capture_source_handle_size,
 	.complete = capture_source_handle_complete,
 	.retry = capture_source_handle_retry,
@@ -181,7 +200,7 @@ capturer_create(struct client *client,
 
 	capt->factory = bind_to_singleton_global(client,
 						 &weston_capture_v1_interface,
-						 1);
+						 2);
 
 	capt->source = weston_capture_v1_create(capt->factory,
 						output->wl_output, src);
@@ -217,6 +236,7 @@ TEST(simple_shot)
 	client_roundtrip(client);
 
 	test_assert_true(capt->events.format);
+	test_assert_true(capt->events.formats_done);
 	test_assert_true(capt->events.size);
 	test_assert_enum(capt->state, CAPTURE_TASK_PENDING);
 	test_assert_u32_eq(capt->drm_format, fix->expected_drm_format);
@@ -257,6 +277,7 @@ TEST(retry_on_wrong_format)
 	client_roundtrip(client);
 
 	test_assert_true(capt->events.format);
+	test_assert_true(capt->events.formats_done);
 	test_assert_true(capt->events.size);
 	test_assert_enum(capt->state, CAPTURE_TASK_PENDING);
 
@@ -298,6 +319,7 @@ TEST(retry_on_wrong_size)
 	client_roundtrip(client);
 
 	test_assert_true(capt->events.format);
+	test_assert_true(capt->events.formats_done);
 	test_assert_true(capt->events.size);
 	test_assert_enum(capt->state, CAPTURE_TASK_PENDING);
 	test_assert_int_gt(capt->width, 5);
@@ -337,6 +359,7 @@ TEST(writeback_on_headless_fails)
 	client_roundtrip(client);
 
 	test_assert_false(capt->events.format);
+	test_assert_false(capt->events.formats_done);
 	test_assert_false(capt->events.size);
 	test_assert_enum(capt->state, CAPTURE_TASK_PENDING);
 
@@ -345,6 +368,7 @@ TEST(writeback_on_headless_fails)
 	client_roundtrip(client);
 
 	test_assert_false(capt->events.format);
+	test_assert_false(capt->events.formats_done);
 	test_assert_false(capt->events.size);
 	test_assert_enum(capt->state, CAPTURE_TASK_FAILED);
 	test_assert_str_eq(capt->last_failure, "source unavailable");

tests/weston-test-client-helper.c

@@ -1818,6 +1818,7 @@ struct output_capturer {
 	int width;
 	int height;
 	uint32_t drm_format;
+	bool formats_done;
 
 	struct weston_capture_v1 *factory;
 	struct weston_capture_source_v1 *source;
@@ -1832,9 +1833,24 @@ output_capturer_handle_format(void *data,
 {
 	struct output_capturer *capt = data;
 
+	if (capt->formats_done) {
+		capt->drm_format = DRM_FORMAT_INVALID;
+		capt->formats_done = false;
+	}
+
+	if (!capt->drm_format)
 		capt->drm_format = drm_format;
 }
 
+static void
+output_capturer_handle_formats_done(void *data,
+				    struct weston_capture_source_v1 *proxy)
+{
+	struct output_capturer *capt = data;
+
+	capt->formats_done = true;
+}
+
 static void
 output_capturer_handle_size(void *data,
 			    struct weston_capture_source_v1 *proxy,
@@ -1874,6 +1890,7 @@ output_capturer_handle_failed(void *data,
 
 static const struct weston_capture_source_v1_listener output_capturer_source_handlers = {
 	.format = output_capturer_handle_format,
+	.formats_done = output_capturer_handle_formats_done,
 	.size = output_capturer_handle_size,
 	.complete = output_capturer_handle_complete,
 	.retry = output_capturer_handle_retry,
@@ -1890,7 +1907,7 @@ client_capture_output(struct client *client,
 
 	capt.factory = bind_to_singleton_global(client,
 						&weston_capture_v1_interface,
-						1);
+						2);
 
 	capt.source = weston_capture_v1_create(capt.factory,
 					       output->wl_output, src);
@@ -1903,6 +1920,7 @@ client_capture_output(struct client *client,
 	test_assert_true(capt.width != 0 &&
 			 capt.height != 0 &&
 			 capt.drm_format != 0 &&
+			 capt.formats_done &&
 			 "capture source not available");
 
 	buf = create_shm_buffer(client,