refactor(video): use generics to simply VideoFrame conversion functions

This commit replaces the contents of toQImage(), toToxAVFrame() and
getAVFrame() with speciailized calls to a generic toGenericObject()
function for better code clarity.

src/video/videoframe.cpp

@@ -155,113 +155,45 @@ void VideoFrame::releaseFrame()
 
 const AVFrame* VideoFrame::getAVFrame(QSize frameSize, const int pixelFormat, const bool requireAligned)
 {
-    frameLock.lockForRead();
+    // Since we are retrieving the AVFrame* directly, we merely need to pass the arguement through
-
+    const std::function<AVFrame*(AVFrame* const)> converter = [](AVFrame* const frame)
-    // We return nullptr if the VideoFrame is no longer valid
-    if(frameBuffer.size() == 0)
     {
-        frameLock.unlock();
+        return frame;
-        return nullptr;
+    };
-    }
 
-    if(frameSize.width() == 0 && frameSize.height() == 0)
+    // We need an explicit null pointer holding object to pass to toGenericObject()
-    {
+    AVFrame* nullPointer = nullptr;
-        frameSize = sourceDimensions.size();
-    }
 
-    AVFrame* ret = retrieveAVFrame(frameSize, pixelFormat, requireAligned);
+    // Returns std::nullptr case of invalid generation
-
+    return toGenericObject(frameSize, pixelFormat, requireAligned, converter, nullPointer);
-    if(ret)
-    {
-        frameLock.unlock();
-        return ret;
-    }
-
-    // VideoFrame does not contain an AVFrame to spec, generate one here
-    ret = generateAVFrame(frameSize, pixelFormat, requireAligned);
-
-    /*
-     * We need to "upgrade" the lock to a write lock so we can update our frameBuffer map.
-     *
-     * It doesn't matter if another thread obtains the write lock before we finish since it is
-     * likely writing to somewhere else. Worst-case scenario, we merely perform the generation
-     * process twice, and discard the old result.
-     */
-    frameLock.unlock();
-    frameLock.lockForWrite();
-
-    storeAVFrame(ret, frameSize, pixelFormat);
-
-    frameLock.unlock();
-    return ret;
 }
 
 QImage VideoFrame::toQImage(QSize frameSize)
 {
-    frameLock.lockForRead();
-
-    // We return an empty QImage if the VideoFrame is no longer valid
-    if(frameBuffer.size() == 0)
-    {
-        frameLock.unlock();
-        return QImage {};
-    }
-
     if(frameSize.width() == 0 && frameSize.height() == 0)
     {
         frameSize = sourceDimensions.size();
     }
 
-    AVFrame* frame = retrieveAVFrame(frameSize, static_cast<int>(AV_PIX_FMT_RGB24), false);
+    // Converter function (constructs QImage out of AVFrame*)
-
+    const std::function<QImage(AVFrame* const)> converter = [&](AVFrame* const frame)
-    if(frame)
     {
-        QImage ret {*(frame->data), frameSize.width(), frameSize.height(), *(frame->linesize), QImage::Format_RGB888};
+        return QImage {*(frame->data), frameSize.width(), frameSize.height(), *(frame->linesize), QImage::Format_RGB888};
+    };
 
-        frameLock.unlock();
+    // Returns an empty constructed QImage in case of invalid generation
-        return ret;
+    return toGenericObject(frameSize, AV_PIX_FMT_RGB24, false, converter, QImage {});
-    }
-
-    // VideoFrame does not contain an AVFrame to spec, generate one here
-    frame = generateAVFrame(frameSize, static_cast<int>(AV_PIX_FMT_RGB24), false);
-
-    /*
-     * We need to "upgrade" the lock to a write lock so we can update our frameBuffer map.
-     *
-     * It doesn't matter if another thread obtains the write lock before we finish since it is
-     * likely writing to somewhere else. Worst-case scenario, we merely perform the generation
-     * process twice, and discard the old result.
-     */
-    frameLock.unlock();
-    frameLock.lockForWrite();
-
-    storeAVFrame(frame, frameSize, static_cast<int>(AV_PIX_FMT_RGB24));
-
-    QImage ret {*(frame->data), frameSize.width(), frameSize.height(), *(frame->linesize), QImage::Format_RGB888};
-
-    frameLock.unlock();
-    return ret;
 }
 
 ToxAVFrame VideoFrame::toToxAVFrame(QSize frameSize)
 {
-    frameLock.lockForRead();
-
-    // We return nullptr if the VideoFrame is no longer valid
-    if(frameBuffer.size() == 0)
-    {
-        frameLock.unlock();
-        return {0, 0, nullptr, nullptr, nullptr};
-    }
-
     if(frameSize.width() == 0 && frameSize.height() == 0)
     {
         frameSize = sourceDimensions.size();
     }
 
-    AVFrame* frame = retrieveAVFrame(frameSize, static_cast<int>(AV_PIX_FMT_YUV420P), true);
+    // Converter function (constructs ToxAVFrame out of AVFrame*)
-
+    const std::function<ToxAVFrame(AVFrame* const)> converter = [&](AVFrame* const frame)
-    if(frame)
     {
         ToxAVFrame ret
         {
@@ -270,34 +202,10 @@ ToxAVFrame VideoFrame::toToxAVFrame(QSize frameSize)
             frame->data[0], frame->data[1], frame->data[2]
         };
 
-        frameLock.unlock();
         return ret;
-    }
-
-    // VideoFrame does not contain an AVFrame to spec, generate one here
-    frame = generateAVFrame(frameSize, static_cast<int>(AV_PIX_FMT_YUV420P), true);
-
-    /*
-     * We need to "upgrade" the lock to a write lock so we can update our frameBuffer map.
-     *
-     * It doesn't matter if another thread obtains the write lock before we finish since it is
-     * likely writing to somewhere else. Worst-case scenario, we merely perform the generation
-     * process twice, and discard the old result.
-     */
-    frameLock.unlock();
-    frameLock.lockForWrite();
-
-    storeAVFrame(frame, frameSize, static_cast<int>(AV_PIX_FMT_YUV420P));
-
-    ToxAVFrame ret
-    {
-        static_cast<std::uint16_t>(frameSize.width()),
-        static_cast<std::uint16_t>(frameSize.height()),
-        frame->data[0], frame->data[1], frame->data[2]
     };
 
-    frameLock.unlock();
+    return toGenericObject(frameSize, AV_PIX_FMT_YUV420P, true, converter, ToxAVFrame {0, 0, nullptr, nullptr, nullptr});
-    return ret;
 }
 
 AVFrame* VideoFrame::retrieveAVFrame(const QSize& dimensions, const int pixelFormat, const bool requireAligned)

