Piped stdout on Windows results in corrupted jpeg data for both capture-movie and capture-preview

[dustinkerstein]

The following commands result in corrupt JPEG data with a Panasonic GH5 with gPhoto2 running on Windows:

gphoto2 --capture-preview --stdout > preview.jpg
gphoto2 --capture-movie --stdout > movie.jpg

When not using --stdout, the JPEG output is valid:

gphoto2 --capture-preview
gphoto2 --capture-movie

gPhoto Version:

gphoto2         2.5.28         gcc, popt(m), exif, no cdk, no aa, jpeg, readline
libgphoto2      2.5.30         standard camlibs, gcc, no ltdl, EXIF
libgphoto2_port 0.12.1         iolibs: disk ptpip usb1, gcc, no ltdl, EXIF, USB, no serial

I am not able to replicate this on MacOS 12.4 with the same camera. My end goal is to use stdout piped into another program, but for replication, the simple commands at the top are enough to replicate. Any ideas on things I could possibly tweak / further debug? Thanks!

Example Corrupt JPEGs and Debug: debug.txt

[ndim]

Hmm. Perhaps unintended LF to CRLF conversion?

[dustinkerstein]

Yah I've been researching the various encoding issues on Windows when dealing with redirected binary output. I tried a bunch of workarounds but nothing has worked yet. Do you think this is something that can be worked around, or would gPhoto possibly need a code change?

