Merge lp:~kdub/mir/server-sent-buffer into lp:mir

[Kevin DuBois]

Give the server the ability to send a buffer to the client without a client request. (part of the buffer semantics changes). The buffer is sent as an event; as its the part of the system that's suited to sending unrequested messages to the server

Give the server the ability to send a buffer to the client without a client request. (part of the buffer semantics changes). The buffer is sent as an event; as its the part of the system that's suited to sending unrequested messages to the server.

Note: this can really only send the 'partial' buffer updates now, a bit more plumbing is needed to hook up the platform-specific packing operations for the full buffer sending.

The transition strategy here is that the client will use the exchange_buffer() call to submit its buffers, until it receives a buffer asynchronously. After receiving the first 'async' buffer, it submits its buffers via the submit_buffer() call.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2628
http://jenkins.qa.ubuntu.com/job/mir-ci/4034/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2753
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/266
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2701/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/190/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2701/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4034/rebuild

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

^^doesnt look related at first glance, but haven't ruled out that is yet (and cant reproduce locally)...

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2628
http://jenkins.qa.ubuntu.com/job/mir-ci/4038/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2758
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/271
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2706/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/194/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2706/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4038/rebuild

review: Needs Fixing (continuous-integration)

[Chris Halse Rogers (raof)]

Is there a document with the proposed new buffer semantics anywhere? This doesn't match my understanding from Dallas. (Not that it's *wrong*, just that it's unexpected ☺)

[Kevin DuBois (kdub)]

Notes are in this card: https://trello.com/c/wGVrm86z

There were two ways that I could think of to do this, the buffer gets sent when its ready (the approach in the MP), or the client has to call some sort of acquire-buffer rpc call. The send-to-client-when-done approach seemed better, as the client gets notified sooner, without asking, and with less IPC.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2629
http://jenkins.qa.ubuntu.com/job/mir-ci/4047/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2771
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/284
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2719
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/203
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/203/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2719
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2719/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5559
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21044

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4047/rebuild

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

Yeah I guess from the Dallas discussion I assumed this to be an event FD (buffer writable event) that would be get populated at the time of creation to avoid rpc.

My understanding was something akin to:

During buffer create, a couple of fence fds are created - one for the server to signal when the buffer is writable by the client again (i.e buffer is no longer being read) and another for the client to signal the server that the buffer is now readable (i.e. finished writing to the buffer).

An RPC call was needed for the client to "schedule" a buffer - as the fence fd did not have enough context for scheduling (i.e. timestamping)

An fd signaling a buffer is writable however, seems like there's enough context there to avoid explicit RPC calls.

review: Needs Information

[Kevin DuBois (kdub)]

There's also the problem that we'd have to send fences (in the android sense) with every signal of the fd pair, which is why I thought we shied away from wanting to do this.

[Chris Halse Rogers (raof)]

My understanding was that submit_buffer was going to take a {bufferid, when-readable-fence} and *return* a fence that becomes signalled when the buffer is client-writable again.

So, I guess the takeaway point is that we're not clear as a team how we expect this to work :)

[Kevin DuBois (kdub)]

Just returning one fence that signals (perhaps we would epoll on it?) still has the problem that we have to go and get the FD that indicates when the buffer is ready. (the 'standard' android fences). The android fences could be (and, without confirming, probably are) a different FD each time a buffer fence is returned from the driver, so we have to send that fd via ipc each time we get one.

There's two timelines going on... one is from the driver, and that fence protects the buffers content (when the GPU/display thinks the buffer is ready). The second timeline is when mir thinks its ready (perhaps there's screenshot going on, or we have to hold the buffer a bit longer for a convoluted multimonitor scenario), and both have to be clear before the gpu can go and write in the buffer on the client side.

[Alberto Aguirre (albaguirre)]

In the android platform I was envisioning returning the fence all the way to the driver not explicitly epolling.

ANW->dequeueBuffer would get any of the buffers that are not currently used by the driver along with its "this buffer is writable now" fence; that way no explicit waiting is done by the client code.

But I guess yeah the android fences could be different each time so an RPC would still be needed to update the fence on the client side.

Also the consumer side needs to be able to make the client wait until before retrieving a buffer (during dequeueBuffer) otherwise there would be a race getting the right sync fence fd.

Given that I guess we do need something like this MP.

[Alberto Aguirre (albaguirre)]

OK for transitioning from exchange buffer.