src/video/videoframe.h

@@ -387,6 +387,70 @@ private:
      */
     void deleteFrameBuffer();
 
+    /**
+     * @brief Converts this VideoFrame to a generic type T based on the given parameters and
+     * supplied converter functions.
+     *
+     * This function is used internally to create various toXObject functions that all follow the
+     * same generation pattern (where XObject is some existing type like QImage).
+     *
+     * In order to create such a type, a object constructor function is required that takes the
+     * generated AVFrame object and creates type T out of it. This function additionally requires
+     * a null object of type T that represents an invalid/null object for when the generation
+     * process fails (e.g. when the VideoFrame is no longer valid).
+     *
+     * @param dimensions the dimensions of the frame.
+     * @param pixelFormat the pixel format of the frame.
+     * @param requireAligned true if the generated frame needs to be frame aligned, false otherwise.
+     * @param objectConstructor a std::function that takes the generated AVFrame and converts it
+     * to an object of type T.
+     * @param nullObject an object of type T that represents the null/invalid object to be used
+     * when the generation process fails.
+     */
+    template <typename T>
+    T toGenericObject(const QSize& dimensions, const int pixelFormat, const bool requireAligned,
+                      const std::function<T(AVFrame* const)> objectConstructor, const T& nullObject)
+    {
+        frameLock.lockForRead();
+
+        // We return nullObject if the VideoFrame is no longer valid
+        if(frameBuffer.size() == 0)
+        {
+            frameLock.unlock();
+            return nullObject;
+        }
+
+        AVFrame* frame = retrieveAVFrame(dimensions, static_cast<int>(pixelFormat), requireAligned);
+
+        if(frame)
+        {
+            T ret = objectConstructor(frame);
+
+            frameLock.unlock();
+            return ret;
+        }
+
+        // VideoFrame does not contain an AVFrame to spec, generate one here
+        frame = generateAVFrame(dimensions, static_cast<int>(pixelFormat), requireAligned);
+
+        /*
+         * We need to "upgrade" the lock to a write lock so we can update our frameBuffer map.
+         *
+         * It doesn't matter if another thread obtains the write lock before we finish since it is
+         * likely writing to somewhere else. Worst-case scenario, we merely perform the generation
+         * process twice, and discard the old result.
+         */
+        frameLock.unlock();
+        frameLock.lockForWrite();
+
+        storeAVFrame(frame, dimensions, static_cast<int>(pixelFormat));
+
+        T ret = objectConstructor(frame);
+
+        frameLock.unlock();
+        return ret;
+    }
+
 private:
     // ID
     const IDType frameID;