From 904495d2bf2b85da4c2f381778a0bb09b4ce1bbf Mon Sep 17 00:00:00 2001 From: initramfs Date: Mon, 25 Apr 2016 10:39:57 -0400 Subject: [PATCH] fix(video): fix slanted video when video size is not divisible by 8 --- src/video/corevideosource.cpp | 2 +- src/video/videoframe.cpp | 129 ++++++++++++++++++++----------- src/video/videoframe.h | 141 ++++++++++++++++++++++++++++++---- 3 files changed, 213 insertions(+), 59 deletions(-) diff --git a/src/video/corevideosource.cpp b/src/video/corevideosource.cpp index d22ed17fd..329efdc11 100644 --- a/src/video/corevideosource.cpp +++ b/src/video/corevideosource.cpp @@ -77,7 +77,7 @@ void CoreVideoSource::pushFrame(const vpx_image_t* vpxframe) int bufSize = av_image_alloc(avframe->data, avframe->linesize, width, height, - static_cast(AV_PIX_FMT_YUV420P), VideoFrame::frameAlignment); + static_cast(AV_PIX_FMT_YUV420P), VideoFrame::dataAlignment); if(bufSize < 0){ av_frame_free(&avframe); diff --git a/src/video/videoframe.cpp b/src/video/videoframe.cpp index 25c0ebce2..39961969f 100644 --- a/src/video/videoframe.cpp +++ b/src/video/videoframe.cpp @@ -27,10 +27,11 @@ extern "C"{ VideoFrame::VideoFrame(AVFrame* sourceFrame, QRect dimensions, int pixFmt, std::function destructCallback, bool freeSourceFrame) : sourceDimensions(dimensions), sourcePixelFormat(pixFmt), + sourceFrameKey(getFrameKey(dimensions.size(), pixFmt, sourceFrame->linesize[0])), freeSourceFrame(freeSourceFrame), destructCallback(destructCallback) { - frameBuffer[pixFmt][dimensionsToKey(dimensions.size())] = sourceFrame; + frameBuffer[sourceFrameKey] = sourceFrame; } VideoFrame::VideoFrame(AVFrame* sourceFrame, std::function destructCallback, bool freeSourceFrame) @@ -62,7 +63,7 @@ bool VideoFrame::isValid() return retValue; } -const AVFrame* VideoFrame::getAVFrame(const QSize& dimensions, const int pixelFormat) +const AVFrame* VideoFrame::getAVFrame(QSize frameSize, const int pixelFormat, const bool requireAligned) { frameLock.lockForRead(); @@ -73,7 +74,12 @@ const AVFrame* VideoFrame::getAVFrame(const QSize& dimensions, const int pixelFo return nullptr; } - AVFrame* ret = retrieveAVFrame(dimensions, pixelFormat); + if(frameSize.width() == 0 && frameSize.height() == 0) + { + frameSize = sourceDimensions.size(); + } + + AVFrame* ret = retrieveAVFrame(frameSize, pixelFormat, requireAligned); if(ret) { @@ -82,19 +88,19 @@ const AVFrame* VideoFrame::getAVFrame(const QSize& dimensions, const int pixelFo } // VideoFrame does not contain an AVFrame to spec, generate one here - ret = generateAVFrame(dimensions, pixelFormat); + 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. Absolute worst-case scenario, we merely perform the - * generation process twice, and discard the old result. + * 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, dimensions, pixelFormat); + storeAVFrame(ret, frameSize, pixelFormat); frameLock.unlock(); return ret; @@ -125,7 +131,7 @@ QImage VideoFrame::toQImage(QSize frameSize) frameSize = sourceDimensions.size(); } - AVFrame* frame = retrieveAVFrame(frameSize, static_cast(AV_PIX_FMT_RGB24)); + AVFrame* frame = retrieveAVFrame(frameSize, static_cast(AV_PIX_FMT_RGB24), false); if(frame) { @@ -136,14 +142,14 @@ QImage VideoFrame::toQImage(QSize frameSize) } // VideoFrame does not contain an AVFrame to spec, generate one here - frame = generateAVFrame(frameSize, static_cast(AV_PIX_FMT_RGB24)); + 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. Absolute worst-case scenario, we merely perform the - * generation process twice, and discard the old result. + * likely writing to somewhere else. Worst-case scenario, we merely perform the generation + * process twice, and discard the old result. */ frameLock.unlock(); frameLock.lockForWrite(); @@ -172,7 +178,7 @@ vpx_image* VideoFrame::toVpxImage(QSize frameSize) frameSize = sourceDimensions.size(); } - AVFrame* frame = retrieveAVFrame(frameSize, static_cast(AV_PIX_FMT_YUV420P)); + AVFrame* frame = retrieveAVFrame(frameSize, static_cast(AV_PIX_FMT_YUV420P), true); if(frame) { @@ -196,14 +202,14 @@ vpx_image* VideoFrame::toVpxImage(QSize frameSize) } // VideoFrame does not contain an AVFrame to spec, generate one here - frame = generateAVFrame(frameSize, static_cast(AV_PIX_FMT_YUV420P)); + 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. Absolute worst-case scenario, we merely perform the - * generation process twice, and discard the old result. + * likely writing to somewhere else. Worst-case scenario, we merely perform the generation + * process twice, and discard the old result. */ frameLock.unlock(); frameLock.lockForWrite(); @@ -229,11 +235,27 @@ vpx_image* VideoFrame::toVpxImage(QSize frameSize) return ret; } -AVFrame* VideoFrame::retrieveAVFrame(const QSize& dimensions, const int pixelFormat) +AVFrame* VideoFrame::retrieveAVFrame(const QSize& dimensions, const int pixelFormat, const bool requireAligned) { - if(frameBuffer.contains(pixelFormat) && frameBuffer[pixelFormat].contains(dimensionsToKey(dimensions))) + if(!requireAligned) { - return frameBuffer[pixelFormat][dimensionsToKey(dimensions)]; + /* + * We attempt to obtain a unaligned frame first because an unaligned linesize corresponds + * to a data aligned frame. + */ + FrameBufferKey frameKey = getFrameKey(dimensions, pixelFormat, false); + + if(frameBuffer.count(frameKey) > 0) + { + return frameBuffer[frameKey]; + } + } + + FrameBufferKey frameKey = getFrameKey(dimensions, pixelFormat, true); + + if(frameBuffer.count(frameKey) > 0) + { + return frameBuffer[frameKey]; } else { @@ -241,7 +263,7 @@ AVFrame* VideoFrame::retrieveAVFrame(const QSize& dimensions, const int pixelFor } } -AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFormat) +AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFormat, const bool requireAligned) { AVFrame* ret = av_frame_alloc(); @@ -254,9 +276,25 @@ AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFor ret->height = dimensions.height(); ret->format = pixelFormat; - int bufSize = av_image_alloc(ret->data, ret->linesize, + /* + * We generate a frame under data alignment only if the dimensions allow us to be frame aligned + * or if the caller doesn't require frame alignment + */ + + int bufSize; + + if(!requireAligned || (dimensions.width() % 8 == 0 && dimensions.height() % 8 == 0)) + { + bufSize = av_image_alloc(ret->data, ret->linesize, dimensions.width(), dimensions.height(), - static_cast(pixelFormat), frameAlignment); + static_cast(pixelFormat), dataAlignment); + } + else + { + bufSize = av_image_alloc(ret->data, ret->linesize, + dimensions.width(), dimensions.height(), + static_cast(pixelFormat), 1); + } if(bufSize < 0){ av_frame_free(&ret); @@ -279,7 +317,7 @@ AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFor return nullptr; } - AVFrame* source = frameBuffer[sourcePixelFormat][dimensionsToKey(sourceDimensions.size())]; + AVFrame* source = frameBuffer[sourceFrameKey]; sws_scale(swsCtx, source->data, source->linesize, 0, sourceDimensions.height(), ret->data, ret->linesize); sws_freeContext(swsCtx); @@ -289,10 +327,12 @@ AVFrame* VideoFrame::generateAVFrame(const QSize& dimensions, const int pixelFor void VideoFrame::storeAVFrame(AVFrame* frame, const QSize& dimensions, const int pixelFormat) { + FrameBufferKey frameKey = getFrameKey(dimensions, pixelFormat, frame->linesize[0]); + // We check the prescence of the frame in case of double-computation - if(frameBuffer.contains(pixelFormat) && frameBuffer[pixelFormat].contains(dimensionsToKey(dimensions))) + if(frameBuffer.count(frameKey) > 0) { - AVFrame* old_ret = frameBuffer[pixelFormat][dimensionsToKey(dimensions)]; + AVFrame* old_ret = frameBuffer[frameKey]; // Free old frame av_freep(&old_ret->data[0]); @@ -300,39 +340,38 @@ void VideoFrame::storeAVFrame(AVFrame* frame, const QSize& dimensions, const int av_frame_free(&old_ret); } - frameBuffer[pixelFormat].insert(dimensionsToKey(dimensions), frame); + frameBuffer[frameKey] = frame; } void VideoFrame::deleteFrameBuffer() { - const quint64 sourceKey = dimensionsToKey(sourceDimensions.size()); - - for(auto pixFmtIter = frameBuffer.begin(); pixFmtIter != frameBuffer.end(); ++pixFmtIter) + for(auto frameIterator = frameBuffer.begin(); frameIterator != frameBuffer.end(); ++frameIterator) { - const auto& pixFmtMap = pixFmtIter.value(); + AVFrame* frame = frameIterator->second; - for(auto dimKeyIter = pixFmtMap.begin(); dimKeyIter != pixFmtMap.end(); ++dimKeyIter) + // Treat source frame and derived frames separately + if(sourceFrameKey == frameIterator->first) { - AVFrame* frame = dimKeyIter.value(); - - // Treat source frame and derived frames separately - if(pixFmtIter.key() == sourcePixelFormat && dimKeyIter.key() == sourceKey) - { - if(freeSourceFrame) - { - av_freep(&frame->data[0]); - } - av_frame_unref(frame); - av_frame_free(&frame); - } - else + if(freeSourceFrame) { av_freep(&frame->data[0]); - av_frame_unref(frame); - av_frame_free(&frame); } + av_frame_unref(frame); + av_frame_free(&frame); + } + else + { + av_freep(&frame->data[0]); + av_frame_unref(frame); + av_frame_free(&frame); } } frameBuffer.clear(); } + +VideoFrame::FrameBufferKey::FrameBufferKey(const int pixFmt, const int width, const int height, const bool lineAligned) + : frameWidth(width), + frameHeight(height), + pixelFormat(pixFmt), + linesizeAligned(lineAligned){} diff --git a/src/video/videoframe.h b/src/video/videoframe.h index 043d5761e..ceebb78ae 100644 --- a/src/video/videoframe.h +++ b/src/video/videoframe.h @@ -20,7 +20,6 @@ #ifndef VIDEOFRAME_H #define VIDEOFRAME_H -#include #include #include #include @@ -33,6 +32,7 @@ extern "C"{ #include #include #include +#include /** * @brief An ownernship and management class for AVFrames. @@ -44,6 +44,14 @@ extern "C"{ * * Every function in this class is thread safe apart from concurrent construction and deletion of * the object. + * + * This class uses the phrase "frame alignment" to specify the property that each frame's width is + * equal to it's maximum linesize. Note: this is NOT "data alignment" which specifies how allocated + * buffers are aligned in memory. Though internally the two are related, unless otherwise specified + * all instances of the term "alignment" exposed from public functions refer to frame alignment. + * + * Frame alignment is an important concept because ToxAV does not support frames with linesizes not + * directly equal to the width. */ class VideoFrame { @@ -91,12 +99,14 @@ public: * If a given frame does not exist, this function will perform appropriate conversions to * return a frame that fulfills the given parameters. * - * @param dimensions the dimensions of the frame. + * @param frameSize the dimensions of the frame to get. If frame size is 0, defaults to source + * frame size. * @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. */ - const AVFrame* getAVFrame(const QSize& dimensions, const int pixelFormat); + const AVFrame* getAVFrame(QSize frameSize, const int pixelFormat, const bool requireAligned); /** * @brief Releases all frames managed by this VideoFrame and invalidates it. @@ -151,21 +161,122 @@ public: /** * @brief Data alignment parameter used to populate AVFrame buffers. * - * This field is public in effort to standardized the frame alignment parameter for all AVFrame + * This field is public in effort to standardized the data alignment parameter for all AVFrame * allocations. * * It's currently set to 32-byte alignment for AVX2 support. */ - static constexpr int frameAlignment = 32; + static constexpr int dataAlignment = 32; + private: /** - * @brief A function to create a hashable key from a given QSize dimension. - * @param dimensions the dimensions to hash. - * @return a hashable unsigned 64-bit number representing the dimension. + * @brief A class representing a structure that stores frame properties to be used as the key + * value for a std::unordered_map. */ - static inline quint64 dimensionsToKey(const QSize& dimensions) + class FrameBufferKey{ + public: + /** + * @brief Constructs a new FrameBufferKey with the given attributes. + * + * @param width the width of the frame. + * @param height the height of the frame. + * @param pixFmt the pixel format of the frame. + * @param lineAligned whether the linesize matches the width of the image. + */ + FrameBufferKey(const int width, const int height, const int pixFmt, const bool lineAligned); + + // Explictly state default constructor/destructor + + FrameBufferKey(const FrameBufferKey&) = default; + FrameBufferKey(FrameBufferKey&&) = default; + ~FrameBufferKey() = default; + + // Assignment operators are disabled for the FrameBufferKey + + const FrameBufferKey& operator=(const FrameBufferKey&) = delete; + const FrameBufferKey& operator=(FrameBufferKey&&) = delete; + + /** + * @brief Comparison operator for FrameBufferKey. + * + * @param other instance to compare against. + * @return true if instances are equivilent, false otherwise. + */ + inline bool operator==(const FrameBufferKey& other) const + { + return pixelFormat == other.pixelFormat && + frameWidth == other.frameWidth && + frameHeight == other.frameHeight && + linesizeAligned == other.linesizeAligned; + } + + /** + * @brief Not equal to operator for FrameBufferKey. + * + * @param other instance to compare against + * @return true if instances are not equivilent, false otherwise. + */ + inline bool operator!=(const FrameBufferKey& other) const + { + return !operator==(other); + } + + /** + * @brief Hash function for class. + * + * This function computes a hash value for use with std::unordered_map. + * + * @param key the given instance to compute hash value of. + * @return the hash of the given instance. + */ + static inline size_t hash(const FrameBufferKey& key) + { + std::hash intHasher; + std::hash boolHasher; + + // Use java-style hash function to combine fields + size_t ret = 47; + + ret = 37 * ret + intHasher(key.frameWidth); + ret = 37 * ret + intHasher(key.frameHeight); + ret = 37 * ret + intHasher(key.pixelFormat); + ret = 37 * ret + boolHasher(key.linesizeAligned); + + return ret; + } + + public: + const int frameWidth; + const int frameHeight; + const int pixelFormat; + const bool linesizeAligned; + }; + +private: + /** + * @brief Generates a key object based on given parameters. + * + * @param frameSize the given size of the frame. + * @param pixFmt the pixel format of the frame. + * @param linesize the maximum linesize of the frame, may be larger than the width. + * @return a FrameBufferKey object representing the key for the frameBuffer map. + */ + static inline FrameBufferKey getFrameKey(const QSize& frameSize, const int pixFmt, const int linesize) { - return (static_cast(dimensions.width()) << 32) | dimensions.height(); + return getFrameKey(frameSize, pixFmt, frameSize.width() == linesize); + } + + /** + * @brief Generates a key object based on given parameters. + * + * @param frameSize the given size of the frame. + * @param pixFmt the pixel format of the frame. + * @param frameAligned true if the frame is aligned, false otherwise. + * @return a FrameBufferKey object representing the key for the frameBuffer map. + */ + static inline FrameBufferKey getFrameKey(const QSize& frameSize, const int pixFmt, const bool frameAligned) + { + return {frameSize.width(), frameSize.height(), pixFmt, frameAligned}; } /** @@ -179,10 +290,11 @@ private: * * @param dimensions the dimensions of the frame. * @param pixelFormat the desired pixel format of the frame. + * @param requireAligned true if the frame must be frame aligned, false otherwise. * @return a pointer to a AVFrame with the given parameters or nullptr if no such frame was * found. */ - AVFrame* retrieveAVFrame(const QSize& dimensions, const int pixelFormat); + AVFrame* retrieveAVFrame(const QSize& dimensions, const int pixelFormat, const bool requireAligned); /** * @brief Generates an AVFrame based on the given specifications. @@ -191,9 +303,10 @@ private: * * @param dimensions the required dimensions for the frame. * @param pixelFormat the required pixel format for the frame. + * @param requireAligned true if the generated frame needs to be frame aligned, false otherwise. * @return an AVFrame with the given specifications. */ - AVFrame* generateAVFrame(const QSize& dimensions, const int pixelFormat); + AVFrame* generateAVFrame(const QSize& dimensions, const int pixelFormat, const bool requireAligned); /** * @brief Stores a given AVFrame within the frameBuffer map. @@ -212,13 +325,15 @@ private: * This function is not thread-safe and must be called from a thread-safe context. */ void deleteFrameBuffer(); + private: // Main framebuffer store - QHash> frameBuffer {}; + std::unordered_map> frameBuffer {3, FrameBufferKey::hash}; // Source frame const QRect sourceDimensions; const int sourcePixelFormat; + const FrameBufferKey sourceFrameKey; const bool freeSourceFrame; // Destructor callback