video: fix concurrent access in sink direct rendering

This patchset prevents client from reading a frame that is being written
by the daemon using a push/pull mechanism.

**WARNING** This breaks the direct rendering API

Issue: #81542
Change-Id: I55d4ee9b986ac682903b326fd86c32d8f5f68825
Signed-off-by: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>

src/client/videomanager.cpp

@@ -138,30 +138,15 @@ hasCameraStarted()
     return videoManager.started;
 }
 
-template <class T>
-static void
-registerSinkTarget_(const std::string& sinkId, std::vector<unsigned char>& frameBuffer, T&& cb)
+void
+registerSinkTarget(const std::string& sinkId, const SinkTarget& target)
 {
    if (auto sink = ring::Manager::instance().getSinkClient(sinkId))
-        sink->registerTarget(std::forward<T>(cb), frameBuffer);
+       sink->registerTarget(target);
    else
        RING_WARN("No sink found for id '%s'", sinkId.c_str());
 }
 
-void
-registerSinkTarget(const std::string& sinkId, std::vector<unsigned char>& frameBuffer,
-                   const std::function<void(int, int)>& cb)
-{
-    registerSinkTarget_(sinkId, frameBuffer, cb);
-}
-
-void
-registerSinkTarget(const std::string& sinkId, std::vector<unsigned char>& frameBuffer,
-                   std::function<void(int, int)>&& cb)
-{
-    registerSinkTarget_(sinkId, frameBuffer, cb);
-}
-
 #ifdef __ANDROID__
 void
 addVideoDevice(const std::string &node)

src/dring/videomanager_interface.h

 /*
  *  Copyright (C) 2012-2015 Savoir-faire Linux Inc.
  *
+ *  Author: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
+ *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
  *  the Free Software Foundation; either version 3 of the License, or
@@ -24,11 +26,29 @@
 #include <map>
 #include <string>
 #include <functional>
+#include <cstdint>
+#include <cstdlib>
 
 #include "dring.h"
 
 namespace DRing {
 
+/* FrameBuffer is a generic video frame container */
+struct FrameBuffer {
+    uint8_t* ptr {nullptr};     // data as a plain raw pointer
+    std::size_t ptrSize {0};      // size in byte of ptr array
+    int format {0};             // as listed by AVPixelFormat (avutils/pixfmt.h)
+    int width {0};              // frame width
+    int height {0};             // frame height
+    std::vector<uint8_t> storage;
+};
+
+struct SinkTarget {
+    using FrameBufferPtr = std::unique_ptr<FrameBuffer>;
+    std::function<FrameBufferPtr(std::size_t bytes)> pull;
+    std::function<void(FrameBufferPtr)> push;
+};
+
 using VideoCapabilities = std::map<std::string, std::map<std::string, std::vector<std::string>>>;
 
 void registerVideoHandlers(const std::map<std::string, std::shared_ptr<CallbackWrapperBase>>&);
@@ -48,8 +68,8 @@ void stopCamera();
 bool hasCameraStarted();
 bool switchInput(const std::string& resource);
 bool switchToCamera();
-void registerSinkTarget(const std::string& sinkId, std::vector<unsigned char>& frameBuffer, const std::function<void(int, int)>& cb);
-void registerSinkTarget(const std::string& sinkId, std::vector<unsigned char>& frameBuffer, std::function<void(int, int)>&& cb);
+void registerSinkTarget(const std::string& sinkId, const SinkTarget& target);
+
 #ifdef __ANDROID__
 void addVideoDevice(const std::string &node);
 void removeVideoDevice(const std::string &node);

src/media/media_buffer.cpp

@@ -20,6 +20,7 @@
 
 #include "libav_deps.h" // MUST BE INCLUDED FIRST
 #include "media_buffer.h"
+#include "dring/videomanager_interface.h"
 
 #include <new>
 #include <cstdlib>
@@ -107,7 +108,7 @@ VideoFrame::reserve(int format, int width, int height)
 }
 
 void
