Code review comment for lp:~kdub/mir/screenshot-to-buffer

+++ include/client/mir_toolkit/mir_buffer.h 2017-02-21 22:04:27 +0000

+ void mir_screencast_capture_to_buffer(
+ MirError const* mir_screencast_capture_to_buffer_sync(MirScreencast* screencast, MirBuffer* buffer);

Shoudn't these be part of the screencast API (mir_screencast.h) rather than the buffer API?

+ if (request->response.has_error())
+ error = std::make_unique<MirError>(mir_error_domain_screencast, mir_screencast_error_failure);
+ else
+ error = nullptr;
+
+ request->available_callback(
+ reinterpret_cast<MirBuffer*>(request->buffer), error.get(), request->available_context);

We apparently support multiple concurrent screencast captures on the same object, but we share the error instance among them, leading to memory errors like:

auto const error1 = mir_screencast_capture_to_buffer(...);
auto const error2 = mir_screencast_capture_to_buffer(...);
// error1 now points to freed memory!

+ class CaptureSync

We can use std::promise<MirError*> instead (or our own no-TLS implementation if needed).