From b64c2570d7ed4b3c02c9e9eb806167cf5136534d Mon Sep 17 00:00:00 2001
From: philippegorley <gorley.philippe@gmail.com>
Date: Tue, 25 Sep 2018 15:17:14 -0400
Subject: [PATCH] dring: expose frame wrappers in abi

Makes use of AVFrame's reference counting if possible instead of copying
the data.

Only bumps minor version because MediaFrame et al weren't in the ABI
until now.

Change-Id: I692e76230ed057c1ad4e46ab59ea5cfd163fb98d
---
 configure.ac                             |  2 +-
 doc/doxygen/core-doc.cfg.in              |  2 +-
 src/client/videomanager.cpp              | 27 +++----
 src/dring/videomanager_interface.h       | 21 ++++--
 src/media/video/video_mixer.cpp          |  2 +-
 test/unitTest/Makefile.am                |  6 ++
 test/unitTest/media/test_media_frame.cpp | 89 ++++++++++++++++++++++++
 7 files changed, 127 insertions(+), 22 deletions(-)
 create mode 100644 test/unitTest/media/test_media_frame.cpp

diff --git a/configure.ac b/configure.ac
index a53a77dc1a..7a76c52b7b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2,7 +2,7 @@ dnl Ring - configure.ac for automake 1.9 and autoconf 2.59
 
 dnl Process this file with autoconf to produce a configure script.
 AC_PREREQ([2.65])
-AC_INIT([Ring Daemon],[6.0.0],[ring@gnu.org],[ring])
+AC_INIT([Ring Daemon],[6.1.0],[ring@gnu.org],[ring])
 
 AC_COPYRIGHT([[Copyright (c) Savoir-faire Linux 2004-2018]])
 AC_REVISION([$Revision$])
diff --git a/doc/doxygen/core-doc.cfg.in b/doc/doxygen/core-doc.cfg.in
index abff1802e7..f8122b134d 100644
--- a/doc/doxygen/core-doc.cfg.in
+++ b/doc/doxygen/core-doc.cfg.in
@@ -31,7 +31,7 @@ PROJECT_NAME           = "Ring Daemon"
 # This could be handy for archiving the generated documentation or
 # if some version control system is used.
 
-PROJECT_NUMBER         = 6.0.0
+PROJECT_NUMBER         = 6.1.0
 
 # Using the PROJECT_BRIEF tag one can provide an optional one line description
 # for a project that appears at the top of each page and should give viewer
diff --git a/src/client/videomanager.cpp b/src/client/videomanager.cpp
index f6ae232865..2c33fd6b1a 100644
--- a/src/client/videomanager.cpp
+++ b/src/client/videomanager.cpp
@@ -52,6 +52,13 @@ MediaFrame::MediaFrame()
         throw std::bad_alloc();
 }
 
+void
+MediaFrame::copyFrom(MediaFrame& o)
+{
+    reset();
+    av_frame_ref(frame_.get(), o.frame_.get());
+}
+
 void
 MediaFrame::reset() noexcept
 {
@@ -72,6 +79,14 @@ VideoFrame::reset() noexcept
     releaseBufferCb_ = {};
 }
 
+void
+VideoFrame::copyFrom(VideoFrame& o)
+{
+    MediaFrame::copyFrom(o);
+    ptr_ = o.ptr_;
+    allocated_ = o.allocated_;
+}
+
 size_t
 VideoFrame::size() const noexcept
 {
@@ -154,7 +169,6 @@ VideoFrame::setReleaseCb(std::function<void(uint8_t*)> cb) noexcept
     }
 }
 
-
 void
 VideoFrame::noise()
 {
@@ -166,17 +180,6 @@ VideoFrame::noise()
     }
 }
 
