From 97120b2537a56f8856df7cdf2142f6c94b9fee29 Mon Sep 17 00:00:00 2001 From: Mirco Miranda Date: Sun, 6 Oct 2024 17:26:25 +0000 Subject: [PATCH] Simplified read/verify header process Where possible, QIODevice::peek has been used instead of transactions or instead of using ungetchar() for sequential access devices and seek() for random access devices. Furthermore: - RAS format gained the ability of read on sequential devices. - Removed unused code in XCF (still related to ungetchar and sequential devices). - These changes should prevent errors like the ones fixed by MR !258 --- src/imageformats/pcx.cpp | 27 +++---------- src/imageformats/pxr.cpp | 6 +-- src/imageformats/qoi.cpp | 22 ++--------- src/imageformats/ras.cpp | 34 +++------------- src/imageformats/rgb.cpp | 26 +------------ src/imageformats/tga.cpp | 33 +--------------- src/imageformats/xcf.cpp | 83 +++++++++++++++------------------------- 7 files changed, 51 insertions(+), 180 deletions(-) diff --git a/src/imageformats/pcx.cpp b/src/imageformats/pcx.cpp index c707e3b..c34c1de 100644 --- a/src/imageformats/pcx.cpp +++ b/src/imageformats/pcx.cpp @@ -266,31 +266,16 @@ PCXHEADER::PCXHEADER() bool peekHeader(QIODevice *d, PCXHEADER& h) { - qint64 oldPos = d->pos(); - QByteArray head = d->read(sizeof(PCXHEADER)); - int readBytes = head.size(); - - if (d->isSequential()) { - for (int pos = readBytes -1; pos >= 0; --pos) { - d->ungetChar(head[pos]); - } - } else { - d->seek(oldPos); - } - - if (readBytes < sizeof(PCXHEADER)) { + auto head = d->peek(sizeof(PCXHEADER)); + if (size_t(head.size()) < sizeof(PCXHEADER)) { return false; } - auto ok = false; - { // datastream is destroyed before working on device - QDataStream ds(head); - ds.setByteOrder(QDataStream::LittleEndian); - ds >> h; - ok = ds.status() == QDataStream::Ok && h.isValid(); - } + QDataStream ds(head); + ds.setByteOrder(QDataStream::LittleEndian); + ds >> h; - return ok; + return ds.status() == QDataStream::Ok && h.isValid(); } static bool readLine(QDataStream &s, QByteArray &buf, const PCXHEADER &header) diff --git a/src/imageformats/pxr.cpp b/src/imageformats/pxr.cpp index 181bddb..668c7d2 100644 --- a/src/imageformats/pxr.cpp +++ b/src/imageformats/pxr.cpp @@ -121,10 +121,8 @@ public: bool peek(QIODevice *d) { - d->startTransaction(); - auto ok = read(d); - d->rollbackTransaction(); - return ok; + m_rawHeader = d->peek(512); + return isValid(); } bool jumpToImageData(QIODevice *d) const diff --git a/src/imageformats/qoi.cpp b/src/imageformats/qoi.cpp index fdc7a5e..09fe138 100644 --- a/src/imageformats/qoi.cpp +++ b/src/imageformats/qoi.cpp @@ -341,12 +341,8 @@ bool QOIHandler::canRead(QIODevice *device) return false; } - device->startTransaction(); - QByteArray head = device->read(QOI_HEADER_SIZE); - qsizetype readBytes = head.size(); - device->rollbackTransaction(); - - if (readBytes < QOI_HEADER_SIZE) { + auto head = device->peek(QOI_HEADER_SIZE); + if (head.size() < QOI_HEADER_SIZE) { return false; } @@ -430,12 +426,7 @@ QVariant QOIHandler::option(ImageOption option) const if (IsSupported(header)) { v = QVariant::fromValue(QSize(header.Width, header.Height)); } else if (auto d = device()) { - // transactions works on both random and sequential devices - d->startTransaction(); - auto ba = d->read(sizeof(QoiHeader)); - d->rollbackTransaction(); - - QDataStream s(ba); + QDataStream s(d->peek(sizeof(QoiHeader))); s.setByteOrder(QDataStream::BigEndian); s >> header; if (s.status() == QDataStream::Ok && IsSupported(header)) { @@ -449,12 +440,7 @@ QVariant QOIHandler::option(ImageOption option) const if (IsSupported(header)) { v = QVariant::fromValue(imageFormat(header)); } else if (auto d = device()) { - // transactions works on both random and sequential devices - d->startTransaction(); - auto ba = d->read(sizeof(QoiHeader)); - d->rollbackTransaction(); - - QDataStream s(ba); + QDataStream s(d->peek(sizeof(QoiHeader))); s.setByteOrder(QDataStream::BigEndian); s >> header; if (s.status() == QDataStream::Ok && IsSupported(header)) { diff --git a/src/imageformats/ras.cpp b/src/imageformats/ras.cpp index 801dfa0..33acf4a 100644 --- a/src/imageformats/ras.cpp +++ b/src/imageformats/ras.cpp @@ -202,8 +202,6 @@ private: static bool LoadRAS(QDataStream &s, const RasHeader &ras, QImage &img) { - s.device()->seek(RasHeader::SIZE); - // The width of a scan line is always a multiple of 16 bits, padded when necessary. auto rasLineSize = (qint64(ras.Width) * ras.Depth + 7) / 8; if (rasLineSize & 1) @@ -368,18 +366,8 @@ bool RASHandler::canRead(QIODevice *device) return false; } - if (device->isSequential()) { - // qWarning("Reading ras files from sequential devices not supported"); - return false; - } - - qint64 oldPos = device->pos(); - QByteArray head = device->read(RasHeader::SIZE); // header is exactly 32 bytes, always FIXME - int readBytes = head.size(); // this should always be 32 bytes - - device->seek(oldPos); - - if (readBytes < RasHeader::SIZE) { + auto head = device->peek(RasHeader::SIZE); // header is exactly 32 bytes, always FIXME + if (head.size() < RasHeader::SIZE) { return false; } @@ -411,9 +399,7 @@ bool RASHandler::read(QImage *outImage) } QImage img; - bool result = LoadRAS(s, ras, img); - - if (result == false) { + if (!LoadRAS(s, ras, img)) { // qDebug() << "Error loading RAS file."; return false; } @@ -443,12 +429,7 @@ QVariant RASHandler::option(ImageOption option) const v = QVariant::fromValue(QSize(header.Width, header.Height)); } else if (auto dev = device()) { - // transactions works on both random and sequential devices - dev->startTransaction(); - auto ba = dev->read(RasHeader::SIZE); - dev->rollbackTransaction(); - - QDataStream s(ba); + QDataStream s(dev->peek(RasHeader::SIZE)); s.setByteOrder(QDataStream::BigEndian); s >> header; if (s.status() == QDataStream::Ok && IsSupported(header)) { @@ -463,12 +444,7 @@ QVariant RASHandler::option(ImageOption option) const v = QVariant::fromValue(imageFormat(header)); } else if (auto dev = device()) { - // transactions works on both random and sequential devices - dev->startTransaction(); - auto ba = dev->read(RasHeader::SIZE); - dev->rollbackTransaction(); - - QDataStream s(ba); + QDataStream s(dev->peek(RasHeader::SIZE)); s.setByteOrder(QDataStream::BigEndian); s >> header; if (s.status() == QDataStream::Ok && IsSupported(header)) { diff --git a/src/imageformats/rgb.cpp b/src/imageformats/rgb.cpp index 224f63d..33367f2 100644 --- a/src/imageformats/rgb.cpp +++ b/src/imageformats/rgb.cpp @@ -578,30 +578,8 @@ bool SGIImagePrivate::isSupported() const bool SGIImagePrivate::peekHeader(QIODevice *device) { - qint64 pos = 0; - if (!device->isSequential()) { - pos = device->pos(); - } - - auto ok = false; - QByteArray header; - { // datastream is destroyed before working on device - header = device->read(512); - QDataStream ds(header); - ok = SGIImagePrivate::readHeader(ds, this) && isValid(); - } - - if (!device->isSequential()) { - return device->seek(pos) && ok; - } - - // sequential device undo - auto head = header.data(); - auto readBytes = header.size(); - while (readBytes > 0) { - device->ungetChar(head[readBytes-- - 1]); - } - return ok; + QDataStream ds(device->peek(512)); + return SGIImagePrivate::readHeader(ds, this) && isValid(); } QSize SGIImagePrivate::size() const diff --git a/src/imageformats/tga.cpp b/src/imageformats/tga.cpp index 3f816be..ccb08c2 100644 --- a/src/imageformats/tga.cpp +++ b/src/imageformats/tga.cpp @@ -195,26 +195,13 @@ static QImage::Format imageFormat(const TgaHeader &head) */ static bool peekHeader(QIODevice *device, TgaHeader &header) { - qint64 oldPos = device->pos(); - QByteArray head = device->read(TgaHeader::SIZE); - int readBytes = head.size(); - - if (device->isSequential()) { - for (int pos = readBytes - 1; pos >= 0; --pos) { - device->ungetChar(head[pos]); - } - } else { - device->seek(oldPos); - } - - if (readBytes < TgaHeader::SIZE) { + auto head = device->peek(TgaHeader::SIZE); + if (head.size() < TgaHeader::SIZE) { return false; } - QDataStream stream(head); stream.setByteOrder(QDataStream::LittleEndian); stream >> header; - return true; } @@ -561,22 +548,6 @@ bool TGAHandler::canRead(QIODevice *device) return false; } - qint64 oldPos = device->pos(); - QByteArray head = device->read(TgaHeader::SIZE); - int readBytes = head.size(); - - if (device->isSequential()) { - for (int pos = readBytes - 1; pos >= 0; --pos) { - device->ungetChar(head[pos]); - } - } else { - device->seek(oldPos); - } - - if (readBytes < TgaHeader::SIZE) { - return false; - } - TgaHeader tga; if (!peekHeader(device, tga)) { qWarning("TGAHandler::canRead() error while reading the header"); diff --git a/src/imageformats/xcf.cpp b/src/imageformats/xcf.cpp index 3d67eca..41efaaf 100644 --- a/src/imageformats/xcf.cpp +++ b/src/imageformats/xcf.cpp @@ -4229,66 +4229,43 @@ bool XCFHandler::canRead(QIODevice *device) } const qint64 oldPos = device->pos(); - if (!device->isSequential()) { - QDataStream ds(device); - XCFImageFormat::XCFImage::Header header; - bool failed = !XCFImageFormat::readXCFHeader(ds, &header); - ds.setDevice(nullptr); - device->seek(oldPos); - if (failed) { - return false; - } - switch (header.precision) { - case XCFImageFormat::GIMP_PRECISION_HALF_LINEAR: - case XCFImageFormat::GIMP_PRECISION_HALF_NON_LINEAR: - case XCFImageFormat::GIMP_PRECISION_HALF_PERCEPTUAL: - case XCFImageFormat::GIMP_PRECISION_FLOAT_LINEAR: - case XCFImageFormat::GIMP_PRECISION_FLOAT_NON_LINEAR: - case XCFImageFormat::GIMP_PRECISION_FLOAT_PERCEPTUAL: - case XCFImageFormat::GIMP_PRECISION_U8_LINEAR: - case XCFImageFormat::GIMP_PRECISION_U8_NON_LINEAR: - case XCFImageFormat::GIMP_PRECISION_U8_PERCEPTUAL: - case XCFImageFormat::GIMP_PRECISION_U16_LINEAR: - case XCFImageFormat::GIMP_PRECISION_U16_NON_LINEAR: - case XCFImageFormat::GIMP_PRECISION_U16_PERCEPTUAL: - case XCFImageFormat::GIMP_PRECISION_U32_LINEAR: - case XCFImageFormat::GIMP_PRECISION_U32_NON_LINEAR: - case XCFImageFormat::GIMP_PRECISION_U32_PERCEPTUAL: - break; - case XCFImageFormat::GIMP_PRECISION_DOUBLE_LINEAR: - case XCFImageFormat::GIMP_PRECISION_DOUBLE_NON_LINEAR: - case XCFImageFormat::GIMP_PRECISION_DOUBLE_PERCEPTUAL: - default: - qCDebug(XCFPLUGIN) << "unsupported precision" << header.precision; - return false; - } + QDataStream ds(device); + XCFImageFormat::XCFImage::Header header; + bool failed = !XCFImageFormat::readXCFHeader(ds, &header); + ds.setDevice(nullptr); - return true; - } - - char head[8]; - qint64 readBytes = device->read(head, sizeof(head)); - if (readBytes != sizeof(head)) { - if (device->isSequential()) { - while (readBytes > 0) { - device->ungetChar(head[readBytes-- - 1]); - } - } else { - device->seek(oldPos); - } + device->seek(oldPos); + if (failed) { return false; } - if (device->isSequential()) { - while (readBytes > 0) { - device->ungetChar(head[readBytes-- - 1]); - } - } else { - device->seek(oldPos); + switch (header.precision) { + case XCFImageFormat::GIMP_PRECISION_HALF_LINEAR: + case XCFImageFormat::GIMP_PRECISION_HALF_NON_LINEAR: + case XCFImageFormat::GIMP_PRECISION_HALF_PERCEPTUAL: + case XCFImageFormat::GIMP_PRECISION_FLOAT_LINEAR: + case XCFImageFormat::GIMP_PRECISION_FLOAT_NON_LINEAR: + case XCFImageFormat::GIMP_PRECISION_FLOAT_PERCEPTUAL: + case XCFImageFormat::GIMP_PRECISION_U8_LINEAR: + case XCFImageFormat::GIMP_PRECISION_U8_NON_LINEAR: + case XCFImageFormat::GIMP_PRECISION_U8_PERCEPTUAL: + case XCFImageFormat::GIMP_PRECISION_U16_LINEAR: + case XCFImageFormat::GIMP_PRECISION_U16_NON_LINEAR: + case XCFImageFormat::GIMP_PRECISION_U16_PERCEPTUAL: + case XCFImageFormat::GIMP_PRECISION_U32_LINEAR: + case XCFImageFormat::GIMP_PRECISION_U32_NON_LINEAR: + case XCFImageFormat::GIMP_PRECISION_U32_PERCEPTUAL: + break; + case XCFImageFormat::GIMP_PRECISION_DOUBLE_LINEAR: + case XCFImageFormat::GIMP_PRECISION_DOUBLE_NON_LINEAR: + case XCFImageFormat::GIMP_PRECISION_DOUBLE_PERCEPTUAL: + default: + qCDebug(XCFPLUGIN) << "unsupported precision" << header.precision; + return false; } - return qstrncmp(head, "gimp xcf", 8) == 0; + return true; } QImageIOPlugin::Capabilities XCFPlugin::capabilities(QIODevice *device, const QByteArray &format) const