Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding stdout output (alternate method) #23

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

apocalyptech
Copy link

I was looking to script this together with revorb and came across #8, which adds STDOUT output to ww2ogg. It looks like that one never got fully cleaned up, though, and I'd prefer a different implementation anyway, so I've done up my own PR.

What this does is:

  1. Adds a --quiet option which silences all "status" type messages
  2. Allows using - as the output file option, which forces the use of --quiet
  3. Status output (when not using --quiet) remains on stdout, rather than switching to stderr as the other PR does.
  4. Checks isatty() as requested in that other PR

I've introduced some OS-specific tweaks to support some of this -- namely, Linux machines will need to include unistd.h and do some #define shenanigans to be able to use _isatty and _fileno, as they're known on the Windows side. The other PR implied that MinGW32 needed fcntl.h for _O_OUTPUT as well, so that's there. The call to _setmode (to ensure that stdout is handled as binary data) doesn't need to be done on Linux because Linux doesn't distinguish between binary + text on the standard streams (and doesn't define _setmode) anyway.

This has received no testing on Windows, w/ MinGW or not, so I can't vouch for that. It also doesn't pull the --quiet functionality into those other commented cout instances throughout the code, since that would involve passing some more vars around and it didn't seem worth it. I have tested this with revorb, and that works great.

I did verify that the output is identical in a variety of circumstances, and checked it versus the unpatched output as well:

Auto-filename:

$ ./ww2ogg 1000168723.wem
Audiokinetic Wwise RIFF/RIFX Vorbis to Ogg Vorbis converter 0.24 by hcs

Input: 1000168723.wem
RIFF WAVE 2 channels 48000 Hz 238352 bps
20209 samples
- 2 byte packet headers, no granule
- stripped setup header
- external codebooks (packed_codebooks.bin)
- modified Vorbis packets
Output: 1000168723.ogg
Done!

Specified filename:

$ ./ww2ogg -o test1.ogg 1000168723.wem
Audiokinetic Wwise RIFF/RIFX Vorbis to Ogg Vorbis converter 0.24 by hcs

Input: 1000168723.wem
RIFF WAVE 2 channels 48000 Hz 238352 bps
20209 samples
- 2 byte packet headers, no granule
- stripped setup header
- external codebooks (packed_codebooks.bin)
- modified Vorbis packets
Output: test1.ogg
Done!

Quiet output:

$ ./ww2ogg -o test2.ogg --quiet 1000168723.wem

Redirected to file manually:

$ ./ww2ogg -o - 1000168723.wem > test3.ogg

Checksums:

$ sha256sum *.ogg
18bd61b76089691ae26deac21cd781454fb2a54d2043531f256dddc910342988  1000168723.ogg
18bd61b76089691ae26deac21cd781454fb2a54d2043531f256dddc910342988  test1.ogg
18bd61b76089691ae26deac21cd781454fb2a54d2043531f256dddc910342988  test2.ogg
18bd61b76089691ae26deac21cd781454fb2a54d2043531f256dddc910342988  test3.ogg

Errors still display when redirecting:

$ ./ww2ogg -o - invalid_input > test4.ogg
Parse error: missing RIFF
$ cat test4.ogg
$

Refuses to output to stdout when on a TTY (also shows some extra usage() text):

$ ./ww2ogg -o - 1000168723.wem
Argument error: STDOUT output is not supported when on a TTY

Audiokinetic Wwise RIFF/RIFX Vorbis to Ogg Vorbis converter 0.24 by hcs

usage: ww2ogg input.wav [-o output.ogg] [--inline-codebooks] [--full-setup]
                        [--mod-packets | --no-mod-packets]
                        [--pcb packed_codebooks.bin] [--quiet]

Use "-o -" to send the ogg file to stdout.  This will also
imply "--quiet".

Anyway, lots of text. Let me know if you want any changes or a different implementation, etc!

@hcs64
Copy link
Owner

hcs64 commented Jun 5, 2021

Cool, thanks, I'll look over this soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants