cleanup: Remove unused video frame functions.

TokTok · Dec 27, 2024 · 389f8b4 · 389f8b4
1 parent 81570a9
commit 389f8b4
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 57 deletions.
diff --git a/src/video/videoframe.cpp b/src/video/videoframe.cpp
@@ -263,33 +263,6 @@ void VideoFrame::releaseFrame()
     deleteFrameBuffer();
 }
 
-/**
- * @brief Retrieves an AVFrame derived from the source based on the given parameters.
- *
- * If a given frame does not exist, this function will perform appropriate conversions to
- * return a frame that fulfills the given parameters.
- *
- * @param frameSize the dimensions of the frame to get. Defaults to source frame size if frameSize
- * is invalid.
- * @param pixelFormat the desired pixel format of the frame.
- * @param requireAligned true if the returned frame must be frame aligned, false if not.
- * @return a pointer to a AVFrame with the given parameters or nullptr if the VideoFrame is no
- * longer valid.
- */
-std::pair<const AVFrame*, FrameLocker> VideoFrame::getAVFrame(QSize frameSize, const int pixelFormat,
-                                                              const bool requireAligned)
-{
-    if (!frameSize.isValid()) {
-        frameSize = sourceDimensions.size();
-    }
-
-    // Returns nullptr case of invalid generation
-    return toGenericObject(frameSize, pixelFormat, requireAligned, [](AVFrame* const frame) {
-        // Since we are retrieving the AVFrame* directly, we merely need to pass the argument through
-        return frame;
-    });
-}
-
 /**
  * @brief Converts this VideoFrame to a QImage that shares this VideoFrame's buffer.
  *
@@ -308,16 +281,17 @@ QImage VideoFrame::toQImage(QSize frameSize)
     }
 
     // Returns an empty constructed QImage in case of invalid generation
-    auto ob = toGenericObject(frameSize, AV_PIX_FMT_RGB24, false, [frameSize](AVFrame* const frame) {
-        // Converter function (constructs QImage out of AVFrame*)
-        return QImage{
-            *frame->data,     frameSize.width(),     frameSize.height(),
-            *frame->linesize, QImage::Format_RGB888,
-        };
-    });
-
-    // No need to lock the frame buffer since we are returning a QImage which owns the data.
-    return ob.first;
+    auto [frame, frameLocker] =
+        toGenericObject(frameSize, AV_PIX_FMT_RGB24, false, [frameSize](AVFrame* const frame) {
+            // Converter function (constructs QImage out of AVFrame*)
+            return QImage{
+                *frame->data,     frameSize.width(),     frameSize.height(),
+                *frame->linesize, QImage::Format_RGB888,
+            };
+        });
+
+    // No need to return the frame buffer lock since we are returning a QImage which owns the data.
+    return frame;
 }
 
 /**
@@ -331,11 +305,9 @@ QImage VideoFrame::toQImage(QSize frameSize)
  * @return a ToxAVFrame structure that represents this VideoFrame, sharing it's buffers or an
  * empty structure if this VideoFrame is no longer valid.
  */