review: Approve

[Alan Griffiths (alan-griffiths)]

I guess this is OK as an intermediate form but...

21 + if (using_exchange_buffer)
22 + return exchange(done, std::move(lock));
23 + return submit(done, std::move(lock));
...
74 + using_exchange_buffer = false; //this stream uses async exchange from here on out
...
127 + bool using_exchange_buffer = true;

This feels like there's an abstraction missing. C.f.

    return state->send_buffer(done, std::move(lock));

Or perhaps a pointer to member function instead of a bool?

~~~~

35 + //in both cases, submit what we have

It would be helpful to explicitly state what the two cases are.

review: Approve

[Alexandros Frantzis (afrantzis)]

OK.

Nits:

432 + bool satisfied = false;

Clearer as 'stub_buffer_arrived' or similar.

429 + //spin-wait for the id to become the current one.

Could use mt::spin_wait_for_condition_or_timeout()

498 + bs->next_buffer([](){});

Could use just '[]{}'.

'[](){}' is a nice collection of brackets... C++ should introduce angle brackets in lambdas '[](){}⟨⟩' to become the first bracket-complete language! :)

review: Approve

[Andreas Pokorny (andreas-pokorny)]

ok looks good

review: Approve

[Kevin DuBois (kdub)]

@abstraction, There is one to be made, but we want to deprecate exchange_buffer soon anyways, so it didn't seem worth it to make this abstraction temporarily

[Kevin DuBois (kdub)]

@Alexandros, stopped shy of using mt::spin_wait_for_condition_or_timeout, as I have to call the swap buffers function in the loop, and didn't want to call code while waiting for a condition. Should have fixed the rest.

@Chris, still hesitant? :)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2632
http://jenkins.qa.ubuntu.com/job/mir-ci/4102/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2850
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/363
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2798
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/258
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/258/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2798
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2798/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5630
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21199

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4102/rebuild

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

Ah, right. Yes. The problem with returning the GPU fence from submit_buffer is that server ownership of that buffer quite likely extends past the GPU read.

Although, hm.

Under the assumption that fences are represented as pollable fds *and* that pollable fds can be used as fences¹, we *could* do the fence-is-allocated-at-buffer-allocation-time thing, by adding a new fence on each submit_buffer.

This requires fences that have a signalled→unsignalled transition, though, which is unusual.

I'm mostly happy with this; if it turns out we can do better then we can do that extra funky stuff later.

¹: Or, alternatively, that we can do composite fences; the main requirement is that we can post-hoc combine fences and Mir can create and signal a fence itself.

[Chris Halse Rogers (raof)]

Now, for the mostly happy bit:

submit_buffer() shouldn't block in that fashion. Why does it need to block at all?

I'm also not entirely sure why submit_buffer returns a MirWaitHandle, and even if it does I don't think that the MirWaitHandle it's returning should be the ‘there's a buffer available’ wait handle.

review: Needs Fixing

[Kevin DuBois (kdub)]

mir_buffer_stream_swap_buffers() currently will call exchange buffer, and return when that operation is done (ie, when BufferQueue signals the complete() function from SessionMediator). In this first pass, the intention is to maintain the same behavior, by ensuring that submit_buffer is done, and a new buffer has arrived ('asynchronously').

Once the protocol changes have completed, we'll have to have to have some exposition of the buffer semantics in the client api (so that nested and multimedia can use the feature), and we'll have to support mir_buffer_stream_swap_buffers by re-plumbing ClientBufferDepository to be more aware of many buffers.

For now, its working this way just in the name of compatibility, and because we haven't rooted out the uni-buffer concept from other client side classes.

[Chris Halse Rogers (raof)]

Isn't that the purpose of the done callback? Why is there the additional submitting condition variable, and the wait on that?

-----Original Message-----
From: "Kevin DuBois" <email address hidden>
Sent: ‎16/‎06/‎2015 21:35
To: "Kevin DuBois" <email address hidden>
Subject: Re: [Merge] lp:~kdub/mir/server-sent-buffer into lp:mir

mir_buffer_stream_swap_buffers() currently will call exchange buffer, and return when that operation is done (ie, when BufferQueue signals the complete() function from SessionMediator). In this first pass, the intention is to maintain the same behavior, by ensuring that submit_buffer is done, and a new buffer has arrived ('asynchronously').

