JuliaMusic/MusicProcessing.jl

Updating to Julia 1.5 Tasks

Closed this issue ยท 23 comments

JuliaMusic/JuliaMusic_documentation.jl#10

mentions that MusicProcessing.jl needs to be updated to Julia 1.5+ . After this is done this package will be registered and announced.

What are some of the things I can help with in this area?

Hm, out of the top of my head, we have:

  • Remove the REQUIRE file and put proper dependencies and compat-list in Project.toml.
  • Port all code into Julia 1.5. I am sure a lot of parts of the source code need to be updated, haven read the entire source yet.
  • Remove appveyor, remove travis.yml, move tests to use GitHub CI, and start running unit tests again
  • Move all plotting code (code that calls PyPlot) to a separate file, which must be added behind a requires block from Requires.jl. Test the plotting code.
  • Many packages are used here. Perhaps we can remove some of them?
  • Rework documentation. Since this package was written 4 years ago, the docs need updating. We need better documentation strings, and also remove the weird """""" syntax that is used throughout.
  • Build and deploy documentation with Documenter.jl.
  • All import statements, e.g. import Base.eps, DSP.spectrogram, DSP.stft must be removed from source. Method extension should explicitly state the parent module, like function Base.eps(...) = ....
  • Eradicate type piracy (which occurs when you define a new method where you don't own any of the dispatch types, or the function name).
  • Some functionality is duplicating functionality already existing in DSP.jl (which, since 4 years ago has advanced quite a bit). This functionality must be removed from here, and instead just call the more well tested and maintained functions from DSP.jl.

I'll start work on the PyPlot todo

Looks like this package is using FFTW to build several things that DSP.jl already does. Should we include DSP.jl as a dependency? I still need to understand a bit more about how all the pieces in the puzzle fit but re-implementing many things from DSP.jl doesn't seem a good use of time to me

Yeap, absolutely, this needs to be "partly rebuilt" to use DSP.jl and not replicate anything already done there. This is what I was trying to say in my last bullet point in the above list.

So right now here are the todos in terms of restoring functionality:

  • complex.jl
  • display.jl
  • mel.jl
    • I'm currently working on this one
  • util.jl
    • I don't actively work on this but if some function in the codebase is broken and depends on util.jl this is the first place I look and see if I can fix it from here

Other todos could include:

  • checking over my updates to the code
    • I've tried updating the Julia code from its old form to best to my ability but without domain knowledge it's hard to know if the changes I made were equivalent to the old code
  • writing tests
    • I've written a couple tests in runtests.jl to start us off

By the way @Datseris do you mind taking a quick peek at complex.jl? Just to see if it has any old Julia code that could be updated?

This by far seems to be the most intimidating code that I've come across in the codebase in terms of advanced Julia knowledge

If there aren't any red flags for you at first glance that would a good first step.

Hi, yes complex.jl needs to be updated. First, package code, especially a package like MusicProcessing.jl which in principle does only basic signal processing, should never have to use Ptr. In addition, the fourier packages have been updated and provide implementations for planning fourier transforms. All this ccall is not necessary anymore, one can use the fourier packages instead.

Ah also the multiply! function is useless. dst .= left .* right achieves the same thing.

Just letting everyone contributing here know, SampledSignals .jl dependencies haven't been updated for years. https://github.com/JuliaAudio/SampledSignals.jl and last commit to it was 17 months ago.

@ashwani-rathee tagging you well.

Just be aware, you may encounter hardships using it with the current status of Julia. I might be wrong, and the package is working flawlessly, I'm simply raising a warning sign because I actually cannot install MusicProcessing.jl because SampledSignals.jl has a dependency on Compat.jl 2.0, which is very old (and in fact there is no reason whatsoever to use Compat.jl at the moment since Julia 1.0 has been out for like a year). One should tread carefully with these things.

see also here See here JuliaAudio/SampledSignals.jl#65

Thanks for this! Wondering why it worked on my machine though the last time I touched MusicProcessing was in february so maybe that's the cause?

Let's see when the admins from JuliaAudio respond, till then we can fix other things here in list.

@ashwani-rathee I invited you to the org, can you accept please?

@Datseris I was thinking about having some test audio files like done here for images: https://github.com/JuliaImages/TestImages.jl

# in which image are read as shown below 
using TestImages

img = testimage("cameraman.tif") # fullname
img = testimage("cameraman) # without extension works
img = testimage("cam") # with only partial name also works

It would be great to have some of them, this pack provided by Xavier Serra is really good one for specific music signal processing tasks( CC 4.0): https://freesound.org/people/xserra/packs/13038/

Also current state of package doesn't provide functions for read/write(original version never had it and used MP3.jl for read/write operations), current state assumes that user already has data in SampleBuf form. MP3.jl is no longer maintained.

@rob-luke today mentioned that SampledSignals.jl will be worked upon and will be maintained. So slightly confused what should we do next regarding these read/write operations? It would be good enough to have wav support for start and build specific functions from there.

WAV.jl is properly supported and we could use this for WAV. Also, yes to including audio files for tests. Regarding SampleBuf, perhaps we should wait @rob-luke to catch up and provide us with some updates?

We can meanwhile focus on writing proper tests using the WAV package I guess.

In current state:

audio.jl(only play is not documented)
TFR.jl(it has spectrogram, but it uses DSP's spectrogram only)(other functions are utility which can be removed too using FFTW)
mel.jl(none of those functions work(old code of melspectrogram and MFCC))
display.jl has some old code for mfcc plots
chroma.jl and constantq.jl are empty

The current work/code flow confuses me as the code is dependent on code of 3 different audio organisations

function spectrogram(audio::SampleBuf{T, 1},
                        windowsize::Int = 1024,
                        hopsize::Int = windowsize >> 2;
                        window = hanning, kwargs...) where T
    noverlap = windowsize - hopsize
    DSP.spectrogram(audio.data, windowsize, noverlap; fs = audio.samplerate, window = window, kwargs...)
end

So if I remove the function spectrogram(because it's type piracy otherwise) from the MusicProcessing.jl, It wouldn't make sense to document it here right? So what I am not able to understand is what do we implement here and what do we borrow if anything?

Like the first thing we need to implement I think is load and play
A big chunk of job can be done by LibSndFile.jl for loading various types of files and for playing to almost any devices available in system PortAudio.jl(current play() uses it but imports it when the function is run) makes a lot of sense, so yeah it's a little confusing what to do. What do you people think?

Also should we add the docs and the benchmark now ? Docs would make the user's workflow/ usage way clear to me

In my mind MusicProcessing is an easy-to-use music library while DSP.jl is a more complicated signal processing library.

So any functions that a person would need for music processing should stay imo even if it's "type piracy".

In my mind it's like how Plots.jl acts as a user facing library for different plotting libraries.

Please correct me if I'm wrong, I'm still new to open source.

Though I'm not sure the user has any need for a spectrogram function so we can take that one out.

I think doing the docs and benchmark would be an easy win. Go for it!

So any functions that a person would need for music processing should stay imo even if it's "type piracy".

I do not understand this point. Type piracy is an absolute no-go and a clear programming situation. It has in principle nothing to do with the exported interface. Type piracy = "Only add new methods if (1) you own the function name or (2) you own one of the types the method dispatches on".

You do not have to "remove spectrogram", you can extend it:

function DSP.spectrogram(audio::SampleBuf{T, 1},  # notice the DSP.
                        windowsize::Int = 1024,
                        hopsize::Int = windowsize >> 2;
                        window = hanning, kwargs...) where T
    noverlap = windowsize - hopsize
    DSP.spectrogram(audio.data, windowsize, noverlap; fs = audio.samplerate, window = window, kwargs...)
end

We define the type SampleBuf, correct?

Yeah, extending makes sense to me. Because these functions like spectrogram, melspectrogram , mfccs,chroma features are essential to music analysis and if we take help from DSP.jl for this and extend them like this, this method works fine.
And yes, we defineSampleBuf

@Datseris Is there any particular time that we register a package? like after certain number of functions etc.
Making documentation and deploying it would be easier if the package is registered.

I would say docs and passing tests are the essentials to register. Functionality can still be lacking, but we should get these two going. We can add the docs of this package to JuliaMusic's docs, which are shared betweeen MIDI, MusicManipulations, and other packages, or we can keep its docs separated. I personally vote to have everything together, what do you guys think?

They would be better together I think if we write them well