Merge lp:~albaguirre/mir/add-mirscreencast-spec into lp:mir

[Alberto Aguirre]

Introduce MirScreencastSpec

MirScreencastParameters is not opaque and modifying it to to add more screencast creation options would break client ABI.
Instead use a MirScreencastSpec similar to MirSurfaceSpec to specify existing and future options.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3424
https://mir-jenkins.ubuntu.com/job/mir-ci/659/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/603
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/640
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/632
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/632
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/613
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/613/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/613
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/613/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/613
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/613/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/613
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/613/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/613
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/613/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/659/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

+MirScreencast* create_screencast(MirScreencastSpec* spec)
more of a future improvement (as this seems built on what's in place), but we can get rid of the pointer release if the resource is managed via the connection with a function like:
connection->create_screencast(...)
(in the same sort of manner as streams/chains/surfaces, and this makes for better cleanup during certain destruction scenarios)

nits:
+ if (valid() && server)
do we have to check server? (initialized from a ref, if its invalid at call site, we'd still have a non-null (and deleted) ptr and still have a problem)

987/991 extra lines

review: Approve

[Alberto Aguirre (albaguirre)]

>more of a future improvement (as this seems built on what's in place), but we can get rid of the >pointer release if the resource is managed via the connection with a function like:
>connection->create_screencast(...)

Yeah I plan to move the construction/destruction to connection like the others - but I wanted to keep this MP/diff small and mainly about the introduction of the MirScreencast spec.

>do we have to check server?

server is a pointer can be null in case an invalid MirScreencast object is created (like in like 535).

>987/991 extra lines

Fixed.

[Mir CI Bot (mir-ci-bot)]

PASSED: Continuous integration, rev:3425
https://mir-jenkins.ubuntu.com/job/mir-ci/672/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/620
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/657
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/649
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/649
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/630
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/630/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/630
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/630/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/630
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/630/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/630
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/630/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/630
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/630/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/672/rebuild

review: Approve (continuous-integration)

=== modified file 'include/client/mir_toolkit/client_types.h'
--- include/client/mir_toolkit/client_types.h	2016-03-23 06:39:56 +0000
+++ include/client/mir_toolkit/client_types.h	2016-03-28 14:29:53 +0000
@@ -41,6 +41,7 @@
 typedef struct MirSurface MirSurface;
 typedef struct MirSurfaceSpec MirSurfaceSpec;
 typedef struct MirScreencast MirScreencast;
+typedef struct MirScreencastSpec MirScreencastSpec;
 typedef struct MirPromptSession MirPromptSession;
 typedef struct MirBufferStream MirBufferStream;
 typedef struct MirPersistentId MirPersistentId;

=== modified file 'include/client/mir_toolkit/mir_screencast.h'
--- include/client/mir_toolkit/mir_screencast.h	2015-06-17 05:20:42 +0000
+++ include/client/mir_toolkit/mir_screencast.h	2016-03-28 14:29:53 +0000
@@ -28,6 +28,91 @@
 #endif
 
 /**
+ * Create a screencast specification.
+ *
+ * \remark For use with mir_screencast_create() at the width, height,
+ * pixel format and capture region must be set.
+ *
+ * \param [in] connection   a valid mir connection
+ * \return                  A handle that can ultimately be passed to
+ *                          mir_surface_create() or mir_surface_apply_spec()
+ */
+MirScreencastSpec* mir_create_screencast_spec(MirConnection* connection);
+
+/**
+ * Set the requested width, in pixels
+ *
+ * \param [in] spec     Specification to mutate
+ * \param [in] width    Requested width.
+ *
+ */
+void mir_screencast_spec_set_width(MirScreencastSpec* spec, unsigned width);
+
+/**
+ * Set the requested height, in pixels
+ *
+ * \param [in] spec     Specification to mutate
+ * \param [in] height   Requested height.
+ *
+ */
+void mir_screencast_spec_set_height(MirScreencastSpec* spec, unsigned height);
+
+/**
+ * Set the requested pixel format.
+ *
+ * \param [in] spec     Specification to mutate
+ * \param [in] format   Requested pixel format
+ *
+ */
+void mir_screencast_spec_set_pixel_format(MirScreencastSpec* spec, MirPixelFormat format);
+
+
+/**
+ * Set the rectangular region to capture.
+ *
+ * \param [in] spec     Specification to mutate
+ * \param [in] region   The rectangular region of the screen to capture
+ *                      specified in virtual screen space coordinates
+ *
+ */
+void mir_screencast_spec_set_capture_region(MirScreencastSpec* spec, MirRectangle const* region);
+
+/**
+ * Release the resources held by a MirScreencastSpec.
+ *
+ * \param [in] spec     Specification to release
+ */
+void mir_screencast_spec_release(MirScreencastSpec* spec);
+
+/**
+ * Create a screencast from a given specification
+ *
+ * \param [in] spec  Specification of the screencast attributes
+ * \return           The resulting screencast
+ */
+MirScreencast* mir_screencast_create_sync(MirScreencastSpec* spec);
+
+/**
+ * Test for a valid screencast
+ *
+ * \param [in] screencast  The screencast to verify
+ * \return                 True if the supplied screencast is valid, false otherwise.
+ */
+bool mir_screencast_is_valid(MirScreencast *screencast);
+
+/**
+ * Retrieve a text description of the error. The returned string is owned by
+ * the library and remains valid until the screencast or the associated
+ * connection has been released.
+ *
+ * \param [in] screencast  The screencast
+ * \return                 A text description of any error resulting in an
+ *                         invalid screencast, or the empty string "" if the
+ *                         screencast is valid.
+ */
+char const *mir_screencast_get_error_message(MirScreencast *screencast);
+
+/**
  * Create a screencast on the supplied connection.
  *
  * A screencast allows clients to read the contents of the screen.
@@ -37,16 +122,16 @@
  *   \param [in] parameters  The screencast parameters
  *   \return                 The resulting screencast
  */
-MirScreencast *mir_connection_create_screencast_sync(
-    MirConnection *connection,
-    MirScreencastParameters *parameters);
+MirScreencast* mir_connection_create_screencast_sync(
+    MirConnection* connection,
+    MirScreencastParameters* parameters);
 
 /**
  * Release the specified screencast.
  *   \param [in] screencast  The screencast to be released
  */
 void mir_screencast_release_sync(
-    MirScreencast *screencast);
+    MirScreencast* screencast);
 
 /**
  * Retrieve the MirBufferStream associated with a screencast 
@@ -54,7 +139,7 @@
  * 
  *   \param[in] screencast The screencast
  */
-MirBufferStream* mir_screencast_get_buffer_stream(MirScreencast *screencast);
+MirBufferStream* mir_screencast_get_buffer_stream(MirScreencast* screencast);
 
 #ifdef __cplusplus
 }

=== modified file 'include/common/mir/optional_value.h'
--- include/common/mir/optional_value.h	2016-01-29 08:18:22 +0000
+++ include/common/mir/optional_value.h	2016-03-28 14:29:53 +0000
@@ -29,7 +29,7 @@
 {
 public:
     optional_value() = default;
-    optional_value(T const& value) : value_{value}, is_set_{true} {}
+    optional_value(T const& value) : value_(value), is_set_{true} {}
 
     optional_value& operator=(T const& value)
     {

=== modified file 'src/client/mir_screencast.cpp'
--- src/client/mir_screencast.cpp	2016-03-23 06:39:56 +0000
+++ src/client/mir_screencast.cpp	2016-03-28 14:29:53 +0000
@@ -30,42 +30,99 @@
 namespace mp = mir::protobuf;
 namespace geom = mir::geometry;
 
+namespace
+{
+mir::protobuf::ScreencastParameters serialize_spec(MirScreencastSpec const& spec)
+{
+    mp::ScreencastParameters message;
+
+#define SERIALIZE_OPTION_IF_SET(option) \
+    if (spec.option.is_set()) \
+        message.set_##option(spec.option.value());
+
+    SERIALIZE_OPTION_IF_SET(width);
+    SERIALIZE_OPTION_IF_SET(height);
+    SERIALIZE_OPTION_IF_SET(pixel_format);
+
+    if (spec.capture_region.is_set())
+    {
+        auto const region = spec.capture_region.value();
+        message.mutable_region()->set_left(region.left);
+        message.mutable_region()->set_top(region.top);
+        message.mutable_region()->set_width(region.width);
+        message.mutable_region()->set_height(region.height);
+    }
+
+    return message;
+}
+
+//TODO: it should be up to the server to decide if the parameters are acceptable
+void throw_if_invalid(MirScreencastSpec const& spec)
+{
+#define THROW_IF_UNSET(option) \
+    if (!spec.option.is_set()) \
+        BOOST_THROW_EXCEPTION(std::runtime_error("Unset "#option));
+
+#define THROW_IF_EQ(option, val) \
+    THROW_IF_UNSET(option); \
+    if (spec.option.is_set() && spec.option.value() == val) \
+        BOOST_THROW_EXCEPTION(std::runtime_error("Invalid "#option));
+
+#define THROW_IF_ZERO(option) THROW_IF_EQ(option, 0)
+
+    THROW_IF_ZERO(width);
+    THROW_IF_ZERO(height);
+    THROW_IF_EQ(pixel_format, mir_pixel_format_invalid);
+    THROW_IF_UNSET(capture_region);
+
+    if (spec.capture_region.is_set())
+    {
+        auto const region = spec.capture_region.value();
+        if (region.width == 0)
+            BOOST_THROW_EXCEPTION(std::runtime_error("Invalid capture region width"));
+        if (region.height == 0)
+            BOOST_THROW_EXCEPTION(std::runtime_error("Invalid capture region height"));
+    }
+}
+}
+
+MirScreencastSpec::MirScreencastSpec() = default;
+
+MirScreencastSpec::MirScreencastSpec(MirConnection* connection)
+    : connection{connection}
+{
+}
+
+MirScreencastSpec::MirScreencastSpec(MirConnection* connection, MirScreencastParameters const& params)
+    : connection{connection},
+      width{params.width},
+      height{params.height},
+      pixel_format{params.pixel_format},
+      capture_region{params.region}
+{
+}
+
+MirScreencast::MirScreencast(std::string const& error)
+    : protobuf_screencast{mcl::make_protobuf_object<mir::protobuf::Screencast>()}
+{
+    protobuf_screencast->set_error(error);
+}
+
 MirScreencast::MirScreencast(
-    geom::Rectangle const& region,
-    geom::Size const& size,
-    MirPixelFormat pixel_format,
-    mir::client::rpc::DisplayServer& server,
-    MirConnection* connection,
+    MirScreencastSpec const& spec,
+    mir::client::rpc::DisplayServer& the_server,
     mir_screencast_callback callback, void* context)
-    : server(server),
-      connection{connection},
-      output_size{size},
+    : server{&the_server},
+      connection{spec.connection},
       protobuf_screencast{mcl::make_protobuf_object<mir::protobuf::Screencast>()},
       protobuf_void{mcl::make_protobuf_object<mir::protobuf::Void>()}
 {
-    if (output_size.width.as_int()  == 0 ||
-        output_size.height.as_int() == 0 ||
-        region.size.width.as_int()  == 0 ||
-        region.size.height.as_int() == 0 ||
-        pixel_format == mir_pixel_format_invalid)
-    {
-        BOOST_THROW_EXCEPTION(std::runtime_error("Invalid parameters"));
-    }
-    protobuf_screencast->set_error("Not initialized");
-
-    mp::ScreencastParameters parameters;
-
-    parameters.mutable_region()->set_left(region.top_left.x.as_int());
-    parameters.mutable_region()->set_top(region.top_left.y.as_int());
-    parameters.mutable_region()->set_width(region.size.width.as_uint32_t());
-    parameters.mutable_region()->set_height(region.size.height.as_uint32_t());
-    parameters.set_width(output_size.width.as_uint32_t());
-    parameters.set_height(output_size.height.as_uint32_t());
-    parameters.set_pixel_format(pixel_format);
+    throw_if_invalid(spec);
+    auto const message = serialize_spec(spec);
 
     create_screencast_wait_handle.expect_result();
-    server.create_screencast(
-        &parameters,
+    server->create_screencast(
+        &message,
         protobuf_screencast.get(),
         google::protobuf::NewCallback(
             this, &MirScreencast::screencast_created,
@@ -79,36 +136,53 @@
 
 bool MirScreencast::valid()
 {
+    std::lock_guard<decltype(mutex)> lock(mutex);
     return !protobuf_screencast->has_error();
 }
 
-MirWaitHandle* MirScreencast::release(
-        mir_screencast_callback callback, void* context)
-{
-    mp::ScreencastId screencast_id;
-    screencast_id.set_value(protobuf_screencast->screencast_id().value());
-    
+char const* MirScreencast::get_error_message()
+{
+    std::lock_guard<decltype(mutex)> lock(mutex);
+    if (protobuf_screencast->has_error())
+    {
+        return protobuf_screencast->error().c_str();
+    }
+    return empty_error_message.c_str();
+}
+
+MirWaitHandle* MirScreencast::release(mir_screencast_callback callback, void* context)
+{
     release_wait_handle.expect_result();
-    server.release_screencast(
-        &screencast_id,
-        protobuf_void.get(),
-        google::protobuf::NewCallback(
-            this, &MirScreencast::released, callback, context));
+    if (valid() && server)
+    {
+        mp::ScreencastId screencast_id;
+        {
+            std::lock_guard<decltype(mutex)> lock(mutex);
+            screencast_id.set_value(protobuf_screencast->screencast_id().value());
+        }
+        server->release_screencast(
+            &screencast_id,
+            protobuf_void.get(),
+            google::protobuf::NewCallback(
+                this, &MirScreencast::released, callback, context));
+    }
+    else
+    {
+        callback(this, context);
+        release_wait_handle.result_received();
+    }
 
     return &release_wait_handle;
 }
 
-void MirScreencast::request_and_wait_for_configure(MirSurfaceAttrib, int)
-{
-}
-
 void MirScreencast::screencast_created(
     mir_screencast_callback callback, void* context)
 {
     if (!protobuf_screencast->has_error() && connection)
     {
+        std::lock_guard<decltype(mutex)> lock(mutex);
         buffer_stream = connection->make_consumer_stream(
-            protobuf_screencast->buffer_stream(), output_size);
+            protobuf_screencast->buffer_stream(), {});
     }
 
     callback(this, context);
@@ -119,14 +193,18 @@
     mir_screencast_callback callback, void* context)
 {
     callback(this, context);
-    if (connection)
-        connection->release_consumer_stream(buffer_stream.get());
-    buffer_stream.reset();
 
+    {
+        std::lock_guard<decltype(mutex)> lock(mutex);
+        if (connection)
+            connection->release_consumer_stream(buffer_stream.get());
+        buffer_stream.reset();
+    }
     release_wait_handle.result_received();
 }
 
 mir::client::ClientBufferStream* MirScreencast::get_buffer_stream()
 {
+    std::lock_guard<decltype(mutex)> lock(mutex);
     return buffer_stream.get();
 }

=== modified file 'src/client/mir_screencast.h'
--- src/client/mir_screencast.h	2016-01-29 08:18:22 +0000
+++ src/client/mir_screencast.h	2016-03-28 14:29:53 +0000
@@ -21,6 +21,7 @@
 
 #include "mir_wait_handle.h"
 #include "mir_toolkit/client_types.h"
+#include "mir/optional_value.h"
 #include "mir/geometry/size.h"
 #include "mir/geometry/rectangle.h"
 
@@ -46,27 +47,40 @@
 }
 }
 
+struct MirScreencastSpec
+{
+    MirScreencastSpec();
+    MirScreencastSpec(MirConnection* connection);
+    MirScreencastSpec(MirConnection* connection, MirScreencastParameters const& params);
+
+    // Required parameters
+    MirConnection* connection{nullptr};
+
+    // Optional parameters
+    mir::optional_value<unsigned int> width;
+    mir::optional_value<unsigned int> height;
+    mir::optional_value<MirPixelFormat> pixel_format;
+    mir::optional_value<MirRectangle> capture_region;
+};
+
 struct MirScreencast
 {
 public:
+    MirScreencast(std::string const& error);
     MirScreencast(
-        mir::geometry::Rectangle const& region,
-        mir::geometry::Size const& size,
-        MirPixelFormat pixel_format,
+        MirScreencastSpec const& spec,
         mir::client::rpc::DisplayServer& server,
-        MirConnection* connection,
         mir_screencast_callback callback, void* context);
 
     MirWaitHandle* creation_wait_handle();
     bool valid();
+    char const* get_error_message();
 
     MirWaitHandle* release(
         mir_screencast_callback callback, void* context);
 
     EGLNativeWindowType egl_native_window();
 
-    void request_and_wait_for_configure(MirSurfaceAttrib a, int value);
-
     mir::client::ClientBufferStream* get_buffer_stream();
 
 private:
@@ -75,16 +89,18 @@
     void released(
         mir_screencast_callback callback, void* context);
 
-    mir::client::rpc::DisplayServer& server;
-    MirConnection* connection;
-    mir::geometry::Size const output_size;
+    std::mutex mutable mutex;
+    mir::client::rpc::DisplayServer* const server{nullptr};
+    MirConnection* const connection{nullptr};
     std::shared_ptr<mir::client::ClientBufferStream> buffer_stream;
 
-    std::unique_ptr<mir::protobuf::Screencast> protobuf_screencast;
-    std::unique_ptr<mir::protobuf::Void> protobuf_void;
+    std::unique_ptr<mir::protobuf::Screencast> const protobuf_screencast;
+    std::unique_ptr<mir::protobuf::Void> const protobuf_void;
 
     MirWaitHandle create_screencast_wait_handle;
     MirWaitHandle release_wait_handle;
+
+    std::string const empty_error_message;
 };
 
 #endif /* MIR_CLIENT_MIR_SCREENCAST_H_ */

=== modified file 'src/client/mir_screencast_api.cpp'
--- src/client/mir_screencast_api.cpp	2015-06-18 02:46:16 +0000
+++ src/client/mir_screencast_api.cpp	2016-03-28 14:29:53 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2014 Canonical Ltd.
+ * Copyright © 2014,2016 Canonical Ltd.
  *
  * This program is free software: you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License version 3,
@@ -22,7 +22,7 @@
 #include "mir_screencast.h"
 #include "mir_connection.h"
 #include "mir/raii.h"
-
+#include "mir/require.h"
 #include "mir/uncaught.h"
 
 #include <stdexcept>
@@ -31,53 +31,78 @@
 namespace
 {
 void null_callback(MirScreencast*, void*) {}
-}
+MirScreencast* create_screencast(MirScreencastSpec* spec)
+{
+    auto& server = spec->connection->display_server();
+    auto screencast = std::make_unique<MirScreencast>(*spec, server, null_callback, nullptr);
+    screencast->creation_wait_handle()->wait_for_all();
+
+    auto raw_screencast = screencast.get();
+    screencast.release();
+    return raw_screencast;
+}
+}
+
+MirScreencastSpec* mir_create_screencast_spec(MirConnection* connection)
+{
+    mir::require(mir_connection_is_valid(connection));
+    return new MirScreencastSpec{connection};
+}
+
+void mir_screencast_spec_set_width(MirScreencastSpec* spec, unsigned width)
+{
+    spec->width = width;
+}
+
+void mir_screencast_spec_set_height(MirScreencastSpec* spec, unsigned height)
+{
+    spec->height = height;
+}
+
+void mir_screencast_spec_set_pixel_format(MirScreencastSpec* spec, MirPixelFormat format)
+{
+    spec->pixel_format = format;
+}
+
+void mir_screencast_spec_set_capture_region(MirScreencastSpec* spec, MirRectangle const* region)
+{
+    spec->capture_region = *region;
+}
+
+void mir_screencast_spec_release(MirScreencastSpec* spec)
+{
+    delete spec;
+}
+
+MirScreencast* mir_screencast_create_sync(MirScreencastSpec* spec)
+try
+{
+    return create_screencast(spec);
+}
+catch (std::exception const& ex)
+{
+    MIR_LOG_UNCAUGHT_EXCEPTION(ex);
+    return new MirScreencast(ex.what());
+}
+
+bool mir_screencast_is_valid(MirScreencast *screencast)
+{
+    return screencast && screencast->valid();
+}
+
+char const *mir_screencast_get_error_message(MirScreencast *screencast)
+{
+    return screencast->get_error_message();
+}
+
 
 MirScreencast* mir_connection_create_screencast_sync(
     MirConnection* connection,
     MirScreencastParameters* parameters)
 {
-    if (!MirConnection::is_valid(connection))
-        return nullptr;
-
-    MirScreencast* screencast = nullptr;
-
-    try
-    {
-        auto const config = mir::raii::deleter_for(
-            mir_connection_create_display_config(connection),
-            &mir_display_config_destroy);
-
-        mir::geometry::Rectangle const region{
-            {parameters->region.left, parameters->region.top},
-            {parameters->region.width, parameters->region.height}
-        };
-        mir::geometry::Size const size{parameters->width, parameters->height};
-
-        std::unique_ptr<MirScreencast> screencast_uptr{
-            new MirScreencast{
-                region,
-                size,
-                parameters->pixel_format,
-                connection->display_server(),
-                connection,
-                null_callback, nullptr}};
-
-        screencast_uptr->creation_wait_handle()->wait_for_all();
-
-        if (screencast_uptr->valid())
-        {
-            screencast = screencast_uptr.get();
-            screencast_uptr.release();
-        }
-    }
-    catch (std::exception const& ex)
-    {
-        MIR_LOG_UNCAUGHT_EXCEPTION(ex);
-        return nullptr;
-    }
-
-    return screencast;
+    mir::require(mir_connection_is_valid(connection));
+    MirScreencastSpec spec{connection, *parameters};
+    return mir_screencast_create_sync(&spec);
 }
 
 void mir_screencast_release_sync(MirScreencast* screencast)

=== modified file 'src/client/symbols.map'
--- src/client/symbols.map	2016-03-26 00:19:29 +0000
+++ src/client/symbols.map	2016-03-28 14:29:53 +0000
@@ -272,6 +272,7 @@
   global:
     mir_connection_confirm_base_display_configuration;
     mir_connection_preview_base_display_configuration;
+    mir_create_screencast_spec;
     mir_display_config_get_mutable_output;
     mir_output_disable;
     mir_output_enable;
@@ -280,6 +281,14 @@
     mir_output_set_pixel_format;
     mir_output_set_position;
     mir_output_set_power_mode;
+    mir_screencast_create_sync;
+    mir_screencast_get_error_message;
+    mir_screencast_is_valid;
+    mir_screencast_spec_release;
+    mir_screencast_spec_set_capture_region;
+    mir_screencast_spec_set_height;
+    mir_screencast_spec_set_pixel_format;
+    mir_screencast_spec_set_width;
 } MIR_CLIENT_0.21;
 
 MIR_CLIENT_DETAIL_9 {

=== modified file 'tests/acceptance-tests/test_client_screencast.cpp'
--- tests/acceptance-tests/test_client_screencast.cpp	2015-06-25 03:00:08 +0000
+++ tests/acceptance-tests/test_client_screencast.cpp	2016-03-28 14:29:53 +0000
@@ -59,14 +59,6 @@
 
 // TODO test case(s) showing screencast works. lp:1396681
 
-TEST_F(Screencast, with_invalid_connection_fails)
-{
-    using namespace testing;
-
-    auto screencast = mir_connection_create_screencast_sync(nullptr, &default_screencast_params);
-    ASSERT_EQ(nullptr, screencast);
-}
-
 TEST_F(Screencast, with_invalid_params_fails)
 {
     using namespace testing;
@@ -79,19 +71,24 @@
     MirScreencastParameters params = default_screencast_params;
     params.width = params.height = 0;
     auto screencast = mir_connection_create_screencast_sync(connection, &params);
-    ASSERT_EQ(nullptr, screencast);
+    EXPECT_FALSE(mir_screencast_is_valid(screencast));
+
+    mir_screencast_release_sync(screencast);
 
     params = default_screencast_params;
     params.region.width = params.region.height = 0;
     screencast = mir_connection_create_screencast_sync(connection, &params);
-    ASSERT_EQ(nullptr, screencast);
+    EXPECT_FALSE(mir_screencast_is_valid(screencast));
+
+    mir_screencast_release_sync(screencast);
 
     params = default_screencast_params;
     params.pixel_format = mir_pixel_format_invalid;
 
     screencast = mir_connection_create_screencast_sync(connection, &params);
-    ASSERT_EQ(nullptr, screencast);
+    EXPECT_FALSE(mir_screencast_is_valid(screencast));
 
+    mir_screencast_release_sync(screencast);
     mir_connection_release(connection);
 }
 
@@ -105,7 +102,8 @@
     auto const connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
 
     auto screencast = mir_connection_create_screencast_sync(connection, &default_screencast_params);
-    ASSERT_EQ(nullptr, screencast);
+    EXPECT_FALSE(mir_screencast_is_valid(screencast));
 
+    mir_screencast_release_sync(screencast);
     mir_connection_release(connection);
 }

=== modified file 'tests/acceptance-tests/throwback/test_client_library_errors.cpp'
--- tests/acceptance-tests/throwback/test_client_library_errors.cpp	2016-03-23 06:39:56 +0000
+++ tests/acceptance-tests/throwback/test_client_library_errors.cpp	2016-03-28 14:29:53 +0000
@@ -17,6 +17,7 @@
  */
 
 #include "mir_toolkit/mir_client_library.h"
+#include "mir_toolkit/mir_screencast.h"
 #include "mir_toolkit/debug/surface.h"
 
 #include "src/include/client/mir/client_platform_factory.h"
@@ -377,3 +378,9 @@
     mir_persistent_id_release(id);
     mir_connection_release(connection);
 }
+
+TEST_F(ClientLibraryErrorsDeathTest, creating_screencast_with_invalid_connection)
+{
+    MirScreencastParameters params{{0, 0, 1, 1}, 1, 1, mir_pixel_format_abgr_8888};
+    EXPECT_DEATH(mir_connection_create_screencast_sync(nullptr, &params), "");
+}

=== modified file 'tests/integration-tests/test_client_screencast.cpp'
--- tests/integration-tests/test_client_screencast.cpp	2016-01-29 08:18:22 +0000
+++ tests/integration-tests/test_client_screencast.cpp	2016-03-28 14:29:53 +0000
@@ -41,6 +41,14 @@
 namespace
 {
 
+MirRectangle as_mir_rect(mir::geometry::Rectangle const& rect)
+{
+    return {rect.top_left.x.as_int(),
+            rect.top_left.y.as_int(),
+            rect.size.width.as_uint32_t(),
+            rect.size.height.as_uint32_t()};
+}
+
 class StubChanger : public mtd::NullDisplayChanger
 {
 public:
@@ -109,6 +117,8 @@
 
     auto screencast = mir_connection_create_screencast_sync(connection, &default_screencast_params);
     ASSERT_NE(nullptr, screencast);
+    ASSERT_TRUE(mir_screencast_is_valid(screencast));
+
     mir_screencast_release_sync(screencast);
 }
 
@@ -137,6 +147,7 @@
 
     auto screencast = mir_connection_create_screencast_sync(connection, &default_screencast_params);
     ASSERT_NE(nullptr, screencast);
+    ASSERT_TRUE(mir_screencast_is_valid(screencast));
 
     mir_screencast_release_sync(screencast);
 }
@@ -156,6 +167,7 @@
 
     auto screencast = mir_connection_create_screencast_sync(connection, &default_screencast_params);
     ASSERT_NE(nullptr, screencast);
+    ASSERT_TRUE(mir_screencast_is_valid(screencast));
 
     auto egl_native_window =
         mir_buffer_stream_get_egl_native_window(mir_screencast_get_buffer_stream(screencast));
@@ -167,10 +179,54 @@
 TEST_F(Screencast, fails_on_client_when_server_request_fails)
 {
     using namespace testing;
-
+    std::string const an_error_message{"boring error message"};
     EXPECT_CALL(mock_screencast(), create_session(_, _, _))
-        .WillOnce(Throw(std::runtime_error("")));
+        .WillOnce(Throw(std::runtime_error(an_error_message)));
 
     auto screencast = mir_connection_create_screencast_sync(connection, &default_screencast_params);
-    ASSERT_EQ(nullptr, screencast);
+    ASSERT_NE(nullptr, screencast);
+    ASSERT_FALSE(mir_screencast_is_valid(screencast));
+
+    EXPECT_THAT(mir_screencast_get_error_message(screencast), HasSubstr(an_error_message));
+    mir_screencast_release_sync(screencast);
+}
+
+TEST_F(Screencast, uses_provided_spec_parameters)
+{
+    using namespace testing;
+
+    mf::ScreencastSessionId const screencast_session_id{99};
+
+    geom::Size const size{default_screencast_params.width, default_screencast_params.height};
+    geom::Rectangle const region {
+        {default_screencast_params.region.left, default_screencast_params.region.top},
+        {default_screencast_params.region.width, default_screencast_params.region.height}};
+    MirPixelFormat const pixel_format {default_screencast_params.pixel_format};
+
+    InSequence seq;
+
+    EXPECT_CALL(mock_screencast(),
+                create_session(region, size, pixel_format))
+        .WillOnce(Return(screencast_session_id));
+
+    EXPECT_CALL(mock_screencast(), capture(_))
+        .WillOnce(Return(std::make_shared<mtd::StubBuffer>()));
+
+    EXPECT_CALL(mock_screencast(), destroy_session(_));
+
+    auto spec = mir_create_screencast_spec(connection);
+
+    mir_screencast_spec_set_width(spec, size.width.as_int());
+    mir_screencast_spec_set_height(spec, size.height.as_int());
+    mir_screencast_spec_set_pixel_format(spec, pixel_format);
+    MirRectangle const capture_region = as_mir_rect(region);
+    mir_screencast_spec_set_capture_region(spec, &capture_region);
+
+    auto screencast = mir_screencast_create_sync(spec);
+    ASSERT_NE(nullptr, screencast);
+    ASSERT_TRUE(mir_screencast_is_valid(screencast));
+    EXPECT_STREQ(mir_screencast_get_error_message(screencast), "");
+
+    mir_screencast_spec_release(spec);
+    mir_screencast_release_sync(screencast);
 }

=== modified file 'tests/unit-tests/client/test_mir_screencast.cpp'
--- tests/unit-tests/client/test_mir_screencast.cpp	2016-01-29 08:18:22 +0000
+++ tests/unit-tests/client/test_mir_screencast.cpp	2016-03-28 14:29:53 +0000
@@ -150,6 +150,13 @@
 {
 }
 
+MirRectangle as_mir_rect(mir::geometry::Rectangle const& rect)
+{
+    return {rect.top_left.x.as_int(),
+            rect.top_left.y.as_int(),
+            rect.size.width.as_uint32_t(),
+            rect.size.height.as_uint32_t()};
+}
 
 
 class MirScreencastTest : public testing::Test
@@ -160,6 +167,10 @@
           default_region{{0, 0}, {1, 1}},
           default_pixel_format{mir_pixel_format_xbgr_8888}
     {
+        default_spec.width = default_size.width.as_uint32_t();
+        default_spec.height = default_size.height.as_uint32_t();
+        default_spec.pixel_format = default_pixel_format;
+        default_spec.capture_region = as_mir_rect(default_region);
     }
 
     testing::NiceMock<MockProtobufServer> mock_server;
@@ -167,6 +178,7 @@
     mir::geometry::Size default_size;
     mir::geometry::Rectangle default_region;
     MirPixelFormat default_pixel_format;
+    MirScreencastSpec default_spec;
     mtd::MockClientBufferStream mock_bs;
 };
 
@@ -180,12 +192,7 @@
                 create_screencast(WithParams(default_region, default_size, default_pixel_format),_,_))
         .WillOnce(RunClosure());
 
-    MirScreencast screencast{
-        default_region,
-        default_size,
-        default_pixel_format, mock_server,
-        nullptr,
-        null_callback_func, nullptr};
+    MirScreencast screencast{default_spec, mock_server, null_callback_func, nullptr};
 }
 
 TEST_F(MirScreencastTest, releases_screencast_on_release)
@@ -204,13 +211,7 @@
                 release_screencast(WithScreencastId(screencast_id),_,_))
         .WillOnce(RunClosure());
 
-    MirScreencast screencast{
-        default_region,
-        default_size,
-        default_pixel_format,
-        mock_server,
-        nullptr,
-        null_callback_func, nullptr};
+    MirScreencast screencast{default_spec, mock_server, null_callback_func, nullptr};
 
     screencast.release(null_callback_func, nullptr);
 }
@@ -222,12 +223,7 @@
     MockCallback mock_cb;
     EXPECT_CALL(mock_cb, call(_, &mock_cb));
 
-    MirScreencast screencast{
-        default_region,
-        default_size,
-        default_pixel_format, stub_server,
-        nullptr,
-        mock_callback_func, &mock_cb};
+    MirScreencast screencast{default_spec, stub_server, mock_callback_func, &mock_cb};
 
     screencast.creation_wait_handle()->wait_for_all();
 }