Once the protocol changes have completed, we'll have to have to have some exposition of the buffer semantics in the client api (so that nested and multimedia can use the feature), and we'll have to support mir_buffer_stream_swap_buffers by re-plumbing ClientBufferDepository to be more aware of many buffers.

For now, its working this way just in the name of compatibility, and because we haven't rooted out the uni-buffer concept from other client side classes.
--
https://code.launchpad.net/~kdub/mir/server-sent-buffer/+merge/261421
You are reviewing the proposed merge of lp:~kdub/mir/server-sent-buffer into lp:mir.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2634
http://jenkins.qa.ubuntu.com/job/mir-ci/4141/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2911
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/426
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2859
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/297
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/297/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2859
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2859/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5685
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21291

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4141/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

> Isn't that the purpose of the done callback? Why is there the additional
> submitting condition variable, and the wait on that?

For now (in the intermediate form), before mcl::BufferStream::next_buffer() returns, we have to wait for two things to happen... for submit buffer to finish and for the new buffer to come back over ipc. If we had a buffer, then we could just process things in the done callback. If the event-buffer arrives after, however, we can wait in the callback (blocking RPC processing). It seemed easiest to just wait for submit buffer to be done like in this MP.

[Chris Halse Rogers (raof)]

Why do we have to wait for submit_buffer to finish?

We process RPC requests in order on the server, so as soon as it's submitted we know that it's going to be processed at the appropriate time.

As far as I can see, the client never needs to know when the server has acknowledged the submission; and for the transition it only needs to know when there's a buffer available to render into.

I think of any behaviour that would be incorrect if you treated submit_buffer as not having any response at all. If you tried it, what went wrong?

[Kevin DuBois (kdub)]

@Chris, I see now what you're saying, makes sense. We don't have to wait for the /completion/ of submit_buffer on the server side, we just have to accomplish the sending-off of the submit_buffer request, and then get another buffer in the client side before returning.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2636
http://jenkins.qa.ubuntu.com/job/mir-ci/4151/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2929
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/444
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2877/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/307/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2877/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4151/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2637
http://jenkins.qa.ubuntu.com/job/mir-ci/4158/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2938
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/453
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2886
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/314/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2886
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2886/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5710
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21333

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4158/rebuild

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

seems like CI just stopped before getting around to unit/integration?

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2637
http://jenkins.qa.ubuntu.com/job/mir-ci/4161/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2944
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/459
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2892/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/317
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/317/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2892
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2892/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5716/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21356

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4161/rebuild

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

Err http://ports.ubuntu.com/ubuntu-ports/ wily/universe glmark2-data all 2014.03+git20150611.fa71af2d-0ubuntu1
  Temporary failure resolving 'ports.ubuntu.com'

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2637
http://jenkins.qa.ubuntu.com/job/mir-ci/4162/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2945
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/460
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2893
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/318
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/318/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2893
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2893/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5717
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21363

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4162/rebuild

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

