On Sun, Aug 24, 2014 at 02:18:18PM +0200, Luca Barbato wrote:
> --- a/libavdevice/x11grab.c
> +++ b/libavdevice/x11grab.c
> @@ -91,16 +91,13 @@ struct x11grab {
> -static void
> -x11grab_draw_region_win(struct x11grab *s)
> +static void x11grab_draw_region_win(struct x11grab *s)
> {
> Display *dpy = s->dpy;
> - int screen;
> - Window win = s->region_win;
> - GC gc;
> + Window win = s->region_win;
> + int screen = DefaultScreen(dpy);
> + GC gc = XCreateGC(dpy, win, 0, 0);
>
> - screen = DefaultScreen(dpy);
> - gc = XCreateGC(dpy, win, 0, 0);
You're also merging declarations and initializations; at the very least
I'd mention that in the log message.
> @@ -116,26 +113,24 @@ x11grab_draw_region_win(struct x11grab *s)
> attribs.override_redirect = True;
> - s->region_win = XCreateWindow(dpy, RootWindow(dpy, screen),
> - s->x_off - REGION_WIN_BORDER,
> - s->y_off - REGION_WIN_BORDER,
> - s->width + REGION_WIN_BORDER * 2,
> - s->height + REGION_WIN_BORDER * 2,
> - 0, CopyFromParent,
> - InputOutput, CopyFromParent,
> - CWOverrideRedirect, &attribs);
> + s->region_win = XCreateWindow(dpy, RootWindow(dpy, screen),
> + s->x_off - REGION_WIN_BORDER,
> + s->y_off - REGION_WIN_BORDER,
> + s->width + REGION_WIN_BORDER *
> 2,
> + s->height + REGION_WIN_BORDER
> * 2,
> + 0, CopyFromParent,
> + InputOutput, CopyFromParent,
> + CWOverrideRedirect, &attribs);
These assignments are unrelated, you don't necessarily have to align them.
Maintain the alignment of + and - at least.
> @@ -180,22 +174,26 @@ x11grab_read_header(AVFormatContext *s1)
>
> - if ((ret = av_parse_video_size(&x11grab->width, &x11grab->height,
> x11grab->video_size)) < 0) {
> + ret = av_parse_video_size(&x11grab->width, &x11grab->height,
> + x11grab->video_size);
> + if (ret < 0) {
> av_log(s1, AV_LOG_ERROR, "Couldn't parse video size.\n");
> goto out;
> }
This also changes the logic hidden in a K&R patch.
> @@ -216,18 +214,21 @@ x11grab_read_header(AVFormatContext *s1)
>
> use_shm = XShmQueryExtension(dpy);
> - av_log(s1, AV_LOG_INFO, "shared memory extension %s found\n", use_shm ?
> "" : "not");
> + av_log(s1, AV_LOG_INFO, "shared memory extension %sfound\n", use_shm ?
> "" : "not ");
typo
> @@ -255,46 +256,54 @@ x11grab_read_header(AVFormatContext *s1)
> case 16:
> - if ( image->red_mask == 0xf800 &&
> - image->green_mask == 0x07e0 &&
> - image->blue_mask == 0x001f ) {
> + if (image->red_mask == 0xf800 &&
> + image->green_mask == 0x07e0 &&
> + image->blue_mask == 0x001f) {
Maintain the alignment.
> - } else if (image->red_mask == 0x7c00 &&
> + } else if (image->red_mask == 0x7c00 &&
> image->green_mask == 0x03e0 &&
> - image->blue_mask == 0x001f ) {
> + image->blue_mask == 0x001f) {
same
> case 24:
> - if ( image->red_mask == 0xff0000 &&
> - image->green_mask == 0x00ff00 &&
> - image->blue_mask == 0x0000ff ) {
> + if (image->red_mask == 0xff0000 &&
> + image->green_mask == 0x00ff00 &&
> + image->blue_mask == 0x0000ff) {
same
> - } else if ( image->red_mask == 0x0000ff &&
> - image->green_mask == 0x00ff00 &&
> - image->blue_mask == 0xff0000 ) {
> + } else if (image->red_mask == 0x0000ff &&
> + image->green_mask == 0x00ff00 &&
> + image->blue_mask == 0xff0000) {
same
> @@ -365,36 +375,34 @@ paint_mouse_pointer(XImage *image, struct x11grab *s)
>
> - for (line = FFMAX(y, y_off); line < to_line; line++) {
> + for (line = FFMAX(y, y_off); line < to_line; line++)
> for (column = FFMAX(x, x_off); column < to_column; column++) {
> - int r = (uint8_t)(xcim->pixels[xcim_addr] >> 0);
> - int g = (uint8_t)(xcim->pixels[xcim_addr] >> 8);
> - int b = (uint8_t)(xcim->pixels[xcim_addr] >> 16);
> - int a = (uint8_t)(xcim->pixels[xcim_addr] >> 24);
> + int r = (uint8_t) (xcim->pixels[xcim_addr] >> 0);
> + int g = (uint8_t) (xcim->pixels[xcim_addr] >> 8);
> + int b = (uint8_t) (xcim->pixels[xcim_addr] >> 16);
> + int a = (uint8_t) (xcim->pixels[xcim_addr] >> 24);
The right-align of the shifts looked slightly better.
> }
> }
> - }
Maintain the {} around the outer for-loop.
> @@ -526,9 +532,10 @@ x11grab_read_packet(AVFormatContext *s1, AVPacket *pkt)
>
> if (s->show_region) {
> if (s->region_win) {
> - XEvent evt;
> - // clean up the events, and do the initinal draw or redraw.
> - for (evt.type = NoEventMask; XCheckMaskEvent(dpy, ExposureMask |
> StructureNotifyMask, &evt); );
> + XEvent evt = { .type = NoEventMask };
> + // clean up the events, and do the initial draw or redraw.
> + while (XCheckMaskEvent(dpy, ExposureMask | StructureNotifyMask,
> + &evt));
Again, this changes logic.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
