Trying to work on functionality for the display and other updates #15

jparcill · 2021-02-16T06:28:52Z

Just wanted someone to look at the code to see that everything's good. Right now I also have a bit of a problem with trying to write the Base.show functions in display.jl. Wondering if you could take a look @Datseris. They're still printing out the arrays instead of plotting.

Datseris · 2021-02-16T11:31:58Z

Hi, I'm a bit busy at the moment will other projects, but I promise to have a look when there is some extra time.

jparcill · 2021-02-16T13:06:42Z

No worries. There is still a bit more I can add like extensive testing and I'll try getting to that

ashwani-rathee · 2021-04-20T14:25:38Z

@Datseris can you give this PR a check?This PR hasn't made progress in past 2 months

Datseris · 2021-04-20T14:47:48Z

@ashwani-rathee feel free to review here; I'll add mine when I have some free time. I haven't had any time for this project since the last message.

(if you want this to progress faster just add a review)

Datseris · 2021-04-20T18:59:24Z

@ashwani-rathee I've updated the test infrastructure. Tests now run properly and should be made passable in this PR. You can click on the "checks" below and see what tests are failing.

Datseris · 2021-04-22T08:41:49Z

@ashwani-rathee and @jparcill you're now members of JuliaMusic and have commit access to this repository. This means that you can (a) branch the repo in-place instead of using a fork and (b) that you can merge pull requests. I did that because I may not have the time to go through PRs early enough. If you want to be merging PRs please follow these guidelines:

Merge PRs only if tests pass
Merge PRs only if there is at least one approving review from someone

ashwani-rathee · 2021-04-22T09:13:29Z

@Datseris I am writing tests and making documentation for the methods. But in audio.jl, I noticed a lot of methods repeat a function just to make them look like as if they're written here which is kind of weird in my opinion.
For example:

function mono(audio::SampleBuf{T, 2}) where {T <: AbstractFloat}
    SampleBuf{T, 1}(
        vec(SampledSignals.mono(audio).data),
        audio.samplerate
    )
end

In this function,

audio_two_channel = SampleBuf(rand(1000,2), 10)
SampledSignals.mono(audio_two_channel) #1
mono(audio_two_channel) #2

Both return the same thing, this package's mono() function is taking result from SampledSignals.mono() and redoing the whole thing again . Both return SampleBuf{Float64,2}...
And this happens in some other functions too.

Before #13 PR, mono() implementation's was a original/its own implementation

function mono(audio::SampleBuf{T, 2}) where {T <: AbstractFloat}
    SampleBuf{T, 1}(
        mean(audio.data, 2)[:],
        audio.samplerate
    )
end

This is one of the issues which concern me and how should we solve it?
And how to get SampledSignals.jl updated? I am thinking of contacting @ssfrr and @haberdashPI(core developers of SampledSignals.jl) over email if we get no responses on PR after a week. I think both are busy with their work,since not much progress has been made in package for quite a long time.
JuliaAudio/SampledSignals.jl#65

ashwani-rathee · 2021-04-22T09:21:57Z

@ashwani-rathee and @jparcill you're now members of JuliaMusic and have commit access to this repository. This means that you can (a) branch the repo in-place instead of using a fork and (b) that you can merge pull requests. I did that because I may not have the time to go through PRs early enough. If you want to be merging PRs please follow these guidelines:

Merge PRs only if tests pass

Merge PRs only if there is at least one approving review from someone

Sure, Those tests somehow work in my local system but it would be much better if
SampeldSignals.jl doesn't create error when doing the testing here on github. As of now, It doesn't run tests. We really need to be extra careful with this 😄 .

Datseris · 2021-04-22T10:59:03Z

But in audio.jl, I noticed a lot of methods repeat a function just to make them look like as if they're written here which is kind of weird in my opinion

yeap that's weird, go ahead and delete all duplicate stuff

haberdashPI · 2021-04-22T16:33:29Z

Hi there! I am not really a core developer of SampledSignals, I've contributed a few PRs here and there, but I ended up moving a lot of my work to a separate repository SignalOperators. It might be worth considering using DimensionalData, depending on your use case, or just using an Array. I have found that SampledSignals is not updated frequently enough for my purposes, and have moved away from using it.

Datseris · 2021-04-22T16:38:56Z

Thanks @haberdashPI . Indeed it looks like an abandoned repo, still haven't found an admin of JuliaAudio.

@jparcill and @ashwani-rathee I'd recommend to drop SampledSignals. Whatever we need from it we can copy/paste it here from its source.

Datseris · 2021-04-22T16:40:18Z

DimensionalData is a good alternative if we really need to bundle everything into one struct, but perhaps using standard arrays is even better... Or we use your cited SignalOperators.jl

jparcill · 2021-04-22T22:23:13Z

@ashwani-rathee I'm assuming you're gonna be getting to work on the SampleBuf replacements? My school term just ended so let me know if you need help with anything!

ashwani-rathee · 2021-04-23T01:33:11Z

Yes, I am doing that👍

ashwani-rathee · 2021-04-24T11:59:17Z