-VideoFrame&
-VideoFrame::operator =(const VideoFrame& src)
-{
-    reserve(src.format(), src.width(), src.height());
-    auto source = src.pointer();
-    av_image_copy(frame_->data, frame_->linesize, (const uint8_t **)source->data,
-                  source->linesize, (AVPixelFormat)frame_->format,
-                  frame_->width, frame_->height);
-    return *this;
-}
-
 VideoFrame* getNewFrame()
 {
     if (auto input = ring::Manager::instance().getVideoManager().videoInput.lock())
diff --git a/src/dring/videomanager_interface.h b/src/dring/videomanager_interface.h
index 95160e4a79..0993072f3b 100644
--- a/src/dring/videomanager_interface.h
+++ b/src/dring/videomanager_interface.h
@@ -64,16 +64,23 @@ struct DRING_PUBLIC SinkTarget {
     std::function<void(FrameBufferPtr)> push;
 };
 
-class MediaFrame {
+class DRING_PUBLIC MediaFrame {
 public:
     // Construct an empty MediaFrame
     MediaFrame();
+    MediaFrame(const MediaFrame&) = delete;
+    MediaFrame& operator =(const MediaFrame& o) = delete;
+    MediaFrame(MediaFrame&& o) = delete;
+    MediaFrame& operator =(MediaFrame&& o) = delete;
 
     virtual ~MediaFrame() = default;
 
     // Return a pointer on underlaying buffer
     AVFrame* pointer() const noexcept { return frame_.get(); }
 
+    // Fill this MediaFrame with data from o
+    void copyFrom(MediaFrame& o);
+
     // Reset internal buffers (return to an empty MediaFrame)
     virtual void reset() noexcept;
 
@@ -81,17 +88,20 @@ protected:
     std::unique_ptr<AVFrame, void(*)(AVFrame*)> frame_;
 };
 
-struct AudioFrame: MediaFrame {};
+class DRING_PUBLIC AudioFrame : public MediaFrame {};
 
-class VideoFrame: public MediaFrame {
+class DRING_PUBLIC VideoFrame : public MediaFrame {
 public:
     // Construct an empty VideoFrame
-    VideoFrame() = default;
+    VideoFrame() : MediaFrame() {}
     ~VideoFrame();
 
     // Reset internal buffers (return to an empty VideoFrame)
     void reset() noexcept override;
 
+    // Fill this VideoFrame with data from o
+    void copyFrom(VideoFrame& o);
+
     // Return frame size in bytes
     std::size_t size() const noexcept;
 
@@ -115,9 +125,6 @@ public:
 
     void noise();
 
-    // Copy-Assignement
-    VideoFrame& operator =(const VideoFrame& src);
-
 private:
     std::function<void(uint8_t*)> releaseBufferCb_ {};
     uint8_t* ptr_ {nullptr};
diff --git a/src/media/video/video_mixer.cpp b/src/media/video/video_mixer.cpp
index f864d9c849..6757bf4c28 100644
--- a/src/media/video/video_mixer.cpp
+++ b/src/media/video/video_mixer.cpp
@@ -111,7 +111,7 @@ VideoMixer::update(Observable<std::shared_ptr<VideoFrame>>* ob,
                 x->update_frame.reset(new VideoFrame);
             else
                 x->update_frame->reset();
-            *x->update_frame = *frame_p; // copy frame content, it will be destroyed after return
+            x->update_frame->copyFrom(*frame_p); // copy frame content, it will be destroyed after return
             x->atomic_swap_render(x->update_frame);
             return;
         }
diff --git a/test/unitTest/Makefile.am b/test/unitTest/Makefile.am
index 7275096abe..a17a541633 100644
--- a/test/unitTest/Makefile.am
+++ b/test/unitTest/Makefile.am
@@ -92,4 +92,10 @@ ut_media_filter_SOURCES = media/test_media_filter.cpp
 check_PROGRAMS += ut_resampler
 ut_resampler_SOURCES = media/audio/test_resampler.cpp
 
+#
+# media_frame
+#
+check_PROGRAMS += ut_media_frame
+ut_media_frame_SOURCES = media/test_media_frame.cpp
+
 TESTS = $(check_PROGRAMS)
diff --git a/test/unitTest/media/test_media_frame.cpp b/test/unitTest/media/test_media_frame.cpp
new file mode 100644
index 0000000000..3a3c2a1948
--- /dev/null
+++ b/test/unitTest/media/test_media_frame.cpp
@@ -0,0 +1,89 @@
+/*
+ *  Copyright (C) 2018 Savoir-faire Linux Inc.
+ *
+ *  Author: Philippe Gorley <philippe.gorley@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
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA.
+ */
+
+#include <cppunit/TestAssert.h>
+#include <cppunit/TestFixture.h>
+#include <cppunit/extensions/HelperMacros.h>
+
+extern "C" {
+#include <libavutil/frame.h>
+#include <libavutil/pixfmt.h>
+}
+
+#include "dring.h"
+#include "videomanager_interface.h"
+
+#include "../../test_runner.h"
+
+namespace ring { namespace test {
+
+class MediaFrameTest : public CppUnit::TestFixture {
+public:
+    static std::string name() { return "media_frame"; }
+
+    void setUp();
+    void tearDown();
+
+private:
+    void testCopy();
+
+    CPPUNIT_TEST_SUITE(MediaFrameTest);
+    CPPUNIT_TEST(testCopy);
+    CPPUNIT_TEST_SUITE_END();
+};
+
+
+CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(MediaFrameTest, MediaFrameTest::name());
+
+void
+MediaFrameTest::setUp()
+{
+    DRing::init(DRing::InitFlag(DRing::DRING_FLAG_DEBUG | DRing::DRING_FLAG_CONSOLE_LOG));
+}
+
+void
+MediaFrameTest::tearDown()
+{
+    DRing::fini();
+}
+
+void
+MediaFrameTest::testCopy()
+{
+    // test allocation
+    DRing::VideoFrame v1;
+    v1.reserve(AV_PIX_FMT_YUV420P, 100, 100);
+    v1.pointer()->data[0][0] = 42;
+    CPPUNIT_ASSERT(v1.pointer());
+
+    // test frame referencing (different pointers, but same data)
+    DRing::VideoFrame v2;
+    v2.copyFrom(v1);
+    CPPUNIT_ASSERT(v1.format() == v2.format());
+    CPPUNIT_ASSERT(v1.width() == v2.width());
+    CPPUNIT_ASSERT(v1.height() == v2.height());
+    CPPUNIT_ASSERT(v1.pointer() != v2.pointer());
+    CPPUNIT_ASSERT(v1.pointer()->data[0][0] == 42);
+    CPPUNIT_ASSERT(v2.pointer()->data[0][0] == 42);
+}
+
+}} // namespace ring::test
+
+RING_TEST_RUNNER(ring::test::MediaFrameTest::name());
-- 
GitLab