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

WIP: First dig at reading WAV PCM samples from pipe, addressing #80 #81

Closed
wants to merge 1 commit into from

Conversation

davidavdav
Copy link
Contributor

@davidavdav davidavdav commented Nov 7, 2019

Hi,

For PCM wav streams I've made a start to address #80, so this is just to gauge your opinion on the approach.

Things that still need to be taken care of are

  • Add support for reading from pipe for other data formats
  • Add support for subrange specification (by properly implementing skip and limited reading)
  • Somehow add sox as a test dependency

Copy link
Owner

@dancasimiro dancasimiro left a comment

Choose a reason for hiding this comment

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

This looks reasonable, but there are some subtle changes in the handling of bytes. Do you need to make the low level byte changes for sox to function?

src/WAV.jl Show resolved Hide resolved
if ispipe
return read(io)
else
return read(io, len)
Copy link
Owner

Choose a reason for hiding this comment

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

Why did this call switch from read! to read?

Copy link
Contributor Author

@davidavdav davidavdav Nov 17, 2019

Choose a reason for hiding this comment

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

In the original code, the length of the data to be read was known, and could be declared in advance. In the case of reading from a pipe, this won't work as way too too much memory is reserved if the process writing to the pipe has lied about the length (as is the case for sox).

I further changed the read! to read in the non-pipe reading case. I assumed that it does not make a difference in efficiency where the memory is declared, since the implementation of read(::IO, len::Int) declares the memory just like in the original code, and then does the readbytes!().

Copy link
Owner

Choose a reason for hiding this comment

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

Alright.

Am I correct that this code depends on the other process being well behaved?

src/WAV.jl Show resolved Hide resolved
@dancasimiro
Copy link
Owner

@davidavdav: Is there still interest in pursuing this pull request? If not, I would like to close it.

@davidavdav
Copy link
Contributor Author

I would still argue it should be possible to read from pipe, but unfortunately I haven't been in the opportunity to use Julia for work much, so I am fine with leaving this PR in limbo for another while

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