From 022da0014faa103901ec107ed6a33e5ab00c7931 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Thu, 1 Oct 2015 13:49:28 -0700 Subject: Clean up some image decoder classes: - Use std::vector instead of raw uint8_t* - Make ICodec_ScanlineDecoder::GetScanline() return const uint8_t* - Add FxFreeDeleter, use it in CCodec_ImageDataCache. - Make CCodec_ImageDataCache encapsulate its data members. R=tsepez@chromium.org Review URL: https://codereview.chromium.org/1361053002 . --- core/include/fxcodec/fx_codec.h | 14 ++- core/include/fxcrt/fx_basic.h | 4 + .../src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp | 6 +- .../fpdfapi/fpdf_render/fpdf_render_loadimage.cpp | 31 ++--- core/src/fxcodec/codec/codec_int.h | 73 +++++------ core/src/fxcodec/codec/fx_codec.cpp | 133 ++++++++++++++------- core/src/fxcodec/codec/fx_codec_jpx_opj.cpp | 80 ++++++------- 7 files changed, 187 insertions(+), 154 deletions(-) diff --git a/core/include/fxcodec/fx_codec.h b/core/include/fxcodec/fx_codec.h index b3133694b8..625ec08e7d 100644 --- a/core/include/fxcodec/fx_codec.h +++ b/core/include/fxcodec/fx_codec.h @@ -7,6 +7,8 @@ #ifndef CORE_INCLUDE_FXCODEC_FX_CODEC_H_ #define CORE_INCLUDE_FXCODEC_FX_CODEC_H_ +#include + #include "../../../third_party/base/nonstd_unique_ptr.h" #include "../fxcrt/fx_basic.h" #include "fx_codec_def.h" @@ -64,6 +66,7 @@ class ICodec_BasicModule { int nComps, int bpc) = 0; }; + class ICodec_ScanlineDecoder { public: virtual ~ICodec_ScanlineDecoder() {} @@ -72,7 +75,7 @@ class ICodec_ScanlineDecoder { virtual void DownScale(int dest_width, int dest_height) = 0; - virtual uint8_t* GetScanline(int line) = 0; + virtual const uint8_t* GetScanline(int line) = 0; virtual FX_BOOL SkipToScanline(int line, IFX_Pause* pPause) = 0; @@ -88,6 +91,7 @@ class ICodec_ScanlineDecoder { virtual void ClearImageData() = 0; }; + class ICodec_FlateModule { public: virtual ~ICodec_FlateModule() {} @@ -211,10 +215,10 @@ class ICodec_JpxModule { FX_DWORD* height, FX_DWORD* components) = 0; - virtual FX_BOOL Decode(CJPX_Decoder* pDecoder, - uint8_t* dest_data, - int pitch, - uint8_t* offsets) = 0; + virtual bool Decode(CJPX_Decoder* pDecoder, + uint8_t* dest_data, + int pitch, + const std::vector& offsets) = 0; virtual void DestroyDecoder(CJPX_Decoder* pDecoder) = 0; }; diff --git a/core/include/fxcrt/fx_basic.h b/core/include/fxcrt/fx_basic.h index 3e556f5439..7412b8db3c 100644 --- a/core/include/fxcrt/fx_basic.h +++ b/core/include/fxcrt/fx_basic.h @@ -947,6 +947,10 @@ class CFX_AutoRestorer { T m_OldValue; }; +struct FxFreeDeleter { + inline void operator()(void* ptr) const { FX_Free(ptr); } +}; + // Used with nonstd::unique_ptr to Release() objects that can't be deleted. template struct ReleaseDeleter { diff --git a/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp b/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp index c9bcff6db6..ace7bf925b 100644 --- a/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp +++ b/core/src/fpdfapi/fpdf_page/fpdf_page_parser_old.cpp @@ -247,10 +247,10 @@ FX_DWORD _DecodeAllScanlines(ICodec_ScanlineDecoder* pDecoder, dest_buf = FX_Alloc2D(uint8_t, pitch, height); dest_size = pitch * height; // Safe since checked alloc returned. for (int row = 0; row < height; row++) { - uint8_t* pLine = pDecoder->GetScanline(row); - if (pLine == NULL) { + const uint8_t* pLine = pDecoder->GetScanline(row); + if (!pLine) break; - } + FXSYS_memcpy(dest_buf + row * pitch, pLine, pitch); } FX_DWORD srcoff = pDecoder->GetSrcOffset(); diff --git a/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp b/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp index 9497943fbd..f1fbf41d6d 100644 --- a/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp +++ b/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp @@ -4,6 +4,8 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com +#include + #include "../../../../third_party/base/nonstd_unique_ptr.h" #include "../../../include/fpdfapi/fpdf_module.h" #include "../../../include/fpdfapi/fpdf_pageobj.h" @@ -57,15 +59,13 @@ FX_SAFE_DWORD CalculatePitch32(int bpp, int width) { return pitch; } -// Wrapper class to hold objects allocated in CPDF_DIBSource::LoadJpxBitmap(), -// because nonstd::unique_ptr does not support custom deleters yet. +// Wrapper class to use with nonstd::unique_ptr for CJPX_Decoder. class JpxBitMapContext { public: explicit JpxBitMapContext(ICodec_JpxModule* jpx_module) - : jpx_module_(jpx_module), decoder_(nullptr), output_offsets_(nullptr) {} + : jpx_module_(jpx_module), decoder_(nullptr) {} ~JpxBitMapContext() { - FX_Free(output_offsets_); jpx_module_->DestroyDecoder(decoder_); } @@ -74,17 +74,9 @@ class JpxBitMapContext { CJPX_Decoder* decoder() { return decoder_; } - // Takes ownership of |output_offsets|. - void set_output_offsets(unsigned char* output_offsets) { - output_offsets_ = output_offsets; - } - - unsigned char* output_offsets() { return output_offsets_; } - private: - ICodec_JpxModule* jpx_module_; // Weak pointer. - CJPX_Decoder* decoder_; // Decoder, owned. - unsigned char* output_offsets_; // Output offsets for decoding, owned. + ICodec_JpxModule* const jpx_module_; // Weak pointer. + CJPX_Decoder* decoder_; // Decoder, owned. // Disallow evil constructors JpxBitMapContext(const JpxBitMapContext&); @@ -747,16 +739,15 @@ void CPDF_DIBSource::LoadJpxBitmap() { return; } m_pCachedBitmap->Clear(0xFFFFFFFF); - context->set_output_offsets(FX_Alloc(uint8_t, components)); + std::vector output_offsets(components); for (int i = 0; i < components; ++i) - context->output_offsets()[i] = i; + output_offsets[i] = i; if (bSwapRGB) { - context->output_offsets()[0] = 2; - context->output_offsets()[2] = 0; + output_offsets[0] = 2; + output_offsets[2] = 0; } if (!pJpxModule->Decode(context->decoder(), m_pCachedBitmap->GetBuffer(), - m_pCachedBitmap->GetPitch(), - context->output_offsets())) { + m_pCachedBitmap->GetPitch(), output_offsets)) { m_pCachedBitmap.reset(); return; } diff --git a/core/src/fxcodec/codec/codec_int.h b/core/src/fxcodec/codec/codec_int.h index 125f36c9eb..682b5d0dbb 100644 --- a/core/src/fxcodec/codec/codec_int.h +++ b/core/src/fxcodec/codec/codec_int.h @@ -11,6 +11,7 @@ #include #include +#include "../../../../third_party/base/nonstd_unique_ptr.h" #include "../../../../third_party/libopenjpeg20/openjpeg.h" // For OPJ_SIZE_T. #include "../../../include/fxcodec/fx_codec.h" #include "../jbig2/JBig2_Context.h" @@ -36,12 +37,6 @@ class CCodec_BasicModule : public ICodec_BasicModule { int bpc); }; -struct CCodec_ImageDataCache { - int m_Width, m_Height; - int m_nCachedLines; - uint8_t m_Data; -}; - class CCodec_ScanlineDecoder : public ICodec_ScanlineDecoder { public: CCodec_ScanlineDecoder(); @@ -50,50 +45,58 @@ class CCodec_ScanlineDecoder : public ICodec_ScanlineDecoder { // ICodec_ScanlineDecoder FX_DWORD GetSrcOffset() override { return -1; } void DownScale(int dest_width, int dest_height) override; - uint8_t* GetScanline(int line) override; + const uint8_t* GetScanline(int line) override; FX_BOOL SkipToScanline(int line, IFX_Pause* pPause) override; int GetWidth() override { return m_OutputWidth; } int GetHeight() override { return m_OutputHeight; } int CountComps() override { return m_nComps; } int GetBPC() override { return m_bpc; } FX_BOOL IsColorTransformed() override { return m_bColorTransformed; } - void ClearImageData() override { - FX_Free(m_pDataCache); - m_pDataCache = NULL; - } + void ClearImageData() override { m_pDataCache.reset(); } protected: - int m_OrigWidth; + class ImageDataCache { + public: + ImageDataCache(int width, int height, int pitch); + ~ImageDataCache(); + + bool AllocateCache(); + void AppendLine(const uint8_t* line); + + int NumLines() const { return m_nCachedLines; } + const uint8_t* GetLine(int line) const; + bool IsSameDimensions(int width, int height) const { + return width == m_Width && height == m_Height; + } + + private: + bool IsValid() const { return m_Data.get() != nullptr; } + + const int m_Width; + const int m_Height; + const int m_Pitch; + int m_nCachedLines; + nonstd::unique_ptr m_Data; + }; - int m_OrigHeight; + virtual FX_BOOL v_Rewind() = 0; + virtual uint8_t* v_GetNextLine() = 0; + virtual void v_DownScale(int dest_width, int dest_height) = 0; - int m_DownScale; + uint8_t* ReadNextLine(); + int m_OrigWidth; + int m_OrigHeight; + int m_DownScale; int m_OutputWidth; - int m_OutputHeight; - int m_nComps; - int m_bpc; - int m_Pitch; - FX_BOOL m_bColorTransformed; - - uint8_t* ReadNextLine(); - - virtual FX_BOOL v_Rewind() = 0; - - virtual uint8_t* v_GetNextLine() = 0; - - virtual void v_DownScale(int dest_width, int dest_height) = 0; - int m_NextLine; - uint8_t* m_pLastScanline; - - CCodec_ImageDataCache* m_pDataCache; + nonstd::unique_ptr m_pDataCache; }; class CCodec_FaxModule : public ICodec_FaxModule { @@ -257,10 +260,10 @@ class CCodec_JpxModule : public ICodec_JpxModule { FX_DWORD* width, FX_DWORD* height, FX_DWORD* components) override; - FX_BOOL Decode(CJPX_Decoder* pDecoder, - uint8_t* dest_data, - int pitch, - uint8_t* offsets) override; + bool Decode(CJPX_Decoder* pDecoder, + uint8_t* dest_data, + int pitch, + const std::vector& offsets) override; void DestroyDecoder(CJPX_Decoder* pDecoder) override; }; diff --git a/core/src/fxcodec/codec/fx_codec.cpp b/core/src/fxcodec/codec/fx_codec.cpp index 0d89ea6e98..46f479e0b1 100644 --- a/core/src/fxcodec/codec/fx_codec.cpp +++ b/core/src/fxcodec/codec/fx_codec.cpp @@ -5,7 +5,13 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com #include "../../../include/fxcodec/fx_codec.h" + +#include + +#include "../../../../third_party/base/logging.h" +#include "../../../include/fxcrt/fx_safe_types.h" #include "codec_int.h" + CCodec_ModuleMgr::CCodec_ModuleMgr() : m_pBasicModule(new CCodec_BasicModule), m_pFaxModule(new CCodec_FaxModule), @@ -14,25 +20,66 @@ CCodec_ModuleMgr::CCodec_ModuleMgr() m_pJbig2Module(new CCodec_Jbig2Module), m_pIccModule(new CCodec_IccModule), m_pFlateModule(new CCodec_FlateModule) {} -CCodec_ScanlineDecoder::CCodec_ScanlineDecoder() { - m_NextLine = -1; - m_pDataCache = NULL; - m_pLastScanline = NULL; + +CCodec_ScanlineDecoder::ImageDataCache::ImageDataCache(int width, + int height, + int pitch) + : m_Width(width), m_Height(height), m_Pitch(pitch), m_nCachedLines(0) { } -CCodec_ScanlineDecoder::~CCodec_ScanlineDecoder() { - FX_Free(m_pDataCache); + +CCodec_ScanlineDecoder::ImageDataCache::~ImageDataCache() { } -uint8_t* CCodec_ScanlineDecoder::GetScanline(int line) { - if (m_pDataCache && line < m_pDataCache->m_nCachedLines) { - return &m_pDataCache->m_Data + line * m_Pitch; + +bool CCodec_ScanlineDecoder::ImageDataCache::AllocateCache() { + if (m_Pitch <= 0 || m_Height < 0) + return false; + + FX_SAFE_SIZE_T size = m_Pitch; + size *= m_Height; + if (!size.IsValid()) + return false; + + m_Data.reset(FX_TryAlloc(uint8_t, size.ValueOrDie())); + return IsValid(); +} + +void CCodec_ScanlineDecoder::ImageDataCache::AppendLine(const uint8_t* line) { + // If the callers adds more lines than there is room, fail. + if (m_Pitch <= 0 || m_nCachedLines >= m_Height) { + NOTREACHED(); + return; } - if (m_NextLine == line + 1) { + + size_t offset = m_Pitch; + FXSYS_memcpy(m_Data.get() + offset * m_nCachedLines, line, m_Pitch); + ++m_nCachedLines; +} + +const uint8_t* CCodec_ScanlineDecoder::ImageDataCache::GetLine(int line) const { + if (m_Pitch <= 0 || line < 0 || line >= m_nCachedLines) + return nullptr; + + size_t offset = m_Pitch; + return m_Data.get() + offset * line; +} + +CCodec_ScanlineDecoder::CCodec_ScanlineDecoder() + : m_NextLine(-1), m_pLastScanline(nullptr) { +} + +CCodec_ScanlineDecoder::~CCodec_ScanlineDecoder() { +} + +const uint8_t* CCodec_ScanlineDecoder::GetScanline(int line) { + if (m_pDataCache && line < m_pDataCache->NumLines()) + return m_pDataCache->GetLine(line); + + if (m_NextLine == line + 1) return m_pLastScanline; - } + if (m_NextLine < 0 || m_NextLine > line) { - if (!v_Rewind()) { - return NULL; - } + if (!v_Rewind()) + return nullptr; m_NextLine = 0; } while (m_NextLine < line) { @@ -43,18 +90,19 @@ uint8_t* CCodec_ScanlineDecoder::GetScanline(int line) { m_NextLine++; return m_pLastScanline; } + FX_BOOL CCodec_ScanlineDecoder::SkipToScanline(int line, IFX_Pause* pPause) { - if (m_pDataCache && line < m_pDataCache->m_nCachedLines) { + if (m_pDataCache && line < m_pDataCache->NumLines()) return FALSE; - } - if (m_NextLine == line || m_NextLine == line + 1) { + + if (m_NextLine == line || m_NextLine == line + 1) return FALSE; - } + if (m_NextLine < 0 || m_NextLine > line) { v_Rewind(); m_NextLine = 0; } - m_pLastScanline = NULL; + m_pLastScanline = nullptr; while (m_NextLine < line) { m_pLastScanline = ReadNextLine(); m_NextLine++; @@ -64,42 +112,35 @@ FX_BOOL CCodec_ScanlineDecoder::SkipToScanline(int line, IFX_Pause* pPause) { } return FALSE; } + uint8_t* CCodec_ScanlineDecoder::ReadNextLine() { uint8_t* pLine = v_GetNextLine(); - if (pLine == NULL) { - return NULL; - } - if (m_pDataCache && m_NextLine == m_pDataCache->m_nCachedLines) { - FXSYS_memcpy(&m_pDataCache->m_Data + m_NextLine * m_Pitch, pLine, m_Pitch); - m_pDataCache->m_nCachedLines++; - } + if (!pLine) + return nullptr; + + if (m_pDataCache && m_NextLine == m_pDataCache->NumLines()) + m_pDataCache->AppendLine(pLine); return pLine; } + void CCodec_ScanlineDecoder::DownScale(int dest_width, int dest_height) { - if (dest_width < 0) { - dest_width = -dest_width; - } - if (dest_height < 0) { - dest_height = -dest_height; - } + dest_width = std::abs(dest_width); + dest_height = std::abs(dest_height); v_DownScale(dest_width, dest_height); - if (m_pDataCache) { - if (m_pDataCache->m_Height == m_OutputHeight && - m_pDataCache->m_Width == m_OutputWidth) { - return; - } - FX_Free(m_pDataCache); - m_pDataCache = NULL; - } - m_pDataCache = (CCodec_ImageDataCache*)FX_TryAlloc( - uint8_t, sizeof(CCodec_ImageDataCache) + m_Pitch * m_OutputHeight); - if (m_pDataCache == NULL) { + + if (m_pDataCache && + m_pDataCache->IsSameDimensions(m_OutputWidth, m_OutputHeight)) { return; } - m_pDataCache->m_Height = m_OutputHeight; - m_pDataCache->m_Width = m_OutputWidth; - m_pDataCache->m_nCachedLines = 0; + + nonstd::unique_ptr cache( + new ImageDataCache(m_OutputWidth, m_OutputHeight, m_Pitch)); + if (!cache->AllocateCache()) + return; + + m_pDataCache = nonstd::move(cache); } + FX_BOOL CCodec_BasicModule::RunLengthEncode(const uint8_t* src_buf, FX_DWORD src_size, uint8_t*& dest_buf, diff --git a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp index b7e032e614..fdffcb66de 100644 --- a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp +++ b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp @@ -6,6 +6,7 @@ #include #include +#include #include "../../../../third_party/lcms2-2.6/include/lcms2.h" #include "../../../../third_party/libopenjpeg20/openjpeg.h" @@ -617,9 +618,9 @@ class CJPX_Decoder { ~CJPX_Decoder(); FX_BOOL Init(const unsigned char* src_data, int src_size); void GetInfo(FX_DWORD* width, FX_DWORD* height, FX_DWORD* components); - FX_BOOL Decode(uint8_t* dest_buf, - int pitch, - uint8_t* offsets); + bool Decode(uint8_t* dest_buf, + int pitch, + const std::vector& offsets); private: const uint8_t* m_SrcData; @@ -738,45 +739,39 @@ void CJPX_Decoder::GetInfo(FX_DWORD* width, *components = (FX_DWORD)image->numcomps; } -FX_BOOL CJPX_Decoder::Decode(uint8_t* dest_buf, - int pitch, - uint8_t* offsets) { - int i, wid, hei, row, col, channel, src; - uint8_t* pChannel; - uint8_t* pScanline; - uint8_t* pPixel; +bool CJPX_Decoder::Decode(uint8_t* dest_buf, + int pitch, + const std::vector& offsets) { + if (image->comps[0].w != image->x1 || image->comps[0].h != image->y1) + return false; + + if (pitch<(int)(image->comps[0].w * 8 * image->numcomps + 31)>> 5 << 2) + return false; - if (image->comps[0].w != image->x1 || image->comps[0].h != image->y1) { - return FALSE; - } - if (pitch<(int)(image->comps[0].w * 8 * image->numcomps + 31)>> 5 << 2) { - return FALSE; - } FXSYS_memset(dest_buf, 0xff, image->y1 * pitch); - uint8_t** channel_bufs = FX_Alloc(uint8_t*, image->numcomps); - FX_BOOL result = FALSE; - int* adjust_comps = FX_Alloc(int, image->numcomps); - for (i = 0; i < (int)image->numcomps; i++) { + std::vector channel_bufs(image->numcomps); + std::vector adjust_comps(image->numcomps); + for (uint32_t i = 0; i < image->numcomps; i++) { channel_bufs[i] = dest_buf + offsets[i]; adjust_comps[i] = image->comps[i].prec - 8; if (i > 0) { if (image->comps[i].dx != image->comps[i - 1].dx || image->comps[i].dy != image->comps[i - 1].dy || image->comps[i].prec != image->comps[i - 1].prec) { - goto done; + return false; } } } - wid = image->comps[0].w; - hei = image->comps[0].h; - for (channel = 0; channel < (int)image->numcomps; channel++) { - pChannel = channel_bufs[channel]; + int width = image->comps[0].w; + int height = image->comps[0].h; + for (uint32_t channel = 0; channel < image->numcomps; ++channel) { + uint8_t* pChannel = channel_bufs[channel]; if (adjust_comps[channel] < 0) { - for (row = 0; row < hei; row++) { - pScanline = pChannel + row * pitch; - for (col = 0; col < wid; col++) { - pPixel = pScanline + col * image->numcomps; - src = image->comps[channel].data[row * wid + col]; + for (int row = 0; row < height; ++row) { + uint8_t* pScanline = pChannel + row * pitch; + for (int col = 0; col < width; ++col) { + uint8_t* pPixel = pScanline + col * image->numcomps; + int src = image->comps[channel].data[row * width + col]; src += image->comps[channel].sgnd ? 1 << (image->comps[channel].prec - 1) : 0; @@ -788,14 +783,14 @@ FX_BOOL CJPX_Decoder::Decode(uint8_t* dest_buf, } } } else { - for (row = 0; row < hei; row++) { - pScanline = pChannel + row * pitch; - for (col = 0; col < wid; col++) { - pPixel = pScanline + col * image->numcomps; + for (int row = 0; row < height; ++row) { + uint8_t* pScanline = pChannel + row * pitch; + for (int col = 0; col < width; ++col) { + uint8_t* pPixel = pScanline + col * image->numcomps; if (!image->comps[channel].data) { continue; } - src = image->comps[channel].data[row * wid + col]; + int src = image->comps[channel].data[row * width + col]; src += image->comps[channel].sgnd ? 1 << (image->comps[channel].prec - 1) : 0; @@ -815,12 +810,7 @@ FX_BOOL CJPX_Decoder::Decode(uint8_t* dest_buf, } } } - result = TRUE; - -done: - FX_Free(channel_bufs); - FX_Free(adjust_comps); - return result; + return true; } CCodec_JpxModule::CCodec_JpxModule() {} @@ -841,10 +831,10 @@ void CCodec_JpxModule::GetImageInfo(CJPX_Decoder* pDecoder, pDecoder->GetInfo(width, height, components); } -FX_BOOL CCodec_JpxModule::Decode(CJPX_Decoder* pDecoder, - uint8_t* dest_data, - int pitch, - uint8_t* offsets) { +bool CCodec_JpxModule::Decode(CJPX_Decoder* pDecoder, + uint8_t* dest_data, + int pitch, + const std::vector& offsets) { return pDecoder->Decode(dest_data, pitch, offsets); } -- cgit v1.2.3