From 6d18c109e8e6c6a695e2b63b651034fb862329f1 Mon Sep 17 00:00:00 2001 From: initramfs Date: Thu, 19 May 2016 20:49:39 +0800 Subject: [PATCH] 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 | 126 ++++++--------------------------------- src/video/videoframe.h | 64 ++++++++++++++++++++ 2 files changed, 81 insertions(+), 109 deletions(-) diff --git a/src/video/videoframe.cpp b/src/video/videoframe.cpp index 74a799e14..be9a0870f 100644 --- a/src/video/videoframe.cpp +++ b/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(); - - // We return nullptr if the VideoFrame is no longer valid - if(frameBuffer.size() == 0) + // Since we are retrieving the AVFrame* directly, we merely need to pass the arguement through + const std::function converter = [](AVFrame* const frame) { - frameLock.unlock(); - return nullptr; - } + return frame; + }; - if(frameSize.width() == 0 && frameSize.height() == 0) - { - frameSize = sourceDimensions.size(); - } + // We need an explicit null pointer holding object to pass to toGenericObject() + AVFrame* nullPointer = nullptr; - AVFrame* ret = retrieveAVFrame(frameSize, pixelFormat, requireAligned); - - 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; + // Returns std::nullptr case of invalid generation + return toGenericObject(frameSize, pixelFormat, requireAligned, converter, nullPointer); } 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(AV_PIX_FMT_RGB24), false); - - if(frame) + // Converter function (constructs QImage out of AVFrame*) + const std::function converter = [&](AVFrame* const 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(); - return ret; - } - - // VideoFrame does not contain an AVFrame to spec, generate one here - frame = generateAVFrame(frameSize, static_cast(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(AV_PIX_FMT_RGB24)); - - QImage ret {*(frame->data), frameSize.width(), frameSize.height(), *(frame->linesize), QImage::Format_RGB888}; - - frameLock.unlock(); - return ret; + // Returns an empty constructed QImage in case of invalid generation + return toGenericObject(frameSize, AV_PIX_FMT_RGB24, false, converter, QImage {}); } 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(AV_PIX_FMT_YUV420P), true); - - if(frame) + // Converter function (constructs ToxAVFrame out of AVFrame*) + const std::function converter = [&](AVFrame* const 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(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(AV_PIX_FMT_YUV420P)); - - ToxAVFrame ret - { - static_cast(frameSize.width()), - static_cast(frameSize.height()), - frame->data[0], frame->data[1], frame->data[2] }; - frameLock.unlock(); - return ret; + return toGenericObject(frameSize, AV_PIX_FMT_YUV420P, true, converter, ToxAVFrame {0, 0, nullptr, nullptr, nullptr}); } AVFrame* VideoFrame::retrieveAVFrame(const QSize& dimensions, const int pixelFormat, const bool requireAligned) diff --git a/src/video/videoframe.h b/src/video/videoframe.h index f638ffc8f..d388cab52 100644 --- a/src/video/videoframe.h +++ b/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 + T toGenericObject(const QSize& dimensions, const int pixelFormat, const bool requireAligned, + const std::function 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(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(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(pixelFormat)); + + T ret = objectConstructor(frame); + + frameLock.unlock(); + return ret; + } + private: // ID const IDType frameID;