LGTM now.

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/client/buffer_stream.cpp'
--- src/client/buffer_stream.cpp	2015-06-17 18:24:42 +0000
+++ src/client/buffer_stream.cpp	2015-06-19 17:03:24 +0000
@@ -167,7 +167,11 @@
 void mcl::BufferStream::process_buffer(mp::Buffer const& buffer)
 {
     std::unique_lock<decltype(mutex)> lock(mutex);
-    
+    process_buffer(buffer, lock);
+}
+
+void mcl::BufferStream::process_buffer(protobuf::Buffer const& buffer, std::unique_lock<std::mutex> const&)
+{
     auto buffer_package = std::make_shared<MirBufferPackage>();
     populate_buffer_package(*buffer_package, buffer);
     
@@ -202,6 +206,37 @@
 
     secured_region.reset();
 
+    if (using_exchange_buffer)
+        return exchange(done, std::move(lock));
+    return submit(done, std::move(lock));
+}
+
+MirWaitHandle* mcl::BufferStream::submit(std::function<void()> const& done, std::unique_lock<std::mutex> lock)
+{
+    //always submit what we have, whether we have a buffer, or will have to wait for an async reply
+    auto request = mcl::make_protobuf_object<mp::BufferRequest>();
+    request->mutable_id()->set_value(protobuf_bs->id().value());
+    request->mutable_buffer()->set_buffer_id(protobuf_bs->buffer().buffer_id());
+    display_server.submit_buffer(nullptr, request.get(), protobuf_void.get(),
+        google::protobuf::NewCallback(google::protobuf::DoNothing));
+
+    if (incoming_buffers.empty())
+    {
+        next_buffer_wait_handle.expect_result();
+        on_incoming_buffer = done; 
+    }
+    else
+    {
+        process_buffer(incoming_buffers.front(), lock);
+        incoming_buffers.pop();
+        done();
+    }
+    return &next_buffer_wait_handle;
+}
+
+MirWaitHandle* mcl::BufferStream::exchange(
+    std::function<void()> const& done, std::unique_lock<std::mutex> lock)
+{
     // TODO: We can fix the strange "ID casting" used below in the second phase
     // of buffer stream which generalizes and clarifies the server side logic.
     if (mode == mcl::BufferStreamMode::Producer)
@@ -395,3 +430,21 @@
 {
     buffer_depository.set_max_buffers(cache_size);
 }
+
+void mcl::BufferStream::buffer_available(mir::protobuf::Buffer const& buffer)
+{
+    std::unique_lock<decltype(mutex)> lock(mutex);
+    using_exchange_buffer = false; //this stream uses async exchange from here on out
+
+    if (on_incoming_buffer)
+    {
+        process_buffer(buffer, lock);
+        next_buffer_wait_handle.result_received();
+        on_incoming_buffer();
+        on_incoming_buffer = std::function<void()>{};
+    }
+    else
+    {
+        incoming_buffers.push(buffer);
+    }
+}
 
=== modified file 'src/client/buffer_stream.h'
--- src/client/buffer_stream.h	2015-06-17 18:24:42 +0000
+++ src/client/buffer_stream.h	2015-06-19 17:03:24 +0000
@@ -31,6 +31,7 @@
 
 #include <EGL/eglplatform.h>
 
+#include <queue>
 #include <memory>
 #include <mutex>
 
@@ -104,6 +105,9 @@
 
     frontend::BufferStreamId rpc_id() const override;
     bool valid() const override;
+    
+    void buffer_available(mir::protobuf::Buffer const& buffer) override;
+    
 protected:
     BufferStream(BufferStream const&) = delete;
     BufferStream& operator=(BufferStream const&) = delete;
@@ -111,13 +115,20 @@
 private:
     void created(mir_buffer_stream_callback callback, void* context);
     void process_buffer(protobuf::Buffer const& buffer);
+    void process_buffer(protobuf::Buffer const& buffer, std::unique_lock<std::mutex> const&);
     void next_buffer_received(
         std::function<void()> done);
     void on_configured();
     void release_cpu_region();
+    MirWaitHandle* exchange(std::function<void()> const& done, std::unique_lock<std::mutex> lk);
+    MirWaitHandle* submit(std::function<void()> const& done, std::unique_lock<std::mutex> lk);
 
     mutable std::mutex mutex; // Protects all members of *this
 
+    bool using_exchange_buffer = true;
+    std::function<void()> on_incoming_buffer;
+    std::queue<mir::protobuf::Buffer> incoming_buffers;
+
     MirConnection* connection;
     mir::protobuf::DisplayServer& display_server;
 
 
=== modified file 'src/client/client_buffer_stream.h'
--- src/client/client_buffer_stream.h	2015-06-17 05:20:42 +0000
+++ src/client/client_buffer_stream.h	2015-06-19 17:03:24 +0000
@@ -63,7 +63,8 @@
     virtual MirWaitHandle* release(mir_buffer_stream_callback callback, void* context) = 0;
 
     virtual bool valid() const = 0;
-    
+    virtual void buffer_available(mir::protobuf::Buffer const& buffer) = 0;
+
 protected:
     ClientBufferStream() = default;
     ClientBufferStream(const ClientBufferStream&) = delete;
 
=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.cpp'
--- src/client/rpc/mir_protobuf_rpc_channel.cpp	2015-06-17 18:24:42 +0000
+++ src/client/rpc/mir_protobuf_rpc_channel.cpp	2015-06-19 17:03:24 +0000
@@ -251,6 +251,14 @@
         lifecycle_control->call_lifecycle_event_handler(seq->lifecycle_event().new_state());
     }
 