-std::pair<ToxYUVFrame, FrameLocker> VideoFrame::toToxYUVFrame(QSize frameSize)
+std::pair<ToxYUVFrame, ReadWriteLocker> VideoFrame::toToxYUVFrame()
 {
-    if (!frameSize.isValid()) {
-        frameSize = sourceDimensions.size();
-    }
+    const QSize frameSize = sourceDimensions.size();
 
     return toGenericObject(frameSize, AV_PIX_FMT_YUV420P, true, [frameSize](AVFrame* const frame) {
         // Converter function (constructs ToxAVFrame out of AVFrame*)
@@ -492,8 +464,6 @@ VideoFrame::FrameBufferKey VideoFrame::getFrameKey(const QSize& frameSize, const
  *
  * This function is not thread-safe and must be called from a thread-safe context.
  *
- * Note: this function differs from getAVFrame() in that it returns a nullptr if no frame was
- * found.
  *
  * @param dimensions the dimensions of the frame, must be valid.
  * @param pixelFormat the desired pixel format of the frame.
@@ -699,19 +669,19 @@ void VideoFrame::deleteFrameBuffer()
  * to an object of type T.
  */
 template <typename F>
-std::pair<std::invoke_result_t<F, AVFrame* const>, FrameLocker>
+std::pair<std::invoke_result_t<F, AVFrame* const>, ReadWriteLocker>
 VideoFrame::toGenericObject(const QSize& dimensions, int pixelFormat, bool requireAligned,
                             const F& objectConstructor)
 {
     using ReturnType = std::invoke_result_t<F, AVFrame* const>;
 
     AVFrame* frame;
     {
-        FrameLocker frameReadLock(&frameLock, FrameLocker::ReadLock);
+        ReadWriteLocker frameReadLock(&frameLock, ReadWriteLocker::ReadLock);
 
         // We return empty if the VideoFrame is no longer valid
         if (frameBuffer.empty()) {
-            return {ReturnType{}, FrameLocker()};
+            return {ReturnType{}, ReadWriteLocker()};
         }
 
         frame = retrieveAVFrame(dimensions, pixelFormat, requireAligned);
@@ -731,7 +701,7 @@ VideoFrame::toGenericObject(const QSize& dimensions, int pixelFormat, bool requi
      * process twice, and discard the old result.
      */
     {
-        FrameLocker frameWriteLock(&frameLock, FrameLocker::WriteLock);
+        ReadWriteLocker frameWriteLock(&frameLock, ReadWriteLocker::WriteLock);
         return {objectConstructor(storeAVFrame(frame, dimensions, pixelFormat)),
                 std::move(frameWriteLock)};
     }

diff --git a/src/video/videoframe.h b/src/video/videoframe.h
@@ -25,7 +25,13 @@ extern "C"
 #include <memory>
 #include <unordered_map>
 
-class FrameLocker
+/**
+ * @brief A combination of QReadLocker and QWriteLocker that unlocks the lock when destroyed.
+ *
+ * Unlike QReadLocker and QWriteLocker, objects of this class are movable, so we use this to
+ * extend a critical section to the calling scope.
+ */
+class ReadWriteLocker
 {
 public:
     enum LockType
@@ -34,9 +40,9 @@ class FrameLocker
         ReadLock,
     };
 
-    FrameLocker() = default;
+    ReadWriteLocker() = default;
 
-    explicit FrameLocker(QReadWriteLock* lock, LockType type)
+    explicit ReadWriteLocker(QReadWriteLock* lock, LockType type)
         : lock(lock)
     {
         switch (type) {
@@ -49,20 +55,20 @@ class FrameLocker
         }
     }
 
-    ~FrameLocker()
+    ~ReadWriteLocker()
     {
         if (lock != nullptr) {
             lock->unlock();
         }
     }
 
-    FrameLocker(FrameLocker&& other)
+    ReadWriteLocker(ReadWriteLocker&& other)
         : lock(other.lock)
     {
         other.lock = nullptr;
     }
 
-    FrameLocker& operator=(FrameLocker&& other)
+    ReadWriteLocker& operator=(ReadWriteLocker&& other)
     {
         if (this != &other) {
             lock = other.lock;
@@ -120,10 +126,8 @@ class VideoFrame
 
     void releaseFrame();
 
-    std::pair<const AVFrame*, FrameLocker> getAVFrame(QSize frameSize, const int pixelFormat,
-                                                      const bool requireAligned);
     QImage toQImage(QSize frameSize = {});
-    std::pair<ToxYUVFrame, FrameLocker> toToxYUVFrame(QSize frameSize = {});
+    std::pair<ToxYUVFrame, ReadWriteLocker> toToxYUVFrame();
 
     IDType getFrameID() const;
     IDType getSourceID() const;
@@ -181,7 +185,7 @@ class VideoFrame
     void deleteFrameBuffer();
 
     template <typename F>
-    std::pair<std::invoke_result_t<F, AVFrame* const>, FrameLocker>
+    std::pair<std::invoke_result_t<F, AVFrame* const>, ReadWriteLocker>
     toGenericObject(const QSize& dimensions, int pixelFormat, bool requireAligned,
                     const F& objectConstructor);