@@ -236,12 +232,7 @@
 {
     using namespace testing;
 
-    MirScreencast screencast{
-        default_region,
-        default_size,
-        default_pixel_format, stub_server,
-        nullptr,
-        null_callback_func, nullptr};
+    MirScreencast screencast{default_spec, stub_server, null_callback_func, nullptr};
 
     screencast.creation_wait_handle()->wait_for_all();
 
@@ -258,30 +249,22 @@
     mir::geometry::Rectangle const invalid_region{{0, 0}, {0, 0}};
 
     EXPECT_THROW({
-        MirScreencast screencast(
-            default_region,
-            invalid_size,
-            default_pixel_format, stub_server,
-            nullptr,
-            null_callback_func, nullptr);
-    }, std::runtime_error);
-
-    EXPECT_THROW({
-        MirScreencast screencast(
-            invalid_region,
-            default_size,
-            default_pixel_format, stub_server,
-            nullptr,
-            null_callback_func, nullptr);
-    }, std::runtime_error);
-
-    EXPECT_THROW({
-        MirScreencast screencast(
-            default_region,
-            default_size,
-            mir_pixel_format_invalid, stub_server,
-            nullptr,
-            null_callback_func, nullptr);
+        MirScreencastSpec spec{default_spec};
+        spec.width = invalid_size.width.as_int();
+        spec.height = invalid_size.height.as_int();
+        MirScreencast screencast(spec, stub_server, null_callback_func, nullptr);
+    }, std::runtime_error);
+
+    EXPECT_THROW({
+        MirScreencastSpec spec{default_spec};
+        spec.capture_region = as_mir_rect(invalid_region);
+        MirScreencast screencast(spec, stub_server, null_callback_func, nullptr);
+    }, std::runtime_error);
+
+    EXPECT_THROW({
+        MirScreencastSpec spec{default_spec};
+        spec.pixel_format = mir_pixel_format_invalid;
+        MirScreencast screencast(spec, stub_server, null_callback_func, nullptr);
     }, std::runtime_error);
 }
 
