diff mbox

[FFmpeg-devel] x11grab: fix vertical repositioning

Message ID 20190328045232.3720-1-octalffdev@alvarezp.org
State Accepted
Commit f4f40cbb578a09319f9ddafc80388a5556ec7713
Headers show

Commit Message

Octavio Alvarez March 28, 2019, 4:52 a.m. UTC 
There is a calculation error in xcbgrab_reposition() that breaks
vertical repositioning on follow_mouse. It made the bottom
reposition occur when moving the mouse lower than N pixels after
the capture bottom edge, instead of before.

This commit fixes the calculation to match the documentation.

follow_mouse: centered or number of pixels. The documentation says:

When it is specified with "centered", the grabbing region follows
the mouse pointer and keeps the pointer at the center of region;
otherwise, the region follows only when the mouse pointer reaches
within PIXELS (greater than zero) to the edge of region.

The patch is untested, but it looks fairly straightforward.

---
 libavdevice/xcbgrab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Carl Eugen Hoyos April 1, 2019, 1:41 p.m. UTC | #1 
2019-03-28 5:52 GMT+01:00, Octavio Alvarez <octalffdev@alvarezp.org>:
> There is a calculation error in xcbgrab_reposition() that breaks
> vertical repositioning on follow_mouse. It made the bottom
> reposition occur when moving the mouse lower than N pixels after
> the capture bottom edge, instead of before.
>
> This commit fixes the calculation to match the documentation.
>
> follow_mouse: centered or number of pixels. The documentation says:
>
> When it is specified with "centered", the grabbing region follows
> the mouse pointer and keeps the pointer at the center of region;
> otherwise, the region follows only when the mouse pointer reaches
> within PIXELS (greater than zero) to the edge of region.

> The patch is untested, but it looks fairly straightforward.

How can this patch be tested?

Carl Eugen
Octavio Alvarez April 1, 2019, 5:17 p.m. UTC | #2 
On 4/1/19 7:41 AM, Carl Eugen Hoyos wrote:
>> The patch is untested, but it looks fairly straightforward.
> 
> How can this patch be tested?

ffmpeg -y -r 30 -f x11grab -show_region 1 -draw_mouse 0 -s 350x614
-follow_mouse 10 -i :0.0+0,0 -r 30 -preset ultrafast followcapture-test-.mkv

A dotted frame will appear. Try to move the mouse outside of the frame.
Cancel with Ctrl+C when needed.

Without the patch the frame follows the mouse in the up, left and right
direction 10 pixels before the mouse exits the frame. This is correct.
However, for the bottom direction, the frame follows 10 pixels *after*
the mouse exits the frame, which is wrong.

With the patch this should be fixed: for the bottom direction it should
also follow the mouse 10 pixels before the pointer exits the frame.

The "10" pixels can be adjusted in the -follow_mouse 10 argument.

Watching the output file should not be needed. It's the mouse-following
behavior that has the bug.

Octavio.
Octavio Alvarez April 4, 2019, 1:55 p.m. UTC | #3 
On 4/1/19 11:17 AM, Octavio Alvarez wrote:
> On 4/1/19 7:41 AM, Carl Eugen Hoyos wrote:
>>> The patch is untested, but it looks fairly straightforward.
>>
>> How can this patch be tested?
> 
> ffmpeg -y -r 30 -f x11grab -show_region 1 -draw_mouse 0 -s 350x614
> -follow_mouse 10 -i :0.0+0,0 -r 30 -preset ultrafast followcapture-test-.mkv

I just tested the patch and it properly works.

Octavio.
diff mbox

Patch

diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
index 6d142abd4f..b7e689343e 100644
--- a/libavdevice/xcbgrab.c
+++ b/libavdevice/xcbgrab.c
@@ -127,7 +127,7 @@  static int xcbgrab_reposition(AVFormatContext *s,
         int left   = x + f;
         int right  = x + w - f;
         int top    = y + f;
-        int bottom = y + h + f;
+        int bottom = y + h - f;
         if (p_x > right) {
             x += p_x - right;
         } else if (p_x < left) {