@Datseris Can we adopt SampledSignals.jl ? I have combined pieces of SampledSignals.jl and MusicProcessing.jl and they work well together in tests. But I am not sure if it will make the package too heavy(maybe not)..
Of course there will be a lot of pruning work since both exported a lot of similar functions like nframes, samplerate, mono, etc but the most important part it provides is the initial reading, writing of audio signals.

Datseris · 2021-04-24T15:19:54Z

At the moment I'd recommend simply porting the required source code of SampledSignals.jl to this package. We can't take the original package as there is noone to even respond to us.

ashwani-rathee · 2021-05-09T13:27:15Z

@jparcill can you check if in the latest version of MusicProcessing, functions i.e. speedup, slowsown, pitchshift work for you?

A persistent error in my system is this : JuliaMath/FFTW.jl#110, I am not sure how to resolve this. I have updated things in my system, and I have added and build new FFTW several times now.

(@v1.6) pkg> activate .
  Activating environment at `~/julia-related/MusicProcessing.jl/Project.toml`

julia> using Revise

julia> using MusicProcessing

julia> using FFTW

julia> FFTW.libfftw3
"/home/ashwani/.julia/artifacts/81791030d1dcd08bf0c67b3e8224cb573d0f5a0a/lib/libfftw3.so"

julia> FFTW.libfftw3f
"/home/ashwani/.julia/artifacts/81791030d1dcd08bf0c67b3e8224cb573d0f5a0a/lib/libfftw3f.so"

julia> using Libdl

julia> Libdl.dlsym(Libdl.dlopen(FFTW.libfftw3), :fftw_threads_set_callback)
Ptr{Nothing} @0x00007fb0086c2c40

julia> audio_one_channel = SampleBuf(rand(1000), 10)
1000-element SampleBuf{Float64, 1}:
 0.7188992365329432
 0.9291267382769475
 0.9235711252208105
 0.7298652018343716
 ⋮
 0.7817233868027189
 0.9691375556064439
 0.009171005643992869

julia> speedup(audio_one_channel, 1.5)
ERROR: could not load library "libfftw3"
libfftw3.so: cannot open shared object file: No such file or directory
Stacktrace:
 [1] plan_dft_c2r_1d!
   @ ~/julia-related/MusicProcessing.jl/src/complex.jl:22 [inlined]
 [2] irfft!(dest::Vector{Float64}, src::Ptr{ComplexF64}, nfft::Int64)
   @ MusicProcessing ~/julia-related/MusicProcessing.jl/src/complex.jl:35
 [3] istft(stft::Matrix{ComplexF64}, samplerate::Float64, windowsize::Int64, hopsize::Int64; nfft::Int64, window::typeof(DSP.Windows.hanning))
   @ MusicProcessing ~/julia-related/MusicProcessing.jl/src/TFR.jl:117
 [4] istft
   @ ~/julia-related/MusicProcessing.jl/src/TFR.jl:101 [inlined]
 [5] #speedup#3
   @ ~/julia-related/MusicProcessing.jl/src/audio.jl:99 [inlined]
 [6] speedup (repeats 3 times)
   @ ~/julia-related/MusicProcessing.jl/src/audio.jl:97 [inlined]
 [7] top-level scope
   @ REPL[15]:1

julia> pathof(FFTW)
"/home/ashwani/.julia/packages/FFTW/Iu2GG/src/FFTW.jl"

julia> FFTW.libfftw3
"/home/ashwani/.julia/artifacts/81791030d1dcd08bf0c67b3e8224cb573d0f5a0a/lib/libfftw3.so"

version info:

julia> versioninfo()
Julia Version 1.6.1
Commit 6aaedecc44 (2021-04-23 05:59 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i5-8250U CPU @ 1.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, skylake)

jparcill · 2021-05-09T13:37:34Z

Sure I'll check it out

jparcill · 2021-05-09T13:45:40Z

Works for me.

Here's my version info:

