From 25cc8ad198731ff8eee35859d82919949afbdff7 Mon Sep 17 00:00:00 2001 From: David Porter Date: Mon, 23 Jul 2018 12:50:33 -0700 Subject: [PATCH] Fixed issue with BGRA RTCCVPixelBuffer scale and crop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BGRA RTCCVPixelBuffers were cropped and scaled incorrectly. Libyuv’s `ARGBScale` method is used in RTCCVPixelBuffer to scale and crop the pixel buffer. To crop by `cropX` and `cropY` pixels, pointer arithmetic is used to offset the src pointer of the original pixel buffer bytes. There is a bug in how this offset is calculated. The offset is done by `src += srcStride * _cropY + _cropX`. Libyuv expects that the src pointer will point to the start of a new pixel. However, if _cropX is a not a multiple of 4 (4 bytes for BGRA), the src pointer will point to a byte in the middle of a pixel and thus libyuv will incorrectly treat the data as the start of pixel (incorrectly treating the first byte as red when it is actually green, etc...). To fix this, the src pointer needs to be offset to always point to the start of a new pixel. Before this change: Original Test Gradient image with a cropX of 2: https://i.imgur.com/gSIgwGV.jpg Scaled image (notice the colors are incorrect): https://i.imgur.com/oPxbTEK.jpg After this change: Scaled image (notice the colors are correct): https://i.imgur.com/dqBsmsH.jpg A new unit test which tests scaling with cropX and cropY values has been added. The test fails without this change and now passes with the correct src pointer offsetting. Bug: webrtc:9555 Change-Id: I87cbd7b91bc139d51fb4e11cc50ccb014cfa8051 Reviewed-on: https://webrtc-review.googlesource.com/89220 Commit-Queue: Kári Helgason Reviewed-by: Kári Helgason Cr-Commit-Position: refs/heads/master@{#24076} --- AUTHORS | 1 + .../Classes/Video/RTCCVPixelBuffer.mm | 8 +++++-- .../UnitTests/RTCCVPixelBuffer_xctest.mm | 22 ++++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index af1688f035..69b5e2fc4a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -14,6 +14,7 @@ Chris Tserng Christophe Dumez Cody Barnes Colin Plumb +David Porter Dax Booysen Dmitry Lizin Eric Rescorla, RTFM Inc. diff --git a/sdk/objc/Framework/Classes/Video/RTCCVPixelBuffer.mm b/sdk/objc/Framework/Classes/Video/RTCCVPixelBuffer.mm index f854c91edc..9e5ac7320b 100644 --- a/sdk/objc/Framework/Classes/Video/RTCCVPixelBuffer.mm +++ b/sdk/objc/Framework/Classes/Video/RTCCVPixelBuffer.mm @@ -323,8 +323,12 @@ const uint8_t* src = static_cast(CVPixelBufferGetBaseAddress(_pixelBuffer)); const int srcStride = CVPixelBufferGetBytesPerRow(_pixelBuffer); - // Crop just by modifying pointers. - src += srcStride * _cropY + _cropX; + // Crop just by modifying pointers. Need to ensure that src pointer points to a byte corresponding + // to the start of a new pixel (byte with B for BGRA) so that libyuv scales correctly. + const int bytesPerPixel = 4; + src += srcStride * _cropY + (_cropX * bytesPerPixel); + + // kCVPixelFormatType_32BGRA corresponds to libyuv::FOURCC_ARGB libyuv::ARGBScale(src, srcStride, _cropWidth, diff --git a/sdk/objc/Framework/UnitTests/RTCCVPixelBuffer_xctest.mm b/sdk/objc/Framework/UnitTests/RTCCVPixelBuffer_xctest.mm index a6200dca5a..834bb222b9 100644 --- a/sdk/objc/Framework/UnitTests/RTCCVPixelBuffer_xctest.mm +++ b/sdk/objc/Framework/UnitTests/RTCCVPixelBuffer_xctest.mm @@ -153,6 +153,14 @@ [self cropAndScaleTestWithRGBPixelFormat:kCVPixelFormatType_32ARGB]; } +- (void)testCropAndScaleWithSmallCropInfo_32ARGB { + [self cropAndScaleTestWithRGBPixelFormat:kCVPixelFormatType_32ARGB cropX:2 cropY:3]; +} + +- (void)testCropAndScaleWithLargeCropInfo_32ARGB { + [self cropAndScaleTestWithRGBPixelFormat:kCVPixelFormatType_32ARGB cropX:200 cropY:300]; +} + - (void)testToI420_NV12 { [self toI420WithPixelFormat:kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange]; } @@ -224,12 +232,24 @@ } - (void)cropAndScaleTestWithRGBPixelFormat:(OSType)pixelFormat { + [self cropAndScaleTestWithRGBPixelFormat:pixelFormat cropX:0 cropY:0]; +} + +- (void)cropAndScaleTestWithRGBPixelFormat:(OSType)pixelFormat cropX:(int)cropX cropY:(int)cropY { CVPixelBufferRef pixelBufferRef = NULL; CVPixelBufferCreate(NULL, 720, 1280, pixelFormat, NULL, &pixelBufferRef); DrawGradientInRGBPixelBuffer(pixelBufferRef); - RTCCVPixelBuffer *buffer = [[RTCCVPixelBuffer alloc] initWithPixelBuffer:pixelBufferRef]; + RTCCVPixelBuffer *buffer = + [[RTCCVPixelBuffer alloc] initWithPixelBuffer:pixelBufferRef + adaptedWidth:CVPixelBufferGetWidth(pixelBufferRef) + adaptedHeight:CVPixelBufferGetHeight(pixelBufferRef) + cropWidth:CVPixelBufferGetWidth(pixelBufferRef) - cropX + cropHeight:CVPixelBufferGetHeight(pixelBufferRef) - cropY + cropX:cropX + cropY:cropY]; + XCTAssertEqual(buffer.width, 720); XCTAssertEqual(buffer.height, 1280);