+    if (seq->has_buffer_request())
+    {
+        surface_map->with_stream_do(mf::BufferStreamId(seq->buffer_request().id().value()),
+        [&] (mcl::ClientBufferStream* stream) {
+            stream->buffer_available(seq->buffer_request().buffer());
+        });
+    }
+
     int const nevents = seq->event_size();
     for (int i = 0; i != nevents; ++i)
     {
 
=== modified file 'src/include/server/mir/frontend/event_sink.h'
--- src/include/server/mir/frontend/event_sink.h	2015-02-22 07:46:25 +0000
+++ src/include/server/mir/frontend/event_sink.h	2015-06-19 17:03:24 +0000
@@ -20,12 +20,14 @@
 #define MIR_EVENTS_EVENT_SINK_H_
 
 #include "mir_toolkit/event.h"
+#include "mir/frontend/buffer_stream_id.h"
 
 namespace mir
 {
 namespace graphics
 {
 class DisplayConfiguration;
+class Buffer;
 }
 namespace frontend
 {
@@ -37,6 +39,7 @@
     virtual void handle_event(MirEvent const& e) = 0;
     virtual void handle_lifecycle_event(MirLifecycleState state) = 0;
     virtual void handle_display_config_change(graphics::DisplayConfiguration const& config) = 0;
+    virtual void send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer) = 0;
 
 protected:
     EventSink() = default;
 
=== modified file 'src/protobuf/mir_protobuf.proto'
--- src/protobuf/mir_protobuf.proto	2015-06-17 19:16:03 +0000
+++ src/protobuf/mir_protobuf.proto	2015-06-19 17:03:24 +0000
@@ -245,6 +245,8 @@
   repeated Event event = 1;
   optional DisplayConfiguration display_configuration = 2;
   optional LifecycleEvent lifecycle_event = 3;
+  optional BufferRequest buffer_request = 4;
+
   optional string error = 127;
 }
 
 
=== modified file 'src/server/frontend/event_sender.cpp'
--- src/server/frontend/event_sender.cpp	2015-06-17 05:20:42 +0000
+++ src/server/frontend/event_sender.cpp	2015-06-19 17:03:24 +0000
@@ -24,6 +24,8 @@
 #include "message_sender.h"
 #include "protobuf_buffer_packer.h"
 
+#include "mir/graphics/buffer.h"
+
 #include "mir_protobuf_wire.pb.h"
 #include "mir_protobuf.pb.h"
 
@@ -95,3 +97,12 @@
         (void) error;
     }
 }
+
+void mfd::EventSender::send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer)
+{
+    mp::EventSequence seq;
+    auto request = seq.mutable_buffer_request();
+    request->mutable_id()->set_value(id.as_value()); 
+    request->mutable_buffer()->set_buffer_id(buffer.id().as_value());
+    send_event_sequence(seq);
+}
 
=== modified file 'src/server/frontend/event_sender.h'
--- src/server/frontend/event_sender.h	2015-02-22 07:46:25 +0000
+++ src/server/frontend/event_sender.h	2015-06-19 17:03:24 +0000
@@ -41,6 +41,7 @@
     void handle_event(MirEvent const& e);
     void handle_lifecycle_event(MirLifecycleState state);
     void handle_display_config_change(graphics::DisplayConfiguration const& config);
+    void send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer);
 
 private:
     void send_event_sequence(protobuf::EventSequence&);
 
=== modified file 'src/server/scene/global_event_sender.cpp'
--- src/server/scene/global_event_sender.cpp	2015-02-22 07:46:25 +0000
+++ src/server/scene/global_event_sender.cpp	2015-06-19 17:03:24 +0000
@@ -45,3 +45,7 @@
         session->send_display_config(config);
     });
 }
+
+void ms::GlobalEventSender::send_buffer(mir::frontend::BufferStreamId, mg::Buffer&)
+{
+}
 
=== modified file 'src/server/scene/global_event_sender.h'
--- src/server/scene/global_event_sender.h	2015-02-22 07:46:25 +0000
+++ src/server/scene/global_event_sender.h	2015-06-19 17:03:24 +0000
@@ -36,6 +36,7 @@
     void handle_event(MirEvent const& e);
     void handle_lifecycle_event(MirLifecycleState state);
     void handle_display_config_change(graphics::DisplayConfiguration const& config);
+    void send_buffer(frontend::BufferStreamId id, graphics::Buffer& buffer);
 
 private:
     std::shared_ptr<SessionContainer> const sessions;
 