@@ -292,12 +275,7 @@
     EXPECT_CALL(mock_server, create_screencast(_,_,_))
         .WillOnce(DoAll(SetCreateError(), RunClosure()));
 
-    MirScreencast screencast{
-        default_region,
-        default_size,
-        default_pixel_format, mock_server,
-        nullptr,
-        null_callback_func, nullptr};
+    MirScreencast screencast{default_spec, mock_server, null_callback_func, nullptr};
 
     screencast.creation_wait_handle()->wait_for_all();
 
@@ -313,14 +291,26 @@
         .WillOnce(DoAll(SetCreateError(), RunClosure()));
     EXPECT_CALL(mock_cb, call(_,&mock_cb));
 
-    MirScreencast screencast{
-        default_region,
-        default_size,
-        default_pixel_format, mock_server,
-        nullptr,
-        mock_callback_func, &mock_cb};
-
-    screencast.creation_wait_handle()->wait_for_all();
-
-    EXPECT_FALSE(screencast.valid());
+    MirScreencast screencast{default_spec, mock_server, mock_callback_func, &mock_cb};
+
+    screencast.creation_wait_handle()->wait_for_all();
+
+    EXPECT_FALSE(screencast.valid());
+}
+
+TEST_F(MirScreencastTest, release_callsback_even_if_invalid)
+{
+    using namespace testing;
+
+    EXPECT_CALL(mock_server, create_screencast(_,_,_))
+        .WillOnce(DoAll(SetCreateError(), RunClosure()));
+
+    MirScreencast screencast{default_spec, mock_server, null_callback_func, nullptr};
+    screencast.creation_wait_handle()->wait_for_all();
+    EXPECT_FALSE(screencast.valid());
+
+    MockCallback mock_cb;
+    EXPECT_CALL(mock_cb, call(_,&mock_cb));
+
+    screencast.release(mock_callback_func, &mock_cb);
 }