happycube / ld-decode

Software defined LaserDisc decoder
GNU General Public License v3.0
294 stars 76 forks source link

Fix pipe output on windows #852

Closed vrunk11 closed 1 year ago

vrunk11 commented 1 year ago

Fix pipe output on Windows for all tool by setting output mode to binary instead of the default mode (text mode)

vrunk11 commented 1 year ago

It compiles on my setup using mingw64 but seem to fail here

oyvindln commented 1 year ago

The includes also needs a #ifdef _WIN32 || _WIN64 guard, those are windows-speficic.

IMO we should also put this in a setup function in library/tbc/something that can be called rather than copy pasting the same code in each file.

vrunk11 commented 1 year ago

The includes also needs a #ifdef _WIN32 || _WIN64 guard, those are windows-speficic.

IMO we should also put this in a setup function in library/tbc/something that can be called rather than copy pasting the same code in each file.

I think we can't put it in a separate file cause the instruction need to be in the main()

oyvindln commented 1 year ago

I don't see why it has to, it doesn't say anything about that in the documentation. The two calls to the windows functions just have to be set before we pipe any data

Like make a set_binary_mode or whatever name functions that contains the

 #ifdef _WIN32
    _setmode(_fileno(stdout), O_BINARY);
    _setmode(_fileno(stdin), O_BINARY); 
#endif

code and call that function at the beginning of each main instead of having those lines copied in each main. This makes it much cleaner.

atsampson commented 1 year ago

There's already some helper code in library/tbc/logging.cpp that gets used in the main function of all the tools. You could add a new file there, or (given it's a really short function and it's sort-of to do with log output) add it to logging.cpp.

I wondered if there was a way of doing this using Qt when stdout is opened, but no - looking at the Qt source, all of their tools have exactly the same hack in them!

(Mind the indentation too - it should match the existing code, with four-space indentation and no tabs.)

vrunk11 commented 1 year ago

ld-process-ac3 don't have the loggin.cpp included, so maybe we should include it ?

oyvindln commented 1 year ago

I don't think they need to, they don't make use of Qt so would be preferable to not link to the qt specific stuff in e.g logging.cpp

vrunk11 commented 1 year ago

it should be good now

oyvindln commented 1 year ago

Looks good now I think other than ld-chroma-decoder/encoder can also use the function since it uses logging