Quick update - This Powershell workaround almost worked, but it hangs indefinitely (and produces 2 files: an empty final output file, and a temp file - and it's the latter that seems to contain correct non-mangled data) - https://www.leeholmes.com/redirecting-binary-output-in-powershell

[ndim]

Might https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/setmode?view=msvc-170 be interesting here?

The gphoto2 program is written with lines ending in "\n". So to produce a native output strings, translation from \n to \r\n needs to happen somewhere in the output stream. So it appears stdout or STDOUT_FILENO is implicitly opened with translation mode letter 't' or numeric constant O_TEXT.

However, to output binary data like a JPG image, that translation must not happen (translation mode letter 'b' or numeric constant O_BINARY).

So when gphoto2 recognizes --stdout (FLAG_STDOUT), do we need to add a statement _setmode(_fileno(stdout), _O_BINARY); when building for Windows? (_fileno() is a Windows specific name for the POSIX function fileno(), and _setmode() and _O_BINARY are a Windows specific function and macro constant, respectively)

Then gphoto2 would produce the proper native output, and things should work.

[msmeissn]

Yes, this is the most likely reason ... We need to open this with O_BINARY enabled I think. Can you do a patch?

[ndim]

As stdout is opened by the (OS specific implementation of the) C runtime before our program even starts, we need to change its translation mode long after the opening has been done.

That translation mode change could happen when we set FLAG_STDOUT.

The code I proposed above should do the job for Windows.

If we ever build for a non-Windows system which does translations as well, we will need to add code there as well.

In a few days, I can write and push a patch and cross-compile for Windows to check that it builds, but would rely on OP for testing.

[dustinkerstein]

I'll be ready to help test whenever there is a build ready. Thanks!

[ndim]

@dustinkerstein Are you familiar enough with git that you can test https://github.com/ndim/gphoto2/tree/translation-mode-binary-for-stdout or do you need help there?

As an aside, where did you get popt? I had to hack popt from git a bit to make it compile for Windows so I could install it into my Fedora 36 hosted cross-compile environment.

[dustinkerstein]

@ndim I've never tried compiling gPhoto for Windows. I was using the msys2 packages before, but I'm happy to try. Do you know of any build instructions (either for a Windows environment or cross-compiling)?

[ndim]

It appears that the generic msys2 package build instructions https://www.msys2.org/dev/new-package/ show how to build an msys2 gphoto2 package from the msys2 gphoto2 package source at https://github.com/msys2/MINGW-packages/tree/master/mingw-w64-gphoto2 and https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-gphoto2/PKGBUILD.

Once that process has been established to work, it should be possible to take a gphoto2 git source tree and build a gphoto2-2.5.28.1.tar.xz dist tarball like

autoreconf -vif
configure
make dist
sha256sum gphoto2-*.tar.xz

and then adapt the mingw-w64-gphoto2/PKGBUILD file with the proper SHA256 checksum and the proper gphoto2 version number.

Hmm. As for some reason, mingw-w64-gphoto2/PKGBUILD actually runs autoreconf, we can make use of that and try running git archive --prefix=gphoto2-2.5.28.1 --output gphoto2-2.5.28.1.tar.xz in the gphoto2 git source tree to generate a gphoto2 source tarball for the msys2 gphoto2 package build process to use.

All in all, the whole process does not look very different from building packages for many Linux distributions, but then I have not actually touched a Windows system in about 20 years, let alone built msys2 packages on one, so there might be some unexpected snags.

[dustinkerstein]

@ndim yeah that was surprisingly painless to build (both the original unmodified source and your fork) by following your / MSYS2 directions. Though I won't be able to test until later today. I'll let you know as soon as I have a chance.

[dustinkerstein]

I was just able to take the new code for a spin, but it seems like it's producing the same results with the commands listed in the first post. After it not working on a first attempt, I did a little digging and found this SO post. So I tried both with and without the underscores:

_setmode(_fileno(stdout), _O_BINARY);
setmode(fileno(stdout), O_BINARY);

But again, no luck. Though the info in that post might be useful as it seems the binary mode setting is compiler dependent. Any thoughts on other things I could check / debug? Thanks for digging into this.

[ndim]

So... what could be happening here?

• You could have mistakenly run version of gphoto2 with the setmode line?

  Not likely, but stuff like that happens.

• Are we sure that the setmode line is actually in the codepath executed for gphoto2 --capture-preview --stdout? I have not actually traced all possible codepaths in the source code.

  I can look more closely into that, but I do not have working camera hardware to control with gphoto2 at this time, so I cannot run actual tests even on my Linux system (I have no Windows systems, the Canon Powershot G2's proprietary cable is missing, and the Canon EOS 70D does not show up with lsusb regardless of what USB Mini-A cable I used to connect to it).

• Are we sure it actually is the O_BINARY issue which causes the problem? That was just a wild guess of mine, after all. Sometimes such stupid mistakes known for 30 or 40 years can be the issue, but sometimes they are not.

  As --capture-preview and --capture-movie both produce files which differ each time you call them (probably even if you leave the lens cap on to produce 100% black images, due to timestamps and the like), it will be difficult to just compare the compare the file size of the same file downloaded with and without --stdout.

  However, one might see some confirmation of the O_BINARY hypothesis in a jpg file with a hex editor (or hexdump -C).

  To verify this, @dustinkerstein would need to produce jpg files with gphoto2 --capture-preview and gphoto2 --capture-preview --stdout so we can confirm whether all 10 value bytes are preceded by 13 value bytes or not, ideally both running stock gphoto2 and the gphoto2 with the above setmode line in it?

What have I forgotten to put in this list?

[dustinkerstein]

Sorry for the delay. I was able to spend some time testing today. It seems like the new code isn't in the code path for the commands above. I added some debug:

+++ b/gphoto2/main.c
@@ -146,8 +146,11 @@ strncpy_lower(char *dst, const char *src, size_t count)
 static void
 gpi_file_set_binary_mode(FILE *file)
 {
+       fprintf(stderr,"DUSTIN1\n");
 #ifdef WIN32
+       fprintf(stderr,"DUSTIN2\n");
        _setmode(_fileno(stdout), _O_BINARY);
+       fprintf(stderr,"DUSTIN3\n");
 #endif
 }

@@ -685,18 +688,20 @@ save_file_to_file (Camera *camera, GPContext *context, Flags flags,
                if (tmpfilename) unlink (tmpfilename);
                return res;
        }
-
+       fprintf(stderr,"DUSTIN4\n");
        if (flags & FLAGS_STDOUT) {
                 const char *data;
                 unsigned long int size;

                 CR (gp_file_get_data_and_size (file, &data, &size));
-
+               fprintf(stderr,"DUSTIN5\n");
                if (flags & FLAGS_STDOUT_SIZE) /* this will be difficult in fd mode */
                         printf ("%li\n", size);
+               fprintf(stderr,"DUSTIN6\n");
                gpi_file_set_binary_mode(stdout);
                 if (1!=fwrite (data, size, 1, stdout))
                        fprintf(stderr,"fwrite failed writing to stdout.\n");
+               fprintf(stderr,"DUSTIN6\n");
                if (ps && ps->fd) close (ps->fd);
                free (ps);
                gp_file_unref (file);
@@ -1995,6 +2000,7 @@ report_failure (int result, int argc, char **argv)
 int
 main (int argc, char **argv, char **envp)
 {
+       fprintf(stderr,"DUSTIN0\n");
        CallbackParams cb_params;
        poptContext ctx;
        int i, help_option_given = 0;

And when I run gphoto2 --capture-preview --stdout 1> preview.jpg 2>err.txt (stdout is 1 and stderr is 2) I only see "DUSTIN0" in the err.txt file. I haven't traced the code path, but it does seem that the setmode line isn't being run. Do you want to take a look? Thanks again.

[aximusnl]

Any news on this? I’m running into the same problem.

Edit:

I checked a jpeg created on windows (using --stdout) in a hex-editor, and indeed all 0x0A (CR) are preceded by a 0x0D (LF). When I replace 0x0D 0x0A with 0x0A; the file is a valid jpeg..