[FFmpeg-devel] PATCH: gdigrab work for DPI in windows

[Roger Pack]

On 10/28/15, Matt Oliver <protogonoi at gmail.com> wrote:
> On 5 October 2015 at 20:05, Matt Oliver <protogonoi at gmail.com> wrote:
>
>> On 5 October 2015 at 19:29, Matt Oliver <protogonoi at gmail.com> wrote:
>>
>>> On 30 September 2015 at 02:48, wm4 <nfxjfg at googlemail.com> wrote:
>>>
>>>> On Wed, 23 Sep 2015 12:04:43 -0600
>>>> Roger Pack <rogerdpack2 at gmail.com> wrote:
>>>>
>>>> > From 6a972dda58bd5ab31524cd4e5326b4bcdeaeaa8c Mon Sep 17 00:00:00 2001
>>>> > From: rogerdpack <rogerpack2005 at gmail.com>
>>>> > Date: Wed, 23 Sep 2015 12:03:27 -0600
>>>> > Subject: [PATCH] gdigrab: grab right desktop size if DPI in use, based
>>>> on
>>>> >  patch from Alexander Brotzge
>>>> >
>>>> > Signed-off-by: rogerdpack <rogerpack2005 at gmail.com>
>>>> > ---
>>>> >  libavdevice/gdigrab.c | 44
>>>> +++++++++++++++++++++++++++++---------------
>>>> >  1 file changed, 29 insertions(+), 15 deletions(-)
>>>> >
>>>> > diff --git a/libavdevice/gdigrab.c b/libavdevice/gdigrab.c
>>>> > index 9a185d4..b0faf45 100644
>>>> > --- a/libavdevice/gdigrab.c
>>>> > +++ b/libavdevice/gdigrab.c
>>>> > @@ -235,6 +235,9 @@ gdigrab_read_header(AVFormatContext *s1)
>>>> >      AVStream   *st       = NULL;
>>>> >
>>>> >      int bpp;
>>>> > +    int vertres;
>>>> > +    int desktopvertres;
>>>> > +    float scale;
>>>> >      RECT virtual_rect;
>>>> >      RECT clip_rect;
>>>> >      BITMAP bmp;
>>>> > @@ -263,14 +266,34 @@ gdigrab_read_header(AVFormatContext *s1)
>>>> >          goto error;
>>>> >      }
>>>> >
>>>> > -    if (hwnd) {
>>>> > -        GetClientRect(hwnd, &virtual_rect);
>>>> > -    } else {
>>>> > +    /* This will get the device context for the selected window, or
>>>> > if
>>>> > +     * none, the primary screen */
>>>> > +    source_hdc = GetDC(hwnd);
>>>> > +    if (!source_hdc) {
>>>> > +        WIN32_API_ERROR("Couldn't get window device context");
>>>> > +        ret = AVERROR(EIO);
>>>> > +        goto error;
>>>> > +    }
>>>> > +    bpp = GetDeviceCaps(source_hdc, BITSPIXEL);
>>>> > +
>>>> > +    scale = 1.0;
>>>> > +    if (hwnd == NULL) {
>>>> > +      /* desktop -- get the right height and width for scaling DPI */
>>>> > +      vertres = GetDeviceCaps(source_hdc, VERTRES);
>>>> > +      desktopvertres = GetDeviceCaps(source_hdc, DESKTOPVERTRES);
>>>> > +      scale = (float) desktopvertres / (float) vertres;
>>>> > +    }
>>>>
>>>
>>> This seems a little redundant as scale is only being set when hwnd is
>>> null however in the below lines its then being used in the opposite
>>> conditional. Since this checks for hwnd essentially being non null then
>>> scale is always going to be the inital value of 1.
>>>
>>>
>>>> > +     if (hwnd) {
>>>> > +         GetClientRect(hwnd, &virtual_rect);
>>>> > +         virtual_rect.right = virtual_rect.right * scale;
>>>> > +         virtual_rect.bottom = virtual_rect.bottom * scale;
>>>> > +     } else {
>>>> >          virtual_rect.left = GetSystemMetrics(SM_XVIRTUALSCREEN);
>>>> >          virtual_rect.top = GetSystemMetrics(SM_YVIRTUALSCREEN);
>>>> > -        virtual_rect.right = virtual_rect.left +
>>>> GetSystemMetrics(SM_CXVIRTUALSCREEN);
>>>> > -        virtual_rect.bottom = virtual_rect.top +
>>>> GetSystemMetrics(SM_CYVIRTUALSCREEN);
>>>> > -    }
>>>> > +        virtual_rect.right = (virtual_rect.left +
>>>> GetSystemMetrics(SM_CXVIRTUALSCREEN)) * scale;
>>>> > +        virtual_rect.bottom = (virtual_rect.top +
>>>> GetSystemMetrics(SM_CYVIRTUALSCREEN)) * scale;
>>>> > +     }
>>>>
>>>
>>> This if else could be combined with the above if as scale is only a value
>>> different to 1 in the 'else' case. Also as wm4 said it would probably be
>>> better to keep the scale factors as ints.
>>>
>>>
>>>> >
>>>> >      /* If no width or height set, use full screen/window area */
>>>> >      if (!gdigrab->width || !gdigrab->height) {
>>>> > @@ -299,15 +322,6 @@ gdigrab_read_header(AVFormatContext *s1)
>>>> >              goto error;
>>>> >      }
>>>> >
>>>> > -    /* This will get the device context for the selected window, or
>>>> > if
>>>> > -     * none, the primary screen */
>>>> > -    source_hdc = GetDC(hwnd);
>>>> > -    if (!source_hdc) {
>>>> > -        WIN32_API_ERROR("Couldn't get window device context");
>>>> > -        ret = AVERROR(EIO);
>>>> > -        goto error;
>>>> > -    }
>>>> > -    bpp = GetDeviceCaps(source_hdc, BITSPIXEL);
>>>> >
>>>> >      if (name) {
>>>> >          av_log(s1, AV_LOG_INFO,
>>>
>>>
>>> However I think the basic dpi scaling technique is correct. However I
>>> would think it should be more along the lines of the modifications Ive
>>> attached.
>>>
>>
>> Woops, allow me to try that again without windows messing up the patches
>> line endings.
>>
>
> Does anyone have any issues with the updated patch or can it be pushed?

OK sorry for the delay finally got around to testing it and LGTM
recommend commit (I'm a gdigrab maintainer).
Cheers!
-roger-