Merge changes I895cb04f,I9741a38f,I928fa370,I8dc9c122 into main
* changes:
Eliminate a short blackout during the resume of the VM display
More refactoring on android display service
Distinguish error messages from scanout surface and cursor surface
Better error handling using android-base/Result.h
diff --git a/android/VmLauncherApp/java/com/android/virtualization/vmlauncher/MainActivity.java b/android/VmLauncherApp/java/com/android/virtualization/vmlauncher/MainActivity.java
index 0b93968..a93c173 100644
--- a/android/VmLauncherApp/java/com/android/virtualization/vmlauncher/MainActivity.java
+++ b/android/VmLauncherApp/java/com/android/virtualization/vmlauncher/MainActivity.java
@@ -457,10 +457,16 @@
+ holder.getSurface()
+ ")");
runWithDisplayService(
- (service) ->
- service.setSurface(
+ s ->
+ s.setSurface(
holder.getSurface(),
false /* forCursor */));
+ // TODO execute the above and the below togther with the same call
+ // to runWithDisplayService. Currently this doesn't work because
+ // setSurface somtimes trigger an exception and as a result
+ // drawSavedFrameForSurface is skipped.
+ runWithDisplayService(
+ s -> s.drawSavedFrameForSurface(false /* forCursor */));
}
@Override
@@ -544,6 +550,12 @@
}
@Override
+ protected void onPause() {
+ super.onPause();
+ runWithDisplayService(s -> s.saveFrameForSurface(false /* forCursor */));
+ }
+
+ @Override
protected void onStop() {
super.onStop();
if (mVirtualMachine != null) {
diff --git a/libs/android_display_backend/Android.bp b/libs/android_display_backend/Android.bp
index 32587dd..f682627 100644
--- a/libs/android_display_backend/Android.bp
+++ b/libs/android_display_backend/Android.bp
@@ -46,6 +46,9 @@
"android.system.virtualizationcommon-ndk",
"android.system.virtualizationservice-ndk",
],
+ static_libs: [
+ "libbase",
+ ],
shared_libs: [
"libbinder_ndk",
"libnativewindow",
diff --git a/libs/android_display_backend/aidl/android/crosvm/ICrosvmAndroidDisplayService.aidl b/libs/android_display_backend/aidl/android/crosvm/ICrosvmAndroidDisplayService.aidl
index e42cdd1..77e3a8c 100644
--- a/libs/android_display_backend/aidl/android/crosvm/ICrosvmAndroidDisplayService.aidl
+++ b/libs/android_display_backend/aidl/android/crosvm/ICrosvmAndroidDisplayService.aidl
@@ -27,4 +27,6 @@
void setSurface(inout Surface surface, boolean forCursor);
void setCursorStream(in ParcelFileDescriptor stream);
void removeSurface(boolean forCursor);
+ void saveFrameForSurface(boolean forCursor);
+ void drawSavedFrameForSurface(boolean forCursor);
}
diff --git a/libs/android_display_backend/crosvm_android_display_client.cpp b/libs/android_display_backend/crosvm_android_display_client.cpp
index 6e4a793..3802a69 100644
--- a/libs/android_display_backend/crosvm_android_display_client.cpp
+++ b/libs/android_display_backend/crosvm_android_display_client.cpp
@@ -16,6 +16,7 @@
#include <aidl/android/crosvm/BnCrosvmAndroidDisplayService.h>
#include <aidl/android/system/virtualizationservice_internal/IVirtualizationServiceInternal.h>
+#include <android-base/result.h>
#include <android/binder_manager.h>
#include <android/binder_process.h>
#include <system/graphics.h> // for HAL_PIXEL_FORMAT_*
@@ -28,16 +29,16 @@
using aidl::android::system::virtualizationservice_internal::IVirtualizationServiceInternal;
using aidl::android::view::Surface;
+using android::base::Error;
+using android::base::Result;
+
namespace {
class SinkANativeWindow_Buffer {
public:
- SinkANativeWindow_Buffer() = default;
- virtual ~SinkANativeWindow_Buffer() = default;
-
- bool configure(uint32_t width, uint32_t height, int format) {
+ Result<void> configure(uint32_t width, uint32_t height, int format) {
if (format != HAL_PIXEL_FORMAT_BGRA_8888) {
- return false;
+ return Error() << "Pixel format " << format << " is not BGRA_8888.";
}
mBufferBits.resize(width * height * 4);
@@ -48,7 +49,7 @@
.format = format,
.bits = mBufferBits.data(),
};
- return true;
+ return {};
}
operator ANativeWindow_Buffer&() { return mBuffer; }
@@ -58,16 +59,28 @@
std::vector<uint8_t> mBufferBits;
};
-// Wrapper which contains the latest available Surface/ANativeWindow
-// from the DisplayService, if available. A Surface/ANativeWindow may
-// not always be available if, for example, the VmLauncherApp on the
-// other end of the DisplayService is not in the foreground / is paused.
+static Result<void> copyBuffer(ANativeWindow_Buffer& from, ANativeWindow_Buffer& to) {
+ if (from.width != to.width || from.height != to.height) {
+ return Error() << "dimension mismatch. from=(" << from.width << ", " << from.height << ") "
+ << "to=(" << to.width << ", " << to.height << ")";
+ }
+ uint32_t* dst = reinterpret_cast<uint32_t*>(to.bits);
+ uint32_t* src = reinterpret_cast<uint32_t*>(from.bits);
+ size_t bytes_on_line = to.width * 4; // 4 bytes per pixel
+ for (int32_t h = 0; h < to.height; h++) {
+ memcpy(dst + (h * to.stride), src + (h * from.stride), bytes_on_line);
+ }
+ return {};
+}
+
+// Wrapper which contains the latest available Surface/ANativeWindow from the DisplayService, if
+// available. A Surface/ANativeWindow may not always be available if, for example, the VmLauncherApp
+// on the other end of the DisplayService is not in the foreground / is paused.
class AndroidDisplaySurface {
public:
- AndroidDisplaySurface() = default;
- virtual ~AndroidDisplaySurface() = default;
+ AndroidDisplaySurface(const std::string& name) : mName(name) {}
- void setSurface(Surface* surface) {
+ void setNativeSurface(Surface* surface) {
{
std::lock_guard lk(mSurfaceMutex);
mNativeSurface = std::make_unique<Surface>(surface->release());
@@ -90,7 +103,7 @@
return mNativeSurface.get();
}
- void configure(uint32_t width, uint32_t height) {
+ Result<void> configure(uint32_t width, uint32_t height) {
std::unique_lock lk(mSurfaceMutex);
mRequestedSurfaceDimensions = Rect{
@@ -98,7 +111,13 @@
.height = height,
};
- mSinkBuffer.configure(width, height, kFormat);
+ if (auto ret = mSinkBuffer.configure(width, height, kFormat); !ret.ok()) {
+ return Error() << "Failed to configure sink buffer: " << ret.error();
+ }
+ if (auto ret = mSavedFrameBuffer.configure(width, height, kFormat); !ret.ok()) {
+ return Error() << "Failed to configure saved frame buffer: " << ret.error();
+ }
+ return {};
}
void waitForNativeSurface() {
@@ -106,7 +125,7 @@
mNativeSurfaceReady.wait(lk, [this] { return mNativeSurface != nullptr; });
}
- int lock(ANativeWindow_Buffer* out_buffer) {
+ Result<void> lock(ANativeWindow_Buffer* out_buffer) {
std::unique_lock lk(mSurfaceMutex);
Surface* surface = mNativeSurface.get();
@@ -114,62 +133,137 @@
// Surface not currently available but not necessarily an error
// if, for example, the VmLauncherApp is not in the foreground.
*out_buffer = mSinkBuffer;
- return 0;
+ return {};
}
ANativeWindow* anw = surface->get();
if (anw == nullptr) {
- return -1;
+ return Error() << "Failed to get ANativeWindow";
}
if (mNativeSurfaceNeedsConfiguring) {
if (!mRequestedSurfaceDimensions) {
- return -1;
+ return Error() << "Surface dimension is not configured yet!";
}
const auto& dims = *mRequestedSurfaceDimensions;
// Ensure locked buffers have our desired format.
if (ANativeWindow_setBuffersGeometry(anw, dims.width, dims.height, kFormat) != 0) {
- return -1;
+ return Error() << "Failed to set buffer geometry.";
}
mNativeSurfaceNeedsConfiguring = false;
}
- return ANativeWindow_lock(anw, out_buffer, nullptr);
+ if (ANativeWindow_lock(anw, out_buffer, nullptr) != 0) {
+ return Error() << "Failed to lock window";
+ }
+ mLastBuffer = *out_buffer;
+ return {};
}
- int unlockAndPost() {
+ Result<void> unlockAndPost() {
std::unique_lock lk(mSurfaceMutex);
Surface* surface = mNativeSurface.get();
if (surface == nullptr) {
// Surface not currently available but not necessarily an error
// if, for example, the VmLauncherApp is not in the foreground.
- return 0;
+ return {};
}
ANativeWindow* anw = surface->get();
if (anw == nullptr) {
- return -1;
+ return Error() << "Failed to get ANativeWindow";
}
- return ANativeWindow_unlockAndPost(anw);
+ if (ANativeWindow_unlockAndPost(anw) != 0) {
+ return Error() << "Failed to unlock and post window";
+ }
+ return {};
}
+ // Saves the last frame drawn
+ Result<void> saveFrame() {
+ std::unique_lock lk(mSurfaceMutex);
+ if (auto ret = copyBuffer(mLastBuffer, mSavedFrameBuffer); !ret.ok()) {
+ return Error() << "Failed to copy frame: " << ret.error();
+ }
+ return {};
+ }
+
+ // Draws the saved frame
+ Result<void> drawSavedFrame() {
+ std::unique_lock lk(mSurfaceMutex);
+ Surface* surface = mNativeSurface.get();
+ if (surface == nullptr) {
+ return Error() << "Surface not ready";
+ }
+
+ ANativeWindow* anw = surface->get();
+ if (anw == nullptr) {
+ return Error() << "Failed to get ANativeWindow";
+ }
+
+ // TODO: dedup this and the one in lock(...)
+ if (mNativeSurfaceNeedsConfiguring) {
+ if (!mRequestedSurfaceDimensions) {
+ return Error() << "Surface dimension is not configured yet!";
+ }
+ const auto& dims = *mRequestedSurfaceDimensions;
+
+ // Ensure locked buffers have our desired format.
+ if (ANativeWindow_setBuffersGeometry(anw, dims.width, dims.height, kFormat) != 0) {
+ return Error() << "Failed to set buffer geometry.";
+ }
+
+ mNativeSurfaceNeedsConfiguring = false;
+ }
+
+ ANativeWindow_Buffer buf;
+ if (ANativeWindow_lock(anw, &buf, nullptr) != 0) {
+ return Error() << "Failed to lock window";
+ }
+
+ if (auto ret = copyBuffer(mSavedFrameBuffer, buf); !ret.ok()) {
+ return Error() << "Failed to copy frame: " << ret.error();
+ }
+
+ if (ANativeWindow_unlockAndPost(anw) != 0) {
+ return Error() << "Failed to unlock and post window";
+ }
+ return {};
+ }
+
+ const std::string& name() const { return mName; }
+
private:
// Note: crosvm always uses BGRA8888 or BGRX8888. See devices/src/virtio/gpu/mod.rs in
// crosvm where the SetScanoutBlob command is handled. Let's use BGRA not BGRX with a hope
// that we will need alpha blending for the cursor surface.
static constexpr const int kFormat = HAL_PIXEL_FORMAT_BGRA_8888;
+ std::string mName;
+
std::mutex mSurfaceMutex;
std::unique_ptr<Surface> mNativeSurface;
std::condition_variable mNativeSurfaceReady;
bool mNativeSurfaceNeedsConfiguring = true;
+ // Buffer which crosvm uses when in background. This is just to not fail crosvm even when
+ // Android-side Surface doesn't exist. The content drawn here is never displayed on the physical
+ // screen.
SinkANativeWindow_Buffer mSinkBuffer;
+ // Buffer which is currently allocated for crosvm to draw onto. This holds the last frame. This
+ // is what gets displayed on the physical screen.
+ ANativeWindow_Buffer mLastBuffer;
+
+ // Copy of mLastBuffer made by the call saveFrameForSurface. This holds the last good (i.e.
+ // non-blank) frame before the VM goes background. When the VM is brought up to foreground,
+ // this is drawn to the physical screen until the VM starts to emit actual frames.
+ SinkANativeWindow_Buffer mSavedFrameBuffer;
+
struct Rect {
uint32_t width = 0;
uint32_t height = 0;
@@ -183,35 +277,50 @@
virtual ~DisplayService() = default;
ndk::ScopedAStatus setSurface(Surface* surface, bool forCursor) override {
- if (forCursor) {
- mCursor.setSurface(surface);
- } else {
- mScanout.setSurface(surface);
- }
+ getSurface(forCursor).setNativeSurface(surface);
return ::ndk::ScopedAStatus::ok();
}
ndk::ScopedAStatus removeSurface(bool forCursor) override {
- if (forCursor) {
- mCursor.removeSurface();
- } else {
- mScanout.removeSurface();
- }
+ getSurface(forCursor).removeSurface();
return ::ndk::ScopedAStatus::ok();
}
- AndroidDisplaySurface* getCursorSurface() { return &mCursor; }
- AndroidDisplaySurface* getScanoutSurface() { return &mScanout; }
-
ndk::ScopedFileDescriptor& getCursorStream() { return mCursorStream; }
ndk::ScopedAStatus setCursorStream(const ndk::ScopedFileDescriptor& in_stream) {
mCursorStream = ndk::ScopedFileDescriptor(dup(in_stream.get()));
return ::ndk::ScopedAStatus::ok();
}
+ ndk::ScopedAStatus saveFrameForSurface(bool forCursor) override {
+ if (auto ret = getSurface(forCursor).saveFrame(); !ret.ok()) {
+ std::string msg = std::format("Failed to save frame: {}", ret.error().message());
+ return ::ndk::ScopedAStatus(
+ AStatus_fromServiceSpecificErrorWithMessage(-1, msg.c_str()));
+ }
+ return ::ndk::ScopedAStatus::ok();
+ }
+
+ ndk::ScopedAStatus drawSavedFrameForSurface(bool forCursor) override {
+ if (auto ret = getSurface(forCursor).drawSavedFrame(); !ret.ok()) {
+ std::string msg = std::format("Failed to draw saved frame: {}", ret.error().message());
+ return ::ndk::ScopedAStatus(
+ AStatus_fromServiceSpecificErrorWithMessage(-1, msg.c_str()));
+ }
+ return ::ndk::ScopedAStatus::ok();
+ }
+
+ AndroidDisplaySurface& getSurface(bool forCursor) {
+ if (forCursor) {
+ return mCursor;
+ } else {
+ return mScanout;
+ }
+ }
+
private:
- AndroidDisplaySurface mScanout;
- AndroidDisplaySurface mCursor;
+ AndroidDisplaySurface mScanout{"scanout"};
+ AndroidDisplaySurface mCursor{"cursor"};
ndk::ScopedFileDescriptor mCursorStream;
};
@@ -287,21 +396,18 @@
return nullptr;
}
- AndroidDisplaySurface* displaySurface = forCursor ? ctx->disp_service->getCursorSurface()
- : ctx->disp_service->getScanoutSurface();
- if (displaySurface == nullptr) {
- ctx->errorf("AndroidDisplaySurface was not created");
- return nullptr;
+ AndroidDisplaySurface& surface = ctx->disp_service->getSurface(forCursor);
+ if (auto ret = surface.configure(width, height); !ret.ok()) {
+ ctx->errorf("Failed to configure surface %s: %s", surface.name().c_str(),
+ ret.error().message().c_str());
}
- displaySurface->configure(width, height);
-
- displaySurface->waitForNativeSurface(); // this can block
+ surface.waitForNativeSurface(); // this can block
// TODO(b/332785161): if we know that surface can get destroyed dynamically while VM is running,
// consider calling ANativeWindow_acquire here and _release in destroy_android_surface, so that
// crosvm doesn't hold a dangling pointer.
- return displaySurface;
+ return &surface;
}
extern "C" void destroy_android_surface(struct AndroidDisplayContext*, ANativeWindow*) {
@@ -321,8 +427,10 @@
return false;
}
- if (surface->lock(out_buffer) != 0) {
- ctx->errorf("Failed to lock buffer");
+ auto ret = surface->lock(out_buffer);
+ if (!ret.ok()) {
+ ctx->errorf("Failed to lock surface %s: %s", surface->name().c_str(),
+ ret.error().message().c_str());
return false;
}
@@ -351,8 +459,10 @@
return;
}
- if (surface->unlockAndPost() != 0) {
- ctx->errorf("Failed to unlock and post AndroidDisplaySurface.");
- return;
+ auto ret = surface->unlockAndPost();
+ if (!ret.ok()) {
+ ctx->errorf("Failed to unlock and post for surface %s: %s", surface->name().c_str(),
+ ret.error().message().c_str());
}
+ return;
}