=== modified file 'tests/include/mir_test_doubles/mock_client_buffer_stream.h'
--- tests/include/mir_test_doubles/mock_client_buffer_stream.h	2015-06-17 05:20:42 +0000
+++ tests/include/mir_test_doubles/mock_client_buffer_stream.h	2015-06-19 17:03:24 +0000
@@ -45,6 +45,7 @@
     MOCK_METHOD0(get_create_wait_handle, MirWaitHandle*(void));
     MOCK_CONST_METHOD0(rpc_id, frontend::BufferStreamId(void));
     MOCK_CONST_METHOD0(valid, bool(void));
+    MOCK_METHOD1(buffer_available, void(mir::protobuf::Buffer const&));
 };
 
 }
 
=== modified file 'tests/include/mir_test_doubles/mock_event_sink.h'
--- tests/include/mir_test_doubles/mock_event_sink.h	2015-02-11 22:10:31 +0000
+++ tests/include/mir_test_doubles/mock_event_sink.h	2015-06-19 17:03:24 +0000
@@ -34,6 +34,7 @@
     MOCK_METHOD1(handle_event, void(MirEvent const&));
     MOCK_METHOD1(handle_lifecycle_event, void(MirLifecycleState));
     MOCK_METHOD1(handle_display_config_change, void(graphics::DisplayConfiguration const&));
+    MOCK_METHOD2(send_buffer, void(frontend::BufferStreamId, graphics::Buffer&));
 };
 }
 }
 
=== modified file 'tests/include/mir_test_doubles/null_event_sink.h'
--- tests/include/mir_test_doubles/null_event_sink.h	2015-02-22 07:46:25 +0000
+++ tests/include/mir_test_doubles/null_event_sink.h	2015-06-19 17:03:24 +0000
@@ -32,6 +32,7 @@
     void handle_event(MirEvent const&) {}
     void handle_lifecycle_event(MirLifecycleState) {}
     void handle_display_config_change(graphics::DisplayConfiguration const&) {}
+    void send_buffer(frontend::BufferStreamId, graphics::Buffer&) {}
 };
 }
 }
 
=== modified file 'tests/integration-tests/test_exchange_buffer.cpp'
--- tests/integration-tests/test_exchange_buffer.cpp	2015-06-17 19:16:03 +0000
+++ tests/integration-tests/test_exchange_buffer.cpp	2015-06-19 17:03:24 +0000
@@ -28,10 +28,13 @@
 #include "mir/graphics/buffer_ipc_message.h"
 #include "mir/graphics/platform_operation_message.h"
 #include "mir/scene/buffer_stream_factory.h"
+#include "mir/scene/session_coordinator.h"
+#include "mir/frontend/event_sink.h"
 #include "mir/compositor/buffer_stream.h"
 #include "src/server/compositor/buffer_bundle.h"
 #include "src/server/compositor/buffer_stream_surfaces.h"
 #include "mir_toolkit/mir_client_library.h"
+#include "mir_toolkit/debug/surface.h"
 #include "src/client/mir_connection.h"
 #include <chrono>
 #include <mutex>
@@ -49,6 +52,7 @@
 namespace mc = mir::compositor;
 namespace geom = mir::geometry;
 namespace mp = mir::protobuf;
+namespace mf = mir::frontend;
 
 namespace
 {
@@ -176,6 +180,47 @@
     std::shared_ptr<mg::PlatformIpcOperations> const ipc_ops;
 };
 