-VideoFrame::setFromMemory(void* ptr, int format, int width, int height) noexcept
+VideoFrame::setFromMemory(uint8_t* ptr, int format, int width, int height)
 {
     reset();
     setGeometry(format, width, height);

src/media/media_buffer.h

@@ -26,6 +26,10 @@
 
 class AVFrame;
 
+namespace DRing {
+class FrameBuffer; //  from dring/videomanager_interface.h
+}
+
 namespace ring {
 
 class MediaFrame {
@@ -74,7 +78,7 @@ class VideoFrame: public MediaFrame {
 
         // Set internal pixel buffers on given memory buffer
         // This buffer must follow given specifications.
-        void setFromMemory(void* ptr, int format, int width, int height) noexcept;
+        void setFromMemory(uint8_t* data, int format, int width, int height);
 
         void noise();
 

src/media/video/shm_header.h

@@ -21,6 +21,7 @@
 #ifndef SHM_HEADER_H_
 #define SHM_HEADER_H_
 
+#include <cstdint>
 #include <semaphore.h>
 
 // Implementation note: double-buffering
@@ -37,7 +38,7 @@ struct SHMHeader {
     unsigned mapSize;           // size to map if you need to see all data
     unsigned readOffset;        // offset of readable frame in data
     unsigned writeOffset;       // offset of writable frame in data
-    char data[];                // the whole shared memory
+    uint8_t data[];             // the whole shared memory
 };
 
 #endif

src/media/video/sinkclient.cpp

@@ -3,6 +3,7 @@
  *
  *  Author: Tristan Matthews <tristan.matthews@savoirfairelinux.com>
  *  Author: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
+ *  Author: Alexandre Lision <alexandre.lision@savoirfairelinux.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -35,6 +36,7 @@
 #include "noncopyable.h"
 #include "client/ring_signal.h"
 #include "dring/videomanager_interface.h"
+#include "libav_utils.h"
 
 #ifndef _WIN32
 #include <sys/mman.h>
@@ -323,7 +325,7 @@ SinkClient::update(Observable<std::shared_ptr<VideoFrame>>* /*obs*/,
     shm_->renderFrame(*f.get());
 #endif
 
-    if (targetData_) {
+    if (target_.pull) {
         VideoFrame dst;
         VideoScaler scaler;
         const int width = f->width();
@@ -336,12 +338,14 @@ SinkClient::update(Observable<std::shared_ptr<VideoFrame>>* /*obs*/,
         const auto bytes = videoFrameSize(format, width, height);
 
         if (bytes > 0) {
-          targetData_->resize(bytes);
-          auto data = targetData_->data();
-
-          dst.setFromMemory(data, format, width, height);
+            if (auto buffer_ptr = target_.pull(bytes)) {
+                buffer_ptr->format = libav_utils::libav_pixel_format(format);
+                buffer_ptr->width = width;
+                buffer_ptr->height = height;
+                dst.setFromMemory(buffer_ptr->ptr, format, width, height);
                 scaler.scale(*f, dst);
-          target_(width, height);
+                target_.push(std::move(buffer_ptr));
+            }
         }
     }
 }

src/media/video/sinkclient.h

@@ -2,6 +2,8 @@
  *  Copyright (C) 2012-2015 Savoir-faire Linux Inc.
  *
  *  Author: Tristan Matthews <tristan.matthews@savoirfairelinux.com>
+ *  Author: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
+ *  Author: Alexandre Lision <alexandre.lision@savoirfairelinux.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -25,6 +27,7 @@
 #endif
 
 #include "video_base.h"
+#include <videomanager_interface.h>
 
 #include <string>
 #include <vector>
@@ -56,18 +59,15 @@ class SinkClient : public VideoFramePassiveReader
 
         void setFrameSize(int width, int height);
 
-        template <class T>
-        void registerTarget(T&& cb, std::vector<unsigned char>& frameBuffer) noexcept {
-            target_ = std::forward<T>(cb);
-            targetData_ = &frameBuffer;
+        void registerTarget(const DRing::SinkTarget& target) noexcept {
+            target_ = target;
         }
 
     private:
         const std::string id_;
         const bool mixer_;
         bool started_ {false}; // used to arbitrate client's stop signal.
-        std::function<void(int, int)> target_;
-        std::vector<unsigned char>* targetData_ {nullptr}; // filled by registerTarget, user owned
+        DRing::SinkTarget target_;
 
 #ifdef DEBUG_FPS
         unsigned frameCount_;