rosell-dk / webp-express

Wordpress plugin for serving autogenerated WebP images instead of jpeg/png to browsers that supports WebP
GNU General Public License v3.0
226 stars 64 forks source link

Bundled cwebp for windows-x64 doesn't support Unicode #400

Open Ruzgfpegk opened 4 years ago

Ruzgfpegk commented 4 years ago

From a .md log of an image:

The following options have been set explicitly. Note: it is the resulting options after merging down the "jpeg" and "png" options and any converter-prefixed options.

  • source: C:\path\to\DocumentRoot/wp-content/uploads/2013/06/痛ブロ 宣伝画像-150x150.jpg

  • destination: C:\path\to\DocumentRoot/wp-content/webp-express/webp-images/doc-root/wp-content\uploads\2013\06\痛ブロ 宣伝画像-150x150.jpg.webp

[...]

Trying to convert by executing the following command:

C:\path\to\DocumentRoot\wp-content\plugins\webp-express\vendor\rosell-dk\webp-convert\src\Convert\Converters\Binaries\cwebp-103-windows-x64.exe -metadata none -q 80 -alpha_q "85" -m 6 -low_memory "C:\path\to\DocumentRoot/wp-content/uploads/2013/06/痛ブロ 宣伝画像-150x150.jpg" -o "C:\path\to\DocumentRoot/wp-content/webp-express/webp-images/doc-root/wp-content\uploads\2013\06\痛ブロ 宣伝画像-150x150.jpg.webp.lossy.webp" 2>&1

Output:

SHCreateStreamOnFile((const LPTSTR)filename, STGM_READ, stream) failed 8007007b

Error opening input file C:\path\to\DocumentRoot/wp-content/uploads/2013/06/??? ????-150x150.jpg (8007007b)

OpenInputStream(filename, &stream) failed 8007007b

cannot open input file 'C:\path\to\DocumentRoot/wp-content/uploads/2013/06/??? ????-150x150.jpg'

Error! Could not process file C:\path\to\DocumentRoot/wp-content/uploads/2013/06/??? ????-150x150.jpg

Error! Cannot read input picture file 'C:\path\to\DocumentRoot/wp-content/uploads/2013/06/??? ????-150x150.jpg'

Exec failed (return code: -1)

And then the process works out in the end using Gd as a failsafe.

The fact that both separator types are used in the path could be noted as another theoretical issue but that doesn't break things in practice so...

From the README of the libwebp project:

Use option -DWEBP_UNICODE=ON for Unicode support on Windows (with chcp 65001).

According to the relevant ticket at the webp project :

Unicode-compatible binaries will be available in a future release of the precompiled utilities. In the meantime this feature is enabled by default on windows targets when using cmake (-DWEBP_UNICODE=1). With Makefile.vc you can pass UNICODE=1 to the nmake invocation.

Even the latest official precompiled binaries 1.1.0 don't come with Unicode enabled by default.

Maybe a custom build would be a good option? In this case I suppose you wouldn't trust anyone blindly throwing a compiled executable into the project? If that's the case then at least a warning should be added somewhere for those who only want to use cwebp, to invite them to compile their own Unicode-enabled cwebp if they need it.

rosell-dk commented 4 years ago

@Ruzgfpegk. Yes, a custom build would be an excellent option. I do however not have Windows nor any practice in compiling on Windows. I bet you do? Perhaps we could solve the trust-issue somehow?

Ruzgfpegk commented 4 years ago

Yes, I already compiled such a version.

The IDE I'm using for C/C++ projects, CLion, supports CMake and can use directly the toolchain bundled with the free Visual Studio Community releases, so cwebp could be built quite quickly with those, and as stated above the libwebp CMake config already enables Unicode by default in Windows.

The resulting binary works as expected in CLI:

C:\path\to\DocumentRoot\wp-content\plugins\webp-express\vendor\rosell-dk\webp-convert\src\Convert\Converters\Binaries\cwebp-110-windows-x64-unicode.exe -metadata none -q 80 -alpha_q "85" -m 6 -low_memory "C:\path\to\DocumentRoot/wp-content/uploads/2013/06/痛ブロ 宣伝画像-150x150.jpg" -o "C:\path\to\DocumentRoot/wp-content/webp-express/webp-images/doc-root/wp-content\uploads\2013\06\痛ブ ロ 宣伝画像-150x150.jpg.webp.lossy.webp" 2>&1 Saving file 'C:\path\to\DocumentRoot/wp-content/webp-express/webp-images/doc-root/wp-content\uploads\2013\06\????????-150x150.jpg.webp.lossy.webp' File: C:\path\to\DocumentRoot/wp-content/uploads/2013/06/????????-150x150.jpg ...

Through the plugin, the conversion log also indicates that it works (after adding the executable to the $suppliedBinariesInfo list in Cwebp.php):

Creating command line options for version: 1.1.0

Quality: 85.

The near-lossless option ignored for lossy

Trying to convert by executing the following command:

C:\path\to\DocumentRoot\wp-content\plugins\webp-express\vendor\rosell-dk\webp-convert\src\Convert\Converters\Binaries\cwebp-110-windows-x64-unicode.exe -metadata none -q 85 -alpha_q "85" -m 6 -low_memory "C:\path\to\DocumentRoot/wp-content/uploads/2014/01/スクリーンショット-2013-07-10-15.22.28-262x200.png" -o "C:\path\to\DocumentRoot/wp-content/webp-express/webp-images/doc-root/wp-content\uploads\2014\01\スクリーンショット-2013-07-10-15.22.28-262x200.png.webp.lossy.webp" 2>&1 2>&1

Output:

Saving file 'C:\path\to\DocumentRoot/wp-content/webp-express/webp-images/doc-root/wp-content\uploads\2014\01\?????????-2013-07-10-15.22.28-262x200.png.webp.lossy.webp'

File: C:\path\to\DocumentRoot/wp-content/uploads/2014/01/?????????-2013-07-10-15.22.28-262x200.png

I'll push a local branch on my fork that includes it, if that can fill a gap.

I'll edit this post soon to add a link to it (but at this stage I'm not sure it would be a good PR).

If you have all the cross-compiling toolchain elements to produce a Windows binary from Linux I could also test it. (for CentOS 7, that I use on my remote server, they removed the relevant packages from EPEL so I couldn't test this method quickly)

EDIT, if some people need it: Branch with the changes: https://github.com/Ruzgfpegk/webp-express/tree/bugfix-400 Differences with v0.17.4: https://github.com/rosell-dk/webp-express/compare/0.17.4...Ruzgfpegk:bugfix-400

rosell-dk commented 4 years ago

As you have been on github for a quite a while and made more than a few commits, in my eyes, you already earned a good bit of trust. We could add to that trust if we meet up in a videomeeting and you screencast the whole process and show the checksum of the final exe file.

rosell-dk commented 4 years ago

On the other hand, perhaps I should go and get Windows. Also in order to be able to test WebP Express properly on Windows. I don't even have access to a Windows server.

Ruzgfpegk commented 4 years ago

With timezone differences and the fact that I'm doing this on my work computer, I'd prefer another option.

I'm trying to use GitHub Actions to compile an untouched fork of libwebp, targeting Windows platforms, which could be the best way to go (the whole build process could be checked by anybody), but I don't have a working workflow file ready yet.

I'll update this issue when I get a successful build, but if other people have more experience and could get it to work before me that would be good too!

Ruzgfpegk commented 4 years ago

Making some progress, thanks to a new CMake template for GitHub Actions.

I can now build a win-amd64 binary and store it as artifact, but at the config step the following pops up:

-- Could NOT find ZLIB (missing: ZLIB_LIBRARY) (found version "1.2.11") -- Could NOT find PNG (missing: PNG_LIBRARY PNG_PNG_INCLUDE_DIR) -- Could NOT find JPEG (missing: JPEG_LIBRARY) (found version "90") -- Could NOT find TIFF (missing: TIFF_LIBRARY) (found version "4.0.10") -- Could NOT find GIF (missing: GIF_LIBRARY) (found version "5.1.9") -- Could NOT find GLUT (missing: GLUT_glut_LIBRARY) -- Could NOT find SDL (missing: SDL_LIBRARY SDL_INCLUDE_DIR)

I suppose the produced cwebp binary uses dynamic libraries from the system instead, because tests with JPG and PNG files worked well. This may affect the quality though.

In the meantime, here's the action file I used if you want to try on your own local fork of libwebp: https://github.com/Ruzgfpegk/libwebp/blob/master/.github/workflows/cmake2.yml You'll have to put this file (or an improved version of it) in the .github/workflows directory of the branch and commit to it, the process should then launch automatically in the "Actions" tab of the project.

Here's the latest run on my side, with the possibility to download the binary (and I suppose, control the whole build process): https://github.com/Ruzgfpegk/libwebp/actions/runs/229650681 (FYI this was run against master, not the 1.1.0 branch)

mo-han commented 3 years ago

Actually those path chars issue could be easily bypassed by using pipe (stdin, stdout) to feed and get image data to and from the cwebp executable, which is what I did when I write my own python wrapper for cwebp.