From 11baa8dc38f2a7f8fd2535def0cd123796d7b97d Mon Sep 17 00:00:00 2001 From: Artem Dyomin Date: Sun, 4 Feb 2024 19:18:06 +0100 Subject: Eliminate copying of frame data upon creating textures from memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's possible to load QRhiTexture from QImage, and QImage supports zero-copy creation from a pointer. It's documented in QRhi that the image format is not considered upon loading QRhiTexture from memory, so we use just any size-compatible image format. Already existing unit tests cover the change, see tst_qvideoframecolormanagement. To make the tests pass, we should fix opengl rendering backend in qtbase, see the matching CR: codereview.qt-project.org/c/qt/qtbase/+/537233 Task-number: QTBUG-121934 Pick-to: 6.7 6.6 6.5 Change-Id: Ief488550e8f8fe5dbbc6680f5284c6e540a1f3b4 Reviewed-by: Jøger Hansegård Reviewed-by: Artem Dyomin --- src/multimedia/video/qvideoframe.h | 1 + src/multimedia/video/qvideoframe_p.h | 12 +++++- src/multimedia/video/qvideoframeconverter.cpp | 32 ++++++++++++++ src/multimedia/video/qvideoframeconverter_p.h | 7 +++ src/multimedia/video/qvideotexturehelper.cpp | 61 ++++++++++++++++----------- 5 files changed, 88 insertions(+), 25 deletions(-) (limited to 'src/multimedia') diff --git a/src/multimedia/video/qvideoframe.h b/src/multimedia/video/qvideoframe.h index 45f79fa6b..f76f0c8f1 100644 --- a/src/multimedia/video/qvideoframe.h +++ b/src/multimedia/video/qvideoframe.h @@ -134,6 +134,7 @@ public: QAbstractVideoBuffer *videoBuffer() const; private: + friend class QVideoFramePrivate; QExplicitlySharedDataPointer d; }; diff --git a/src/multimedia/video/qvideoframe_p.h b/src/multimedia/video/qvideoframe_p.h index 905fdacd6..f5037f7a5 100644 --- a/src/multimedia/video/qvideoframe_p.h +++ b/src/multimedia/video/qvideoframe_p.h @@ -15,8 +15,9 @@ // We mean it. // +#include "qvideoframe.h" #include "qabstractvideobuffer_p.h" -#include "qshareddata.h" + #include #include // std::once @@ -30,6 +31,15 @@ public: ~QVideoFramePrivate() { delete buffer; } + static QVideoFramePrivate *handle(QVideoFrame &frame) { return frame.d.get(); }; + + QVideoFrame adoptThisByVideoFrame() + { + QVideoFrame frame; + frame.d = QExplicitlySharedDataPointer(this, QAdoptSharedDataTag{}); + return frame; + } + qint64 startTime = -1; qint64 endTime = -1; QAbstractVideoBuffer::MapData mapData; diff --git a/src/multimedia/video/qvideoframeconverter.cpp b/src/multimedia/video/qvideoframeconverter.cpp index ca4a6ad92..0a58b2bc5 100644 --- a/src/multimedia/video/qvideoframeconverter.cpp +++ b/src/multimedia/video/qvideoframeconverter.cpp @@ -4,6 +4,7 @@ #include "qvideoframeconverter_p.h" #include "qvideoframeconversionhelper_p.h" #include "qvideoframeformat.h" +#include "qvideoframe_p.h" #include #include @@ -423,5 +424,36 @@ QImage qImageFromVideoFrame(const QVideoFrame &frame, QtVideo::Rotation rotation QImage::Format_RGBA8888_Premultiplied, imageCleanupHandler, imageData); } +QImage videoFramePlaneAsImage(QVideoFrame &frame, int plane, QImage::Format targetFormat, + QSize targetSize) +{ + if (plane >= frame.planeCount()) + return {}; + + if (!frame.map(QVideoFrame::ReadOnly)) { + qWarning() << "Cannot map a video frame in ReadOnly mode!"; + return {}; + } + + auto frameHandle = QVideoFramePrivate::handle(frame); + + // With incrementing the reference counter, we share the mapped QVideoFrame + // with the target QImage. The function imageCleanupFunction is going to adopt + // the frameHandle by QVideoFrame and dereference it upon the destruction. + frameHandle->ref.ref(); + + auto imageCleanupFunction = [](void *data) { + QVideoFrame frame = reinterpret_cast(data)->adoptThisByVideoFrame(); + Q_ASSERT(frame.isMapped()); + frame.unmap(); + }; + + const auto bytesPerLine = frame.bytesPerLine(plane); + + return QImage(reinterpret_cast(frame.bits(plane)), targetSize.width(), + qMin(targetSize.height(), frame.mappedBytes(plane) / bytesPerLine), bytesPerLine, + targetFormat, imageCleanupFunction, frameHandle); +} + QT_END_NAMESPACE diff --git a/src/multimedia/video/qvideoframeconverter_p.h b/src/multimedia/video/qvideoframeconverter_p.h index 73579ddd9..d22491f66 100644 --- a/src/multimedia/video/qvideoframeconverter_p.h +++ b/src/multimedia/video/qvideoframeconverter_p.h @@ -21,6 +21,13 @@ QT_BEGIN_NAMESPACE Q_MULTIMEDIA_EXPORT QImage qImageFromVideoFrame(const QVideoFrame &frame, QtVideo::Rotation rotation = QtVideo::Rotation::None, bool mirrorX = false, bool mirrorY = false); +/** + * @brief Maps the video frame and returns an image having a shared ownership for the video frame + * and referencing to its mapped data. + */ +Q_MULTIMEDIA_EXPORT QImage videoFramePlaneAsImage(QVideoFrame &frame, int plane, + QImage::Format targetFromat, QSize targetSize); + QT_END_NAMESPACE #endif diff --git a/src/multimedia/video/qvideotexturehelper.cpp b/src/multimedia/video/qvideotexturehelper.cpp index 75bf10256..d1ff90cfc 100644 --- a/src/multimedia/video/qvideotexturehelper.cpp +++ b/src/multimedia/video/qvideotexturehelper.cpp @@ -2,8 +2,8 @@ // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only #include "qvideotexturehelper_p.h" -#include "qvideoframe.h" #include "qabstractvideobuffer_p.h" +#include "qvideoframeconverter_p.h" #include #include @@ -561,10 +561,31 @@ void updateUniformData(QByteArray *dst, const QVideoFrameFormat &format, const Q ud->maxLum = fromLinear(float(maxNits)/100.f); } -static bool updateTextureWithMap(QVideoFrame& frame, QRhi *rhi, QRhiResourceUpdateBatch *rub, int plane, std::unique_ptr &tex) +// There's no maping between QRhiTexture formats and QImage formats. +// The function gets a size compatible format; it works fine as +// QRhi relies on the given QRhiTexture format instead of the image format. +static QImage::Format sizeCompatibleImageFormat(QRhiTexture::Format rhiFromat) { - Q_ASSERT(frame.isMapped()); + switch (rhiFromat) { + case QRhiTexture::RGBA8: + case QRhiTexture::BGRA8: + case QRhiTexture::RG16: + return QImage::Format_ARGB32; + case QRhiTexture::R16: + case QRhiTexture::RG8: + return QImage::Format_Grayscale16; + case QRhiTexture::R8: + return QImage::Format_Grayscale8; + default: + qWarning() << "QRhiTexture::Format" << rhiFromat + << "must be mapped to a size-compatible QImage::Format"; + Q_ASSERT(!"QRhiTexture::Format is not mapped to a size-compatible QImage::Format"); + return QImage::Format_Grayscale8; + } +} +static bool updateTextureWithMap(QVideoFrame& frame, QRhi *rhi, QRhiResourceUpdateBatch *rub, int plane, std::unique_ptr &tex) +{ QVideoFrameFormat fmt = frame.surfaceFormat(); QVideoFrameFormat::PixelFormat pixelFormat = fmt.pixelFormat(); QSize size = fmt.frameSize(); @@ -593,26 +614,25 @@ static bool updateTextureWithMap(QVideoFrame& frame, QRhi *rhi, QRhiResourceUpda QRhiTextureSubresourceUploadDescription subresDesc; QImage image; if (pixelFormat == QVideoFrameFormat::Format_Jpeg) { + Q_ASSERT(plane == 0); + + // calling QVideoFrame::toImage is not accurate. To be fixed. image = frame.toImage(); image.convertTo(QImage::Format_ARGB32); - subresDesc.setData(QByteArray((const char *)image.bits(), image.bytesPerLine()*image.height())); - subresDesc.setDataStride(image.bytesPerLine()); - } else { - const auto frameBits = reinterpret_cast(frame.bits(plane)); - const auto mappedBytes = frame.mappedBytes(plane); - const auto underlyingByteArray = frame.videoBuffer()->underlyingByteArray(plane); - if (underlyingByteArray.size() == mappedBytes) { - Q_ASSERT(underlyingByteArray.constData() == frameBits); - subresDesc.setData(underlyingByteArray); - } - else { - subresDesc.setData(QByteArray::fromRawData(frameBits, mappedBytes)); - } + } else { + image = videoFramePlaneAsImage( + frame, plane, sizeCompatibleImageFormat(texDesc.textureFormat[plane]), planeSize); + } - subresDesc.setDataStride(frame.bytesPerLine(plane)); + if (image.isNull()) { + qWarning() << "Cannot represent the plane" << plane << "as an image"; + return false; } + subresDesc.setImage(image); + subresDesc.setDataStride(image.bytesPerLine()); + QRhiTextureUploadEntry entry(0, 0, subresDesc); QRhiTextureUploadDescription desc({ entry }); rub->uploadTexture(tex.get(), desc); @@ -695,13 +715,6 @@ static std::unique_ptr createTexturesFromMemory(QVideoFrame if (oldArray) textures = oldArray->takeTextures(); - if (!frame.map(QVideoFrame::ReadOnly)) { - qWarning() << "could not map data of QVideoFrame for upload"; - return {}; - } - - auto unmapFrameGuard = qScopeGuard([&frame] { frame.unmap(); }); - bool ok = true; for (quint8 plane = 0; plane < texDesc.nplanes; ++plane) { ok &= updateTextureWithMap(frame, rhi, rub, plane, textures[plane]); -- cgit v1.2.3