[FFmpeg-devel] [PATCH] libavdevice/gdigrab: fix capture window title contain non-ASCII chars

[Jan Ekström]

On Sat, Mar 20, 2021 at 5:34 PM <1160386205 at qq.com> wrote:
>
> From: He Yang <1160386205 at qq.com>
>
> Signed-off-by: He Yang <1160386205 at qq.com>

Sorry for taking a while to respond, and thank you for the
contribution. I have verified that this conversion and FindWindowW
usage indeed fixes issues with non-ASCII window titles.

Before:
[gdigrab @ 000001d1f24b2cc0] Can't find window 'ジャンキーナイトタウンオーケストラ _
すりぃ feat.鏡音レン-sm36109943.mp4 - mpv', aborting.
title=ジャンキーナイトタウンオーケストラ _ すりぃ feat.鏡音レン-sm36109943.mp4 - mpv: I/O error

After:
[gdigrab @ 0000017d298b2cc0] Found window ジャンキーナイトタウンオーケストラ _  すりぃ
feat.鏡音レン-sm36109943.mp4 - mpv, capturing 1920x1080x32 at (0,0)
Input #0, gdigrab, from 'title=ジャンキーナイトタウンオーケストラ _ すりぃ
feat.鏡音レン-sm36109943.mp4 - mpv':

Now, taking things step-by-step, first from the most clear things:
1. FFmpeg utilizes C99 features, but follows the rule that no
declarations should happen after non-declaring code within a
scope/context.
src/libavdevice/gdigrab.c: In function 'gdigrab_read_header':
src/libavdevice/gdigrab.c:249:9: warning: ISO C90 forbids mixed
declarations and code [-Wdeclaration-after-statement]
  249 |         const wchar_t *name_w = NULL;
      |         ^~~~~

-> Basically fixed by moving the new wchar_t as the first thing in the
scope of that if branch.

2. Mismatch between function and the calling code in `const`ness.
Const things are nice, but in this case the function takes in a
non-const pointer.

src/libavdevice/gdigrab.c:250:30: warning: passing argument 2 of
'utf8towchar' from incompatible pointer type
[-Wincompatible-pointer-types]
  250 |         if(utf8towchar(name, &name_w)) {
      |                              ^~~~~~~
      |                              |
      |                              const wchar_t ** {aka const short
unsigned int **}
In file included from src/libavformat/os_support.h:148,
                 from src/libavformat/internal.h:28,
                 from src/libavdevice/gdigrab.c:32:
src/libavutil/wchar_filename.h:27:68: note: expected 'wchar_t **' {aka
'short unsigned int **'} but argument is of type 'const wchar_t **'
{aka 'const short unsigned int **'}
   27 | static inline int utf8towchar(const char *filename_utf8,
wchar_t **filename_w)
      |
~~~~~~~~~~^~~~~~~~~~

-> Fixed by removing the const from the wchar_t pointer.

Thus we move to actual review:

1. The libavutil header should be explicitly #included. That way users
of headers should be more easily find'able.
2. When utf8towchar returns nonzero, ret should probably be set to
AVERROR(errno). That way we are not re-guessing implementation
specifics of the function. (noticed by Martin)
3. Some whitespace would be good between the variable
declarations/setting, doing the conversion and finally the actual
window finding.

As I had to go through these points for the review process, I
basically posted a version with these changes @
https://github.com/jeeb/ffmpeg/commits/gdigrab_unicode_fix . I also
took the liberty of rewording the commit message somewhat. If you
think these changes are acceptable, then unless something new is
noticed, I consider this LGTM.

Best regards,
Jan