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.

Sign up for GitHub

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

Jump to bottom

Add support for stdin and stdout #89

Merged
merged 8 commits into from
Nov 2, 2019
Merged

Add support for stdin and stdout #89

merged 8 commits into from
Nov 2, 2019

Conversation

chrisn
Copy link
Member

@chrisn chrisn commented Apr 19, 2019

  • Write status messages to stderr instead of stdout
  • Added --input-format and --output-format options
  • Changed file I/O code to be able to read/write stdio or files

@chrisn
Add support for stdin and stdout
4fac261 
- Write status messages to stderr instead of stdout
- Added --input-format and --output-format options
- Changed file I/O code to be able to read/write stdio or files
This was referenced Apr 19, 2019
Closed
Closed
runlevel5
runlevel5 reviewed Apr 28, 2019
View reviewed changes
README.md
standard input and write to standard output. For example, the following command
generates a waveform data file by converting a video file using ffmpeg:

$ ffmpeg -i test.mp4 -f wav - | audiowaveform --input-format wav --output-format dat -b 8 > test.dat
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

runlevel5
runlevel5 reviewed Apr 28, 2019
View reviewed changes
src/AudioFileReader.cpp Outdated
@@ -64,7 +64,7 @@ void AudioFileReader::showProgress(long long done, long long total)
if (percent != percent_) {
percent_ = percent;

output_stream << "\rDone: " << percent << "%" << std::flush;
error_stream << "\rDone: " << percent << "%" << std::flush;
Copy link

@runlevel5 runlevel5 Apr 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rightful place for progress info however it might be a bit concerning for users who log the stdout for progress. I think you should mention this breaking behaviour in CHANGELOG

@runlevel5
Copy link

LGTM

@chrisn
Copy link
Member Author

chrisn commented Apr 29, 2019 •
edited
Loading

Still to do:

  • Add test cases
  • Read .dat files from stdin

@chrisn
Fixed processing files to/from stdio
515a94f 
- Duration calculation
- More consistent status messages
- Error reporting if unable to perform requested conversion
- Test cases for all combinations of file and stdio input and output
- Prevent illegal seek error writing WAV output
- Remove code duplication in WaveformBuffer
@chrisn chrisn marked this pull request as ready for review August 3, 2019 12:46
@chrisn
Copy link
Member Author

chrisn commented Aug 3, 2019

@joneslee85 I think this PR is ready to merge. I'd welcome feedback on this, if you're interested in doing some tests.

@runlevel5
Copy link

@joneslee85 I think this PR is ready to merge. I'd welcome feedback on this, if you're interested in doing some tests.

Code looks good. I am going to run few tests locally then come back to you.

@ffxsam
Copy link

ffxsam commented Sep 15, 2019

Very interested in this PR, as I'd love the ability to stream directly from AWS S3 into audiowaveform. Awesome work!

@ffxsam
Copy link

ffxsam commented Sep 15, 2019 •
edited
Loading

I tried this on an mp3 file, and it didn't seem to work:

$ cat test.mp3 | audiowaveform --no-axis-labels -e 246 --waveform-color 0073d1 --background-color 00000000 -o file.png --input-format mp3
Input file: /dev/stdin
Failed to read file: Illegal seek

Works fine on WAV files though. And when doing -i test.mp3, it also works as expected.

@raedatoui
Copy link

@ffxsam how would you got about streaming S3 files, signed URLs or boto3 or something else?

@chrisn
Copy link
Member Author

chrisn commented Oct 31, 2019 •
edited
Loading

Currently investigating the Illegal seek error. This is caused by the code that skips ID3 tags in the MP3 reader.

@chrisn chrisn merged commit dfb20b9 into master Nov 2, 2019
@chrisn chrisn deleted the stdio2 branch November 2, 2019 14:46
@ffxsam
Copy link

ffxsam commented Nov 2, 2019 •
edited
Loading

Thanks @chrisn!

@raedatoui: Just using the AWS JavaScript SDK.

const stream = s3.getObject({ Bucket: 'my-bucket', Key: 'file' }).createReadStream();

Once you have that stream, you can use Node.js fs to pipe that through a spawned process.

@chrisn
Copy link
Member Author

chrisn commented Nov 3, 2019

Version 1.4.0 is now published. Thanks all for your feeback!

@tslater
Copy link

tslater commented Nov 25, 2023 •
edited
Loading

I tried this on an mp3 file, and it didn't seem to work:

$ cat test.mp3 | audiowaveform --no-axis-labels -e 246 --waveform-color 0073d1 --background-color 00000000 -o file.png --input-format mp3
Input file: /dev/stdin
Failed to read file: Illegal seek

Works fine on WAV files though. And when doing -i test.mp3, it also works as expected.

I'm seeing a similar issue issue with FLAC files:
stderr: Failed to read input: Error : flac decoder lost sync.

@chrisn
Copy link
Member Author

chrisn commented Nov 25, 2023

@tslater Please see #103.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@runlevel5 runlevel5 runlevel5 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@chrisn @runlevel5 @ffxsam @raedatoui @tslater