+class SinkSkimmingCoordinator : public msc::SessionCoordinator
+{
+public:
+    SinkSkimmingCoordinator(std::shared_ptr<msc::SessionCoordinator> const& wrapped) :
+        wrapped(wrapped)
+    {
+    }
+
+    void set_focus_to(std::shared_ptr<msc::Session> const& focus) override
+    {
+        wrapped->set_focus_to(focus);
+    }
+
+    void unset_focus() override
+    {
+        wrapped->unset_focus();
+    }
+
+    std::shared_ptr<msc::Session> open_session(
+        pid_t client_pid,
+        std::string const& name,
+        std::shared_ptr<mf::EventSink> const& sink) override
+    {
+        last_sink = sink;
+        return wrapped->open_session(client_pid, name, sink);
+    }
+
+    void close_session(std::shared_ptr<msc::Session> const& session) override
+    {
+        wrapped->close_session(session);
+    }
+
+    std::shared_ptr<msc::Session> successor_of(std::shared_ptr<msc::Session> const& session) const override
+    {
+        return wrapped->successor_of(session);
+    }
+
+    std::shared_ptr<msc::SessionCoordinator> const wrapped;
+    std::weak_ptr<mf::EventSink> last_sink;
+};
+
 struct ExchangeServerConfiguration : mtf::StubbedServerConfiguration
 {
     ExchangeServerConfiguration(
@@ -186,6 +231,14 @@
     {
     }
 
+    std::shared_ptr<msc::SessionCoordinator> the_session_coordinator() override
+    {
+        return session_coordinator([this]{
+            coordinator = std::make_shared<SinkSkimmingCoordinator>(
+                DefaultServerConfiguration::the_session_coordinator());
+            return coordinator;
+        });
+    }
     std::shared_ptr<mg::Platform> the_graphics_platform() override
     {
         return platform;
@@ -196,8 +249,9 @@
         return stream_factory;
     }
 
-    std::shared_ptr<msc::BufferStreamFactory> const stream_factory;
+    std::shared_ptr<StubBundleFactory> const stream_factory;
     std::shared_ptr<mg::Platform> const platform;
+    std::shared_ptr<SinkSkimmingCoordinator> coordinator;
 };
 
 struct ExchangeBufferTest : mir_test_framework::InProcessServer
@@ -217,8 +271,6 @@
         cv.notify_all();
     }
 
-    //TODO: once the next_buffer rpc is deprecated, change this code out for the
-    //      mir_surface_next_buffer() api call
     bool exchange_buffer(mp::DisplayServer& server)
     {
         std::unique_lock<decltype(mutex)> lk(mutex);
@@ -359,6 +411,37 @@
     mir_connection_release(connection);
 }
 
+TEST_F(ExchangeBufferTest, server_can_send_buffer)
+{
+    using namespace testing;
+    using namespace std::literals::chrono_literals;
+    mtd::StubBuffer stub_buffer;
+    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
+    auto surface = mtf::make_any_surface(connection);
+    auto sink = server_configuration.coordinator->last_sink.lock();
+    sink->send_buffer(mf::BufferStreamId{0}, stub_buffer);
+
+    //spin-wait for the id to become the current one.
+    //The notification doesn't generate a client-facing callback on the stream yet
+    //(although probably should, seems something a media decoder would need
+    bool buffer_arrived = false;
+    auto timeout = std::chrono::steady_clock::now() + 5s;
+    while(!buffer_arrived && std::chrono::steady_clock::now() < timeout)
+    {
+        mir_buffer_stream_swap_buffers_sync(mir_surface_get_buffer_stream(surface));
+        if (mir_debug_surface_current_buffer_id(surface) == stub_buffer.id().as_value())
+        {
+            buffer_arrived = true;
+            break;
+        }
+        std::this_thread::yield();
+    }
+    EXPECT_THAT(buffer_arrived, Eq(true)) << "failed to see the sent buffer become the current one";
+
+    mir_surface_release_sync(surface);
+    mir_connection_release(connection);
+}
+
 //TODO: check that a buffer arrives asynchronously.
 TEST_F(ExchangeBufferTest, allocate_buffers_doesnt_time_out)
 {
 
=== modified file 'tests/unit-tests/client/test_client_buffer_stream.cpp'
--- tests/unit-tests/client/test_client_buffer_stream.cpp	2015-06-17 05:20:42 +0000
+++ tests/unit-tests/client/test_client_buffer_stream.cpp	2015-06-19 17:03:24 +0000
@@ -50,6 +50,11 @@
                       mp::ScreencastId const* /*request*/,
                       mp::Buffer* /*response*/,
                       google::protobuf::Closure* /*done*/));
+    MOCK_METHOD4(submit_buffer,
+                 void(google::protobuf::RpcController* /*controller*/,
+                      mp::BufferRequest const* /*request*/,
+                      mp::Void* /*response*/,
+                      google::protobuf::Closure* /*done*/));
     MOCK_METHOD4(exchange_buffer,
                  void(google::protobuf::RpcController* /*controller*/,
                       mp::BufferRequest const* /*request*/,
@@ -113,7 +118,7 @@
 
 struct ClientBufferStreamTest : public testing::Test
 {
-    mtd::MockClientBufferFactory mock_client_buffer_factory;
+    testing::NiceMock<mtd::MockClientBufferFactory> mock_client_buffer_factory;
     mtd::StubClientBufferFactory stub_client_buffer_factory;
 
     MockProtobufServer mock_protobuf_server;
@@ -283,7 +288,7 @@
 
     auto bs = make_buffer_stream(protobuf_bs, mcl::BufferStreamMode::Producer);
     
-    bs->next_buffer([](){});
+    bs->next_buffer([]{});
 }
 
 TEST_F(ClientBufferStreamTest, consumer_streams_call_screencast_buffer_on_next_buffer)
@@ -298,7 +303,7 @@
 
     auto bs = make_buffer_stream(protobuf_bs, mcl::BufferStreamMode::Consumer);
     
-    bs->next_buffer([](){});
+    bs->next_buffer([]{});
 }
 
 TEST_F(ClientBufferStreamTest, invokes_callback_on_next_buffer)
@@ -363,7 +368,7 @@
     auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory);
 
     EXPECT_EQ(client_buffer_1, bs->get_current_buffer());
-    bs->next_buffer([](){});
+    bs->next_buffer([]{});
     EXPECT_EQ(client_buffer_2, bs->get_current_buffer());
 }
 
@@ -397,7 +402,7 @@
     auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory);
 
     EXPECT_EQ(client_buffer_1, bs->get_current_buffer());
-    bs->next_buffer([](){});
+    bs->next_buffer([]{});
     EXPECT_EQ(client_buffer_2, bs->get_current_buffer());
 }
 
@@ -453,3 +458,60 @@
         std::make_shared<StubClientPlatform>(mt::fake_shared(stub_client_buffer_factory)),
         protobuf_bs, mt::fake_shared(mock_perf_report), name);
 }
+
+TEST_F(ClientBufferStreamTest, receives_unsolicited_buffer)
+{
+    using namespace ::testing;
+    int id = 88;
+    MockClientBuffer mock_client_buffer;
+    MockClientBuffer second_mock_client_buffer;
+    MirBufferPackage buffer_package = a_buffer_package();
+    auto protobuf_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage, buffer_package);
+    EXPECT_CALL(mock_client_buffer_factory, create_buffer(BufferPackageMatches(buffer_package),_,_))
+        .WillOnce(Return(mt::fake_shared(mock_client_buffer)));
+    auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory);
+
+    mir::protobuf::Buffer another_buffer_package;
+    another_buffer_package.set_buffer_id(id);
+    EXPECT_CALL(mock_client_buffer_factory, create_buffer(_,_,_))
+        .WillOnce(Return(mt::fake_shared(second_mock_client_buffer)));
+    EXPECT_CALL(mock_protobuf_server, submit_buffer(_,_,_,_))
+        .WillOnce(RunProtobufClosure());
+    bs->buffer_available(another_buffer_package);
+    bs->next_buffer([]{});
+
+    EXPECT_THAT(bs->get_current_buffer().get(), Eq(&second_mock_client_buffer));
+    EXPECT_THAT(bs->get_current_buffer_id(), Eq(id));
+}
+
+//useful only in transitioning from exchange to async
+TEST_F(ClientBufferStreamTest, after_receiving_an_unsolicited_buffer_exchange_buffer_return_are_ignored)
+{
+    using namespace ::testing;
+    int id = 88;
+    MockClientBuffer mock_client_buffer;
+    MockClientBuffer second_mock_client_buffer;
+    MirBufferPackage buffer_package = a_buffer_package();
+    mir::protobuf::Buffer another_buffer_package;
+    another_buffer_package.set_buffer_id(id);
+    auto protobuf_bs = a_protobuf_buffer_stream(default_pixel_format, default_buffer_usage, buffer_package);
+    ON_CALL(mock_client_buffer_factory, create_buffer(_,_,_))
+        .WillByDefault(Return(mt::fake_shared(mock_client_buffer)));
+    auto bs = make_buffer_stream(protobuf_bs, mock_client_buffer_factory);
+
+    int a_few_times = 11;
+    EXPECT_CALL(mock_protobuf_server, exchange_buffer(_,_,_,_))
+        .Times(a_few_times)
+        .WillRepeatedly(RunProtobufClosure());
+    EXPECT_CALL(mock_protobuf_server, submit_buffer(_,_,_,_))
+        .Times(a_few_times)
+        .WillRepeatedly(RunProtobufClosure());
+
+    for(auto i = 0; i < a_few_times; i++) 
+        bs->next_buffer([]{});
+
+    bs->buffer_available(another_buffer_package);
+
+    for(auto i = 0; i < a_few_times; i++) 
+        bs->next_buffer([]{});
+}