diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 659d1ceb..29d083ea 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -1539,7 +1539,7 @@ namespace MobileGL::MG_Remote::Server { static_cast(seq), why.c_str()); return false; } - SI::ImageRef image = SI::Identify(fd, why); + SI::ImageRef image = SI::Identify(fd, op.Width, op.Height, op.Format, why); #if !defined(_WIN32) ::close(fd); #endif diff --git a/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp b/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp index d6010906..71d10739 100644 --- a/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp +++ b/MobileGL/MG_Remote/Server/SharedImageRegistry.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #if defined(__ANDROID__) @@ -43,6 +44,9 @@ namespace MobileGL::MG_Remote::Server::SharedImages { std::atomic nextId{1}; UnorderedMap> byId; std::map, std::weak_ptr> byIdentity; + // Images an ambiguous-identity import has already named, so the next one takes the + // next image the server exported under that identity instead of naming this one twice. + std::set claimedByIdentity; // Set once two live images were seen with one identity: the kernel shares an inode // between dma-bufs, so a descriptor can no longer say which image it is. Bool identityAmbiguous = false; @@ -226,7 +230,8 @@ namespace MobileGL::MG_Remote::Server::SharedImages { if (!reg.identityAmbiguous) { MGLOG_E("MG_Remote server: two live shared images have one dma-buf identity (dev %llu ino %llu); " "this kernel does not keep dma-buf inodes unique, so a returned descriptor cannot name its " - "image and every shared-image import is refused from here on", + "image. Imports degrade to naming it by extent and format over the images exported under " + "that identity, in allocation order (SharedImages::Identify)", static_cast(key.first), static_cast(key.second)); } reg.identityAmbiguous = true; @@ -235,9 +240,9 @@ namespace MobileGL::MG_Remote::Server::SharedImages { return image; } - ImageRef Identify(int fd, std::string& why) { + ImageRef Identify(int fd, Uint32 width, Uint32 height, Uint32 fourcc, std::string& why) { #if defined(_WIN32) - (void)fd; + (void)fd, (void)width, (void)height, (void)fourcc; why = "shared images need POSIX descriptors"; return nullptr; #else @@ -248,9 +253,28 @@ namespace MobileGL::MG_Remote::Server::SharedImages { } Registry& reg = Reg(); const std::lock_guard lock(reg.mutex); + // A KERNEL THAT DOES NOT NAME ITS BUFFERS. Every descriptor of every image reports one + // identity here, so the descriptor cannot say which image it is; the import's own extent + // and format do, over the images exported under that identity, oldest first and each + // claimed once - the order the producer allocated and hands them over in. if (reg.identityAmbiguous) { - why = "dma-buf identities are not unique on this kernel"; - return nullptr; + ImageRef chosen; + for (const auto& [id, weak] : reg.byId) { + const ImageRef image = weak.lock(); + if (image == nullptr || image->IdentityDev != dev || image->IdentityIno != ino) continue; + if (image->Width != width || image->Height != height || image->Fourcc != fourcc) continue; + if (reg.claimedByIdentity.count(image->Id) != 0) continue; + if (chosen == nullptr || image->Id < chosen->Id) chosen = image; + } + if (chosen == nullptr) { + why = Format("no unclaimed live image of this extent and format was exported under the " + "only identity this kernel reports for descriptors (%llux%llu fourcc %llx)", + static_cast(width) | (static_cast(height) << 32), + static_cast(fourcc), 0); + return nullptr; + } + reg.claimedByIdentity.insert(chosen->Id); + return chosen; } const auto it = reg.byIdentity.find(std::make_pair(dev, ino)); ImageRef image = it != reg.byIdentity.end() ? it->second.lock() : nullptr; diff --git a/MobileGL/MG_Remote/Server/SharedImageRegistry.h b/MobileGL/MG_Remote/Server/SharedImageRegistry.h index dfd28c04..21b5e59a 100644 --- a/MobileGL/MG_Remote/Server/SharedImageRegistry.h +++ b/MobileGL/MG_Remote/Server/SharedImageRegistry.h @@ -87,9 +87,19 @@ namespace MobileGL::MG_Remote::Server::SharedImages { // A new image of `width` x `height` `fourcc`, registered. Null with `why` on failure. ImageRef Allocate(Uint32 width, Uint32 height, Uint32 fourcc, std::string& why); - // The live image `fd` was exported from, or null (`why` says which test failed): a descriptor - // this server never exported, or one whose identity the kernel does not keep unique. - ImageRef Identify(int fd, std::string& why); + // The live image the descriptor was exported from, or null (why says which test failed): a + // descriptor this server never exported, or one no live image matches. + // + // WHICH IDENTITY A DESCRIPTOR CARRIES IS THE KERNEL'S TO SAY. (st_dev, st_ino) is the + // honest answer, and the only one on kernels that give every dma-buf its own inode. Some + // do not: there, every descriptor reports one identity (measured on a 4.14 vendor kernel: + // dev 12, ino 10658, st_size 0, and /proc/self/fdinfo carries nothing else either), so the + // descriptor cannot name its image. The import then names it by what it DOES carry - the + // extent and the format it declares - and the images the server exported under that + // identity are taken in allocation order, each claimable once, which is the order the + // producer hands them over. A kernel whose descriptors are ambiguous degrades to that, + // and says so once. + ImageRef Identify(int fd, Uint32 width, Uint32 height, Uint32 fourcc, std::string& why); // The live image with this id, or null. ImageRef Find(Uint64 id);