Merge pull request #41 from SDWebImage/performance_avoid_copy_progressive_data_and_use_lock

Fix the potential buffer crash issue during progressive animated webp decoding, and avoid extra copy for partial image data
This commit is contained in:
DreamPiggy 2020-04-13 13:18:50 +08:00 committed by GitHub
commit 20ab7d2b44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 23 additions and 33 deletions

View File

@ -86,7 +86,6 @@ static CGSize SDCalculateThumbnailSize(CGSize fullSize, BOOL preserveAspectRatio
@implementation SDImageWebPCoder { @implementation SDImageWebPCoder {
WebPIDecoder *_idec; WebPIDecoder *_idec;
WebPDemuxer *_demux; WebPDemuxer *_demux;
WebPData *_webpdata; // Copied for progressive animation demuxer
NSData *_imageData; NSData *_imageData;
CGFloat _scale; CGFloat _scale;
NSUInteger _loopCount; NSUInteger _loopCount;
@ -114,10 +113,6 @@ static CGSize SDCalculateThumbnailSize(CGSize fullSize, BOOL preserveAspectRatio
WebPDemuxDelete(_demux); WebPDemuxDelete(_demux);
_demux = NULL; _demux = NULL;
} }
if (_webpdata) {
WebPDataClear(_webpdata);
_webpdata = NULL;
}
if (_canvas) { if (_canvas) {
CGContextRelease(_canvas); CGContextRelease(_canvas);
_canvas = NULL; _canvas = NULL;
@ -305,41 +300,36 @@ static CGSize SDCalculateThumbnailSize(CGSize fullSize, BOOL preserveAspectRatio
if (_finished) { if (_finished) {
return; return;
} }
_imageData = data;
_finished = finished; _finished = finished;
if (!_demux) { // check whether we can detect Animated WebP or Static WebP, they need different codec (Demuxer or IDecoder)
VP8StatusCode status = WebPIUpdate(_idec, data.bytes, data.length); if (!_hasAnimation) {
if (status == VP8_STATUS_OK || status == VP8_STATUS_SUSPENDED) { _imageData = [data copy];
VP8StatusCode status = WebPIUpdate(_idec, _imageData.bytes, _imageData.length);
// For Static WebP, all things done.
// For Animated WebP (currently use `VP8_STATUS_UNSUPPORTED_FEATURE` to check), continue to create demuxer
if (status != VP8_STATUS_UNSUPPORTED_FEATURE) {
return; return;
} }
// This case may be Animated WebP progressive decode _hasAnimation = YES;
if (status == VP8_STATUS_UNSUPPORTED_FEATURE) { }
WebPDemuxState state; // libwebp current have no API to update demuxer, so we always delete and recreate demuxer
WebPData tmpData; // Use lock to avoid progressive animation decoding thread safe issue
WebPDataInit(&tmpData); SD_LOCK(_lock);
tmpData.bytes = data.bytes; if (_demux) {
tmpData.size = data.length; // next line `_imageData = nil` ARC will release the raw buffer, but need release the demuxer firstly because libwebp don't use retain/release rule
// Copy to avoid the NSData dealloc and VP8 internal retain the pointer
_webpdata = malloc(sizeof(WebPData));
WebPDataCopy(&tmpData, _webpdata);
_demux = WebPDemuxPartial(_webpdata, &state);
}
} else {
// libwebp current have no API to update demuxer, so we always delete and recreate demuxer
WebPDemuxDelete(_demux); WebPDemuxDelete(_demux);
_demux = NULL; _demux = NULL;
WebPDemuxState state;
WebPData tmpData;
WebPDataInit(&tmpData);
tmpData.bytes = data.bytes;
tmpData.size = data.length;
// Copy to avoid the NSData dealloc and VP8 internal retain the pointer
WebPDataClear(_webpdata);
WebPDataCopy(&tmpData, _webpdata);
_demux = WebPDemuxPartial(_webpdata, &state);
} }
_imageData = [data copy];
WebPData webpData;
WebPDataInit(&webpData);
webpData.bytes = _imageData.bytes;
webpData.size = _imageData.length;
WebPDemuxState state;
_demux = WebPDemuxPartial(&webpData, &state);
SD_UNLOCK(_lock);
if (_demux) { if (_demux && state != WEBP_DEMUX_PARSE_ERROR) {
[self scanAndCheckFramesValidWithDemuxer:_demux]; [self scanAndCheckFramesValidWithDemuxer:_demux];
} }
} }