Fix crash in XServerPixelBuffer.
XServerPixelBuffer contains x_image_ field, that may be allocated using XGetImage() or XShmCreateImage() depending how the last frame was captured. x_image_ is passed XShmGetImage(), which may crash if it gets image allocated with XGetImage(). Added x_shm_image_ to ensure that SHM and non-SHM capture paths are separate and XShmGetImage() is allways called with the correct XImage. The linked bug appears to be a regressiona after https://codereview.webrtc.org/2044693002 BUG=chromium:697823 Review-Url: https://codereview.webrtc.org/2796673002 Cr-Commit-Position: refs/heads/master@{#17518}
This commit is contained in:
parent
4eeb53748a
commit
0786c04f97
@ -18,6 +18,8 @@
|
||||
#include "webrtc/modules/desktop_capture/x11/x_error_trap.h"
|
||||
#include "webrtc/system_wrappers/include/logging.h"
|
||||
|
||||
namespace webrtc {
|
||||
|
||||
namespace {
|
||||
|
||||
// Returns the number of bits |mask| has to be shifted left so its last
|
||||
@ -55,9 +57,78 @@ bool IsXImageRGBFormat(XImage* image) {
|
||||
image->blue_mask == 0xff;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
// We expose two forms of blitting to handle variations in the pixel format.
|
||||
// In FastBlit(), the operation is effectively a memcpy.
|
||||
void FastBlit(XImage* x_image,
|
||||
uint8_t* src_pos,
|
||||
const DesktopRect& rect,
|
||||
DesktopFrame* frame) {
|
||||
int src_stride = x_image->bytes_per_line;
|
||||
int dst_x = rect.left(), dst_y = rect.top();
|
||||
|
||||
namespace webrtc {
|
||||
uint8_t* dst_pos = frame->data() + frame->stride() * dst_y;
|
||||
dst_pos += dst_x * DesktopFrame::kBytesPerPixel;
|
||||
|
||||
int height = rect.height();
|
||||
int row_bytes = rect.width() * DesktopFrame::kBytesPerPixel;
|
||||
for (int y = 0; y < height; ++y) {
|
||||
memcpy(dst_pos, src_pos, row_bytes);
|
||||
src_pos += src_stride;
|
||||
dst_pos += frame->stride();
|
||||
}
|
||||
}
|
||||
|
||||
void SlowBlit(XImage* x_image,
|
||||
uint8_t* src_pos,
|
||||
const DesktopRect& rect,
|
||||
DesktopFrame* frame) {
|
||||
int src_stride = x_image->bytes_per_line;
|
||||
int dst_x = rect.left(), dst_y = rect.top();
|
||||
int width = rect.width(), height = rect.height();
|
||||
|
||||
uint32_t red_mask = x_image->red_mask;
|
||||
uint32_t green_mask = x_image->red_mask;
|
||||
uint32_t blue_mask = x_image->blue_mask;
|
||||
|
||||
uint32_t red_shift = MaskToShift(red_mask);
|
||||
uint32_t green_shift = MaskToShift(green_mask);
|
||||
uint32_t blue_shift = MaskToShift(blue_mask);
|
||||
|
||||
int bits_per_pixel = x_image->bits_per_pixel;
|
||||
|
||||
uint8_t* dst_pos = frame->data() + frame->stride() * dst_y;
|
||||
dst_pos += dst_x * DesktopFrame::kBytesPerPixel;
|
||||
// TODO(hclam): Optimize, perhaps using MMX code or by converting to
|
||||
// YUV directly.
|
||||
// TODO(sergeyu): This code doesn't handle XImage byte order properly and
|
||||
// won't work with 24bpp images. Fix it.
|
||||
for (int y = 0; y < height; y++) {
|
||||
uint32_t* dst_pos_32 = reinterpret_cast<uint32_t*>(dst_pos);
|
||||
uint32_t* src_pos_32 = reinterpret_cast<uint32_t*>(src_pos);
|
||||
uint16_t* src_pos_16 = reinterpret_cast<uint16_t*>(src_pos);
|
||||
for (int x = 0; x < width; x++) {
|
||||
// Dereference through an appropriately-aligned pointer.
|
||||
uint32_t pixel;
|
||||
if (bits_per_pixel == 32) {
|
||||
pixel = src_pos_32[x];
|
||||
} else if (bits_per_pixel == 16) {
|
||||
pixel = src_pos_16[x];
|
||||
} else {
|
||||
pixel = src_pos[x];
|
||||
}
|
||||
uint32_t r = (pixel & red_mask) << red_shift;
|
||||
uint32_t g = (pixel & green_mask) << green_shift;
|
||||
uint32_t b = (pixel & blue_mask) << blue_shift;
|
||||
// Write as 32-bit RGB.
|
||||
dst_pos_32[x] =
|
||||
((r >> 8) & 0xff0000) | ((g >> 16) & 0xff00) | ((b >> 24) & 0xff);
|
||||
}
|
||||
dst_pos += frame->stride();
|
||||
src_pos += src_stride;
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
XServerPixelBuffer::XServerPixelBuffer() {}
|
||||
|
||||
@ -70,6 +141,10 @@ void XServerPixelBuffer::Release() {
|
||||
XDestroyImage(x_image_);
|
||||
x_image_ = nullptr;
|
||||
}
|
||||
if (x_shm_image_) {
|
||||
XDestroyImage(x_shm_image_);
|
||||
x_shm_image_ = nullptr;
|
||||
}
|
||||
if (shm_pixmap_) {
|
||||
XFreePixmap(display_, shm_pixmap_);
|
||||
shm_pixmap_ = 0;
|
||||
@ -131,18 +206,18 @@ void XServerPixelBuffer::InitShm(const XWindowAttributes& attributes) {
|
||||
shm_segment_info_->shmid = -1;
|
||||
shm_segment_info_->shmaddr = nullptr;
|
||||
shm_segment_info_->readOnly = False;
|
||||
x_image_ = XShmCreateImage(display_, default_visual, default_depth, ZPixmap,
|
||||
0, shm_segment_info_, window_size_.width(),
|
||||
window_size_.height());
|
||||
if (x_image_) {
|
||||
x_shm_image_ = XShmCreateImage(display_, default_visual, default_depth,
|
||||
ZPixmap, 0, shm_segment_info_,
|
||||
window_size_.width(), window_size_.height());
|
||||
if (x_shm_image_) {
|
||||
shm_segment_info_->shmid =
|
||||
shmget(IPC_PRIVATE, x_image_->bytes_per_line * x_image_->height,
|
||||
shmget(IPC_PRIVATE, x_shm_image_->bytes_per_line * x_shm_image_->height,
|
||||
IPC_CREAT | 0600);
|
||||
if (shm_segment_info_->shmid != -1) {
|
||||
void* shmat_result = shmat(shm_segment_info_->shmid, 0, 0);
|
||||
if (shmat_result != reinterpret_cast<void*>(-1)) {
|
||||
shm_segment_info_->shmaddr = reinterpret_cast<char*>(shmat_result);
|
||||
x_image_->data = shm_segment_info_->shmaddr;
|
||||
x_shm_image_->data = shm_segment_info_->shmaddr;
|
||||
|
||||
XErrorTrap error_trap(display_);
|
||||
using_shm = XShmAttach(display_, shm_segment_info_);
|
||||
@ -235,7 +310,7 @@ void XServerPixelBuffer::Synchronize() {
|
||||
XErrorTrap error_trap(display_);
|
||||
// XShmGetImage fails if the window is partially out of screen.
|
||||
xshm_get_image_succeeded_ =
|
||||
XShmGetImage(display_, window_, x_image_, 0, 0, AllPlanes);
|
||||
XShmGetImage(display_, window_, x_shm_image_, 0, 0, AllPlanes);
|
||||
}
|
||||
}
|
||||
|
||||
@ -244,6 +319,7 @@ bool XServerPixelBuffer::CaptureRect(const DesktopRect& rect,
|
||||
assert(rect.right() <= window_size_.width());
|
||||
assert(rect.bottom() <= window_size_.height());
|
||||
|
||||
XImage* image;
|
||||
uint8_t* data;
|
||||
|
||||
if (shm_segment_info_ && (shm_pixmap_ || xshm_get_image_succeeded_)) {
|
||||
@ -253,9 +329,12 @@ bool XServerPixelBuffer::CaptureRect(const DesktopRect& rect,
|
||||
rect.left(), rect.top());
|
||||
XSync(display_, False);
|
||||
}
|
||||
data = reinterpret_cast<uint8_t*>(x_image_->data) +
|
||||
rect.top() * x_image_->bytes_per_line +
|
||||
rect.left() * x_image_->bits_per_pixel / 8;
|
||||
|
||||
image = x_shm_image_;
|
||||
data = reinterpret_cast<uint8_t*>(image->data) +
|
||||
rect.top() * image->bytes_per_line +
|
||||
rect.left() * image->bits_per_pixel / 8;
|
||||
|
||||
} else {
|
||||
if (x_image_)
|
||||
XDestroyImage(x_image_);
|
||||
@ -264,85 +343,17 @@ bool XServerPixelBuffer::CaptureRect(const DesktopRect& rect,
|
||||
if (!x_image_)
|
||||
return false;
|
||||
|
||||
data = reinterpret_cast<uint8_t*>(x_image_->data);
|
||||
image = x_image_;
|
||||
data = reinterpret_cast<uint8_t*>(image->data);
|
||||
}
|
||||
|
||||
if (IsXImageRGBFormat(x_image_)) {
|
||||
FastBlit(data, rect, frame);
|
||||
if (IsXImageRGBFormat(image)) {
|
||||
FastBlit(image, data, rect, frame);
|
||||
} else {
|
||||
SlowBlit(data, rect, frame);
|
||||
SlowBlit(image, data, rect, frame);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
void XServerPixelBuffer::FastBlit(uint8_t* image,
|
||||
const DesktopRect& rect,
|
||||
DesktopFrame* frame) {
|
||||
uint8_t* src_pos = image;
|
||||
int src_stride = x_image_->bytes_per_line;
|
||||
int dst_x = rect.left(), dst_y = rect.top();
|
||||
|
||||
uint8_t* dst_pos = frame->data() + frame->stride() * dst_y;
|
||||
dst_pos += dst_x * DesktopFrame::kBytesPerPixel;
|
||||
|
||||
int height = rect.height();
|
||||
int row_bytes = rect.width() * DesktopFrame::kBytesPerPixel;
|
||||
for (int y = 0; y < height; ++y) {
|
||||
memcpy(dst_pos, src_pos, row_bytes);
|
||||
src_pos += src_stride;
|
||||
dst_pos += frame->stride();
|
||||
}
|
||||
}
|
||||
|
||||
void XServerPixelBuffer::SlowBlit(uint8_t* image,
|
||||
const DesktopRect& rect,
|
||||
DesktopFrame* frame) {
|
||||
int src_stride = x_image_->bytes_per_line;
|
||||
int dst_x = rect.left(), dst_y = rect.top();
|
||||
int width = rect.width(), height = rect.height();
|
||||
|
||||
uint32_t red_mask = x_image_->red_mask;
|
||||
uint32_t green_mask = x_image_->red_mask;
|
||||
uint32_t blue_mask = x_image_->blue_mask;
|
||||
|
||||
uint32_t red_shift = MaskToShift(red_mask);
|
||||
uint32_t green_shift = MaskToShift(green_mask);
|
||||
uint32_t blue_shift = MaskToShift(blue_mask);
|
||||
|
||||
int bits_per_pixel = x_image_->bits_per_pixel;
|
||||
|
||||
uint8_t* dst_pos = frame->data() + frame->stride() * dst_y;
|
||||
uint8_t* src_pos = image;
|
||||
dst_pos += dst_x * DesktopFrame::kBytesPerPixel;
|
||||
// TODO(hclam): Optimize, perhaps using MMX code or by converting to
|
||||
// YUV directly.
|
||||
// TODO(sergeyu): This code doesn't handle XImage byte order properly and
|
||||
// won't work with 24bpp images. Fix it.
|
||||
for (int y = 0; y < height; y++) {
|
||||
uint32_t* dst_pos_32 = reinterpret_cast<uint32_t*>(dst_pos);
|
||||
uint32_t* src_pos_32 = reinterpret_cast<uint32_t*>(src_pos);
|
||||
uint16_t* src_pos_16 = reinterpret_cast<uint16_t*>(src_pos);
|
||||
for (int x = 0; x < width; x++) {
|
||||
// Dereference through an appropriately-aligned pointer.
|
||||
uint32_t pixel;
|
||||
if (bits_per_pixel == 32) {
|
||||
pixel = src_pos_32[x];
|
||||
} else if (bits_per_pixel == 16) {
|
||||
pixel = src_pos_16[x];
|
||||
} else {
|
||||
pixel = src_pos[x];
|
||||
}
|
||||
uint32_t r = (pixel & red_mask) << red_shift;
|
||||
uint32_t g = (pixel & green_mask) << green_shift;
|
||||
uint32_t b = (pixel & blue_mask) << blue_shift;
|
||||
// Write as 32-bit RGB.
|
||||
dst_pos_32[x] = ((r >> 8) & 0xff0000) | ((g >> 16) & 0xff00) |
|
||||
((b >> 24) & 0xff);
|
||||
}
|
||||
dst_pos += frame->stride();
|
||||
src_pos += src_stride;
|
||||
}
|
||||
}
|
||||
|
||||
} // namespace webrtc
|
||||
|
||||
@ -63,20 +63,12 @@ class XServerPixelBuffer {
|
||||
void InitShm(const XWindowAttributes& attributes);
|
||||
bool InitPixmaps(int depth);
|
||||
|
||||
// We expose two forms of blitting to handle variations in the pixel format.
|
||||
// In FastBlit(), the operation is effectively a memcpy.
|
||||
void FastBlit(uint8_t* image,
|
||||
const DesktopRect& rect,
|
||||
DesktopFrame* frame);
|
||||
void SlowBlit(uint8_t* image,
|
||||
const DesktopRect& rect,
|
||||
DesktopFrame* frame);
|
||||
|
||||
Display* display_ = nullptr;
|
||||
Window window_ = 0;
|
||||
DesktopSize window_size_;
|
||||
XImage* x_image_ = nullptr;
|
||||
XShmSegmentInfo* shm_segment_info_ = nullptr;
|
||||
XImage* x_shm_image_ = nullptr;
|
||||
Pixmap shm_pixmap_ = 0;
|
||||
GC shm_gc_ = nullptr;
|
||||
bool xshm_get_image_succeeded_ = false;
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user