Julia Version 1.6.1
Commit 6aaedecc44* (2021-04-23 05:59 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i3-4130 CPU @ 3.40GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.1.0 (ORCJIT, haswell)

clouds56 · 2021-06-11T12:56:44Z

src/mel.jl

+        lower = (fftfreqs .- melfreqs[i]) ./ (melfreqs[i+1] .- melfreqs[i])
+        upper = (melfreqs[i+2] .- fftfreqs) ./ (melfreqs[i+2] .- melfreqs[i+1])


Suggested change

lower = (fftfreqs .- melfreqs[i]) ./ (melfreqs[i+1] .- melfreqs[i])

upper = (melfreqs[i+2] .- fftfreqs) ./ (melfreqs[i+2] .- melfreqs[i+1])

lower = (fftfreqs .- melfreqs[i]) ./ (melfreqs[i+1] - melfreqs[i])

upper = (melfreqs[i+2] .- fftfreqs) ./ (melfreqs[i+2] - melfreqs[i+1])

this is scalar minus I think

it doesn't make any difference in the actual code though

clouds56 · 2021-06-11T12:57:45Z

src/mel.jl

+        for j in 1:second_dim
+            weights[i, j] = max(0, min(lower[j], upper[j])) * enorm[i]
+        end


Suggested change

for j in 1:second_dim

weights[i, j] = max(0, min(lower[j], upper[j])) * enorm[i]

end

weights[i, :] .= max.(0, min.(lower, upper)) .* enorm[i]

for loop version is guaranteed to be faster. Although I have no idea if this is performance critical code.

clouds56 · 2021-06-11T12:58:30Z

src/util.jl

+        for j in 1:length(samples)
+            basis[i, j] = cos(i * samples[j])
+        end


Suggested change

for j in 1:length(samples)

basis[i, j] = cos(i * samples[j])

end

basis[i, :] .= cos.(i .* samples)

.= instead

Is .= necessary? I've tried in REPL

a = [1,2,3] a[:] = [4,5,6] println(a) # [4,5,6]

it is the preferred way to do it, as it allows loop fusion (here it doesn't apply, but better to follow the best style)

in addition your example allocates one extra vector for not using .=

Thanks for explanation!

nevertheless, the for loop version is guaranteed to be faster.

clouds56 · 2021-06-11T13:00:25Z

src/display.jl

@@ -16,11 +17,11 @@ function yticklabels(tfr::MFCC, yticks::Array)
    map(Int, map(round, yticks)), "MFCC number"
 end

-heatmap(tfr::DSP.Periodograms.TFR) = log10(power(tfr))
+heatmap(tfr::DSP.Periodograms.TFR) = map(x -> log(10,x), tfr.power)


Suggested change

heatmap(tfr::DSP.Periodograms.TFR) = map(x -> log(10,x), tfr.power)

heatmap(tfr::DSP.Periodograms.TFR) = log10.(power(tfr))

I cannot see why use tfr.power instead of power(tfr), I think the latter (original) is more flexible

Datseris · 2022-03-28T21:55:08Z

I'd say let's wrap this up (i.e., merge it) and return to #10 and see what is remaining to be done?

jparcill · 2024-09-17T00:18:09Z

Hey sorry for abandoning this PR. I'll take a new look at this PR and see if I can get these merged

jparcill · 2024-09-17T00:21:23Z

Going to be a bit for me to regain context on this however

jparcill · 2024-09-17T00:30:33Z

Oh wait looks like this has already been done. I can close this PR then

jparcill added 8 commits August 21, 2020 22:21

Adding the dependencies

7ace4f2

starting work on testing

604a85b

Restoring functionality to audio.jl

aef8ba1

Adding tests for audio.jl

0a7d7bc

Removing Manifest.toml and adding to .gitignore

8b8e446

Working way through TFR.jl and starting util.jl

a481640

Adding in the small tweaks from the PR as well as fixing istft function

167b941

Restoring heatmap function and updating usage of some old functions

78bdf80

Datseris added 2 commits April 20, 2021 20:53

remove uncapitalized project.toml

8780942

Merge branch 'master' into jparcill_writing_tests

1314012

Merge branch 'master' into jparcill_writing_tests

6dfce05

Merge branch 'master' into jparcill_writing_tests

d0c297f

clouds56 reviewed Jun 11, 2021

View reviewed changes

Datseris mentioned this pull request Mar 28, 2022

Music Transcription: GSOC'22 #35

Open

jparcill closed this Sep 17, 2024

		lower = (fftfreqs .- melfreqs[i]) ./ (melfreqs[i+1] .- melfreqs[i])
		upper = (melfreqs[i+2] .- fftfreqs) ./ (melfreqs[i+2] .- melfreqs[i+1])

	heatmap(tfr::DSP.Periodograms.TFR) = map(x -> log(10,x), tfr.power)
	heatmap(tfr::DSP.Periodograms.TFR) = log10.(power(tfr))

Trying to work on functionality for the display and other updates #15

Trying to work on functionality for the display and other updates #15

Conversation

jparcill commented Feb 16, 2021

Datseris commented Feb 16, 2021

jparcill commented Feb 16, 2021

ashwani-rathee commented Apr 20, 2021

Datseris commented Apr 20, 2021 • edited Loading

Datseris commented Apr 20, 2021

Datseris commented Apr 22, 2021

ashwani-rathee commented Apr 22, 2021

ashwani-rathee commented Apr 22, 2021

Datseris commented Apr 22, 2021

haberdashPI commented Apr 22, 2021

Datseris commented Apr 22, 2021

Datseris commented Apr 22, 2021 • edited Loading

jparcill commented Apr 22, 2021

ashwani-rathee commented Apr 23, 2021

ashwani-rathee commented Apr 24, 2021

Datseris commented Apr 24, 2021

ashwani-rathee commented May 9, 2021

jparcill commented May 9, 2021

jparcill commented May 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clouds56 Jun 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clouds56 Jun 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Datseris commented Mar 28, 2022

jparcill commented Sep 17, 2024

jparcill commented Sep 17, 2024

jparcill commented Sep 17, 2024

Datseris commented Apr 20, 2021 •

edited

Loading

Datseris commented Apr 22, 2021 •

edited

Loading

clouds56 Jun 11, 2021 •

edited

Loading

clouds56 Jun 11, 2021 •

edited

Loading