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

Migrating to RingBuffers.jl #7

Open
standarddeviant opened this issue Dec 6, 2017 · 15 comments
Open

Migrating to RingBuffers.jl #7

standarddeviant opened this issue Dec 6, 2017 · 15 comments

Comments

@standarddeviant
Copy link

This might be intended, but I just noticed this behavior:

julia> Pkg.add("JACKAudio")
ERROR: JACKAudio can't be installed because it has no versions that support 0.6.1 of julia. You may need to update METADATA by running `Pkg.update()`
resolve(::Dict{String,Base.Pkg.Types.VersionSet}, ::Dict{String,Dict{VersionNumber,Base.Pkg.Types.Available}}, ::Dict{String,Tuple{VersionNumber,Bool}}, ::Dict{String,Base.Pkg.Types.Fixed}, ::Dict{String,VersionNumber}, ::Set{String}) at ./pkg/entry.jl:486
resolve(::Dict{String,Base.Pkg.Types.VersionSet}, ::Dict{String,Dict{VersionNumber,Base.Pkg.Types.Available}}, ::Dict{String,Tuple{VersionNumber,Bool}}, ::Dict{String,Base.Pkg.Types.Fixed}) at ./pkg/entry.jl:479
edit(::Function, ::String, ::Base.Pkg.Types.VersionSet, ::Vararg{Base.Pkg.Types.VersionSet,N} where N) at ./pkg/entry.jl:30
(::Base.Pkg.Entry.##1#3{String,Base.Pkg.Types.VersionSet})() at ./task.jl:335
Stacktrace:
 [1] sync_end() at ./task.jl:287
 [2] macro expansion at ./task.jl:303 [inlined]
 [3] add(::String, ::Base.Pkg.Types.VersionSet) at ./pkg/entry.jl:51
 [4] (::Base.Pkg.Dir.##4#7{Array{Any,1},Base.Pkg.Entry.#add,Tuple{String}})() at ./pkg/dir.jl:36
 [5] cd(::Base.Pkg.Dir.##4#7{Array{Any,1},Base.Pkg.Entry.#add,Tuple{String}}, ::String) at ./file.jl:70
 [6] #cd#1(::Array{Any,1}, ::Function, ::Function, ::String, ::Vararg{String,N} where N) at ./pkg/dir.jl:36
 [7] add(::String) at ./pkg/pkg.jl:117

julia> Pkg.clone("https://github.com/JuliaAudio/JACKAudio.jl.git")
INFO: Cloning JACKAudio from https://github.com/JuliaAudio/JACKAudio.jl.git
INFO: Computing changes...
WARNING: julia is fixed at 0.6.1 conflicting with requirement for JACKAudio: [0.4.0,0.6.0-rc1)
INFO: Cloning cache of AbstractFFTs from https://github.com/JuliaMath/AbstractFFTs.jl.git
INFO: Cloning cache of BinDeps from https://github.com/JuliaLang/BinDeps.jl.git
INFO: Cloning cache of Compat from https://github.com/JuliaLang/Compat.jl.git
INFO: Cloning cache of DSP from https://github.com/JuliaDSP/DSP.jl.git
INFO: Cloning cache of FFTW from https://github.com/JuliaMath/FFTW.jl.git
INFO: Cloning cache of FixedPointNumbers from https://github.com/JuliaMath/FixedPointNumbers.jl.git
INFO: Cloning cache of Polynomials from https://github.com/JuliaMath/Polynomials.jl.git
INFO: Cloning cache of Reexport from https://github.com/simonster/Reexport.jl.git
INFO: Cloning cache of SHA from https://github.com/staticfloat/SHA.jl.git
INFO: Cloning cache of SIUnits from https://github.com/Keno/SIUnits.jl.git
INFO: Cloning cache of SampledSignals from https://github.com/JuliaAudio/SampledSignals.jl.git
INFO: Cloning cache of SpecialFunctions from https://github.com/JuliaMath/SpecialFunctions.jl.git
INFO: Cloning cache of TexExtensions from https://github.com/Keno/TexExtensions.jl.git
INFO: Cloning cache of URIParser from https://github.com/JuliaWeb/URIParser.jl.git
INFO: Installing AbstractFFTs v0.2.0
INFO: Installing BinDeps v0.8.0
INFO: Installing Compat v0.39.0
INFO: Installing DSP v0.3.4
INFO: Installing FFTW v0.0.4
INFO: Installing FixedPointNumbers v0.4.3
INFO: Installing Polynomials v0.1.6
INFO: Installing Reexport v0.0.3
INFO: Installing SHA v0.5.2
INFO: Installing SIUnits v0.1.0
INFO: Installing SampledSignals v1.1.0
INFO: Installing SpecialFunctions v0.3.5
INFO: Installing TexExtensions v0.1.0
INFO: Installing URIParser v0.2.0
INFO: Building SpecialFunctions
INFO: Building FFTW
INFO: Package database updated

julia> 
@standarddeviant
Copy link
Author

When I just did a Pkg.clone(...) of JACKAudio and tried to run using JACKAudio I got the following error:

ERROR: LoadError: UndefVarError: SingleAsyncWork not defined
Stacktrace:
 [1] include_from_node1(::String) at ./loading.jl:576
 [2] include(::String) at ./sysimg.jl:14
 [3] anonymous at ./<missing>:2
while loading /home/preq/.julia/v0.6/JACKAudio/src/JACKAudio.jl, in expression starting on line 521
ERROR: Failed to precompile JACKAudio to /home/preq/.julia/lib/v0.6/JACKAudio.ji.
Stacktrace:
 [1] compilecache(::String) at ./loading.jl:710
 [2] _require(::Symbol) at ./loading.jl:497
 [3] require(::Symbol) at ./loading.jl:405

After a little digging, this makes sense given that this commit is labeled "rename SingleAsyncWork to AsyncCondition, JuliaLang/julia@eaf1ba5

So the requirements are pretty sensible. If I get this working on 0.6.1, I'll submit a PR.

@ssfrr
Copy link
Contributor

ssfrr commented Dec 6, 2017

IIRC AsyncCondition is also not just a renaming of SingleAsyncWork, they're also used somewhat differently (SingleAsyncWork takes a callback, but AsyncCondition is waited on).

The real solution I think is to port over JACKAudio to use RingBuffers.jl, which provides a thread-safe lockfree ringbuffer that can be called from C or Julia. I use it in PortAudio.jl and it has cleaned up things pretty substantially.

@standarddeviant
Copy link
Author

@ssfrr Do you think this would require a C shim layer, similar to pa_shim.c ?

If not, why not? I'm still coming up to speed on the mechanics of AsyncCondition in an effort to be helpful :-)

@ssfrr
Copy link
Contributor

ssfrr commented Dec 14, 2017

Yeah, I think the overall architecture would be very similar to how PortAudio.jl is set up. There'd be a jack_shim.c that would implement the JACK callback function and handle exchanging data between the ringbuffer and the JACK buffers (using the pa_ringbuffer API). Then on the Julia side the stream would read and write from the ringbuffers using RingBuffers.jl (which is basically a Julia wrapper around pa_ringbuffer).

That's definitely one likely point of confusion - RingBuffers.jl is independent of PortAudio, even though it uses the PortAudio ringbuffer implementation (which is shipped as a separate library). So a hypothetical jack_shim.c would replace all the portaudio audio callback stuff to its JACK equivalent, but would still use the pa_ringbuffer stuff to communicate with RingBuffers.jl.

@standarddeviant
Copy link
Author

Thanks for the explanation. That makes sense PortAudio.jl uses pa_ringbuffer, but that pa_ringbuffer is just a ring buffer implementation by itself.

I'm taking a shot at this here but it's no where near ready.

@ssfrr
Copy link
Contributor

ssfrr commented Dec 16, 2017

That's awesome, thanks! Let me know if you have any questions or want me to check anything out.

@standarddeviant
Copy link
Author

So, jack typically has a "channel buffer" model, where each input and output channel has its own buffer. portaudio has a "multichannel interleaved buffer" model, where all input (and output) channels are transported as a single, interleaved buffer.

Given that, it makes sense that pa_ringbuffer.h and PortAudio.jl also organize data as a "multichannel interleaved buffer".

For JackAudio.jl, does it make sense to try to use the pa_ringbuffer.h implementation with interleaved data? I'm wondering if it makes sense to read from (and write to) pa_ringbuffer.h with a series of "one-channel buffer". pa_ringbuffer.h wouldn't know the data is non-interleaved, but just transport it faithfully.

The downside seems like RingBuffer.jl wouldn't have the right convenience functions, but maybe they could be added.

Thoughts?

@ssfrr
Copy link
Contributor

ssfrr commented Dec 18, 2017

I think in general a ringbuf-per-channel approach would work well, and then you don't have to worry about interleaving/deinterleaving on both sides of the ring buffer.

One potential gotcha would be making sure that the channels are always in sync, particularly after an overflow or underflow. The issue could pop up if the JACK callback gets called while the Julia side is in the middle of processing and has maybe processed some channels but not others. You could get into a state where the channels processed before the JACK callback underflowed/overflowed but the channels processed afterwards were fine, so the channels are now de-synced. Doing an initial pass to determine worst case readability/writability should mitigate the problem. I think whenever reading or writing to the collection of ringbuffers you'd want to first go through the list of them and find the worst-case data/space availability, then go through and read/write that number of samples.

@standarddeviant
Copy link
Author

@ssfrr Great points. I think jack_shim.c might be in a serviceable state with your above suggestions.
At the very least, jack_shim.c compiles without error:
https://github.com/standarddeviant/JACKAudio.jl/blob/SD_asynccondition_ringbuffer_shim/deps/src/jack_shim.c

jack_shim.jl still definitely needs work. I haven't quite thought how that could interface with the other parts of JACKAudio.jl, but am appreciative of any suggestions or insights.

@ssfrr
Copy link
Contributor

ssfrr commented Dec 20, 2017

Looking good so far, thanks for taking this on!

It looks like you've got the gist of info->sync, but to make it more clear/explicit, the idea is that in a lot of casual cases you might want to allow the user to open both input and output channels, but then write to the output without doing the corresponding reads, or vice-versa record from the inputs without writing anything to the outputs. I'm thinking of playing around at the REPL. The issue is that if there's an over or underflow, you change the total round-trip through the input and output ringbuffers, changing the round-trip latency. The idea is that if you care about deterministic latency you'll set info->sync=true, which will force the same number of samples to be read and written each block. In some ways I think of it as a casual/strict switch.

That's also related to your question about whether it's too spammy to notify on each over/underflow - it might be a good enough heuristic that if info->sync is false than the user doesn't care about over and underflows.

Your question about when to notify the Julia side is a good one, and I'm not 100% sure of the right answer right now. I'd think you'd only want to notify after reading/writing all of the channels, but that might mess up some of the task blocking logic on the Julia side. On the other hand, maybe if you process the channels in the same order on the Julia and C sides, than the task wake-up might actually work as expected. Not sure if it would cause more context-switches than necessary though... Hopefully the answer will present itself. 😄

@standarddeviant
Copy link
Author

I'm a bit naive about REPL use cases, but would like to know more. My intended application for some of these changes is to allow for easier coding of "long running" clients that will run for hours or days without interruption. For that kind of use case, I added an info->synchandle that notifies when all channels are written/read at the end of a single call to shim_processcb_c in jack_shim.c. I also made some small changes regarding info->sync and notifying on each channel write/read. I'm wondering if the input/output handles should have channel information associated with them for the REPL use cases.

Right now, I either make "long running clients" in C or Python. Both have their benefits, but I think JACK clients written in Julia could be the best of both worlds. I'm reading through JACKAudio.jl and one of my goals is to be able to write simple julia functions that are the effectively the jack process function. Ideally, they'd just have to worry about two function inputs:

  • a 2D input array (samples x input-channels)
  • a 2D output array (samples x output-channels)

@ssfrr
Copy link
Contributor

ssfrr commented Dec 27, 2017

Rather than structuring the application around a processing callback function, the JuliaAudio packages are stream-oriented, so a long-running process would look like:

# set up for stereo IO, also promise that we'll make sure to read the same number
# of frames as we write, and in exchange JACKAudio will make sure the ringbuffer
# latency is deterministic
client = JACKClient("Processor", 2, 2, synced=true)

# start by reading a block of audio, so the samplerate, element type, etc. all match
# the source this will be a `SampleBuf`, which is basically a samplerate-aware
# wrapper around an len x chan array
buf = read(client, 4096)
while true
    super_advanced_dsp_process!(buf) # this is where you do your stuff
    write(client, buf)
    read!(client, buf)
end

If you want separate input and output buffers to work with you could pre-allocate an output buffer as well (creating the SampleBuf explicitly), rather than mutating buf in your process! function, and then your read! and write calls would use your input and output buffers, respectively.

The idea with REPL usage is just that I want people to be able to do things like open a JACKClient in one cell of a Jupyter notebook or at the REPL, and then some time later they might read from it to record something, or load a sound file and play it. In this sort of usage we don't expect the user to be responsible for constantly filling the output ringbuffer and reading from the input ringbuffer - we cut them some slack and just handle the under/over-flows.

@ssfrr ssfrr changed the title Can't run "Pkg.add" on julia v0.6.1 Migrating to RingBuffers.jl Dec 27, 2017
@standarddeviant
Copy link
Author

@ssfrr That makes sense. I'll work towards that type of interface.

Out of curiosity, I'm looking at the code, and I see that
See https://github.com/JuliaAudio/JACKAudio.jl/blob/master/src/JACKAudio.jl#L308

Base.read!(client::JACKClient, args...) = read!(client.sources[1], args...)
Base.read(client::JACKClient, args...) = read(client.sources[1], args...)
Base.write(client::JACKClient, args...) = write(client.sinks[1], args...)

But I don't see where read!(source::JACKSource, args...) is implemented. Am I missing something?

@standarddeviant
Copy link
Author

standarddeviant commented Jan 7, 2018

UPDATE: Nevermind, I just wasn't linking jack_shim.o against libjack. Now I just get a segfault. Everything is going according to plan. :-)

Original post:
So, currently, I can run

include("/home/dave/src/JACKAudio.jl/src/JACKAudio.jl")
using JACKAudio
c = JACKClient()

And ultimately Julia quits with the following information:

/home/dave/julia/bin/julia: symbol lookup error: /home/dave/src/JACKAudio.jl/src/../deps/usr/lib/jack_shim.so: undefined symbol: jack_port_get_buffer

Which is odd, given that in Julia, I have the following behavior...

julia> Libdl.dlpath(:libjack)
"/usr/lib/x86_64-linux-gnu/libjack.so.0"

@ssfrr Have you ever seen something like this where the .so files get "mixed up"? I get this behavior as of this commit
~~

@ssfrr
Copy link
Contributor

ssfrr commented Jan 9, 2018

Ah, fantastic, glad you're back to the expected segfaults. 😄

Regarding your previous question, there's an abstract implemention in SampledSignals.jl that handles reading/writing from sources/sinks (respectively). The call works its way through the samplerate/channelcount conversion machinery (if needed) and ends up in calls to unsafe_read!, which can then assume that the channel count, etc. all match up.

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

No branches or pull requests

2 participants