vadymmarkov / Beethoven

:guitar: A maestro of pitch detection.
https://github.com/vadymmarkov
Other
825 stars 146 forks source link

Requires `start` twice #45

Closed orkenstein closed 6 years ago

orkenstein commented 7 years ago

By some reason mic recognition start only when start called twice:

class ProcessingUnit: NSObject {

  var pitchChangedHandler: ((String) -> ())?

  let pitchEngine: PitchEngine = {
    let config = Config()
    let engine = PitchEngine(config: config, delegate: nil)
    return engine
  }()

  override init() {
    super.init()
    pitchEngine.delegate = self
    pitchEngine.start()
    pitchEngine.start()
  }

}

extension ProcessingUnit: PitchEngineDelegate {

  func pitchEngineDidReceiveError(_ pitchEngine: PitchEngine, error: Error) {

  }

  func pitchEngineWentBelowLevelThreshold(_ pitchEngine: PitchEngine) {

  }

  func pitchEngineDidReceivePitch(_ pitchEngine: PitchEngine, pitch: Pitch) {
    let value = pitch.note.letter.rawValue + "\(pitch.note.octave)"
    pitchChangedHandler?(value)
  }
}
vadymmarkov commented 7 years ago

@orkenstein Did you get a system popup asking for microphone permissions? Or maybe if you try to print an error in pitchEngineDidReceiveError delegate method it will put some light on this.

orkenstein commented 7 years ago

Yep, it asked for the permission. No error from delegate. These same thing for the example app: it works when you tap start for the second time.

11 янв. 2017 г., в 22:09, Vadym Markov notifications@github.com написал(а):

@orkenstein Did you get a system popup asking for microphone permissions? Or maybe if you try to print an error in pitchEngineDidReceiveError delegate method it will put some light on this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

holeg commented 7 years ago

Same happens to me. Only works when calling start 2 times.

makuearth commented 7 years ago

Same here. However, a usleep of 3 seconds after initialising and prior to calling start also solves the problem as well.

thbwd commented 7 years ago

Sometimes calling start simply doesn’t seem to have any effect. Any news on this?

DebanjanChakrabortyCN commented 7 years ago

Same here. I initialzed the AudioEngine, added the config, but the delegates never get called, even though I've added pitchEngine.start()

0a1c commented 6 years ago

This happens to me as well, and with one additional issue. When I call pitchEngine.start() twice (to get it to start at all), the app crashes sometimes when I call pitchEngine.stop().

Any way to fix this?

vadymmarkov commented 6 years ago

Hi @anandchandra50, I'm not sure why this happens, but I'll try to test and find out the problem when I get some time.

makuearth commented 6 years ago

I fixed the problem by inverting the initialization calls in InputSignalTracker.swift. On line 71 the code now reads

try audioEngine?.start()
captureSession.startRunning()
guard captureSession.isRunning == true else {
    throw InputSignalTrackerError.inputNodeMissing
}

Note that audioEngine?.prepare() is not needed as it is called by audioEngine?.start().

Hope this helps.

Anyhow, a big THANKS to vadymmarkov for this implementation!!!!

Has anyone some input on how to make YIN more robust with respect to to the transients during note attack and pitch changes?

vadymmarkov commented 6 years ago

That's great @makuearth! Would be awesome if you could make a PR with your fix.

vadymmarkov commented 6 years ago

@anandchandra50 I merged https://github.com/vadymmarkov/Beethoven/pull/55. Could you test the framework from master to verify that it's fixed now?

0a1c commented 6 years ago

@vadymmarkov I implemented makuearth's change -- if that's the only difference, then it is fixed.

makuearth commented 6 years ago

Glad to hear that it worked. I made a PR in a fork.

Cheers, Martin

On 5 Jan 2018, at 03:53, Anand Chandra notifications@github.com wrote:

@vadymmarkov I implemented makuearth's change -- if that's the only difference, then it is fixed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

vadymmarkov commented 6 years ago

Awesome, then I'm closing this issue. Thanks @makuearth

0a1c commented 6 years ago

Going back to makuearth's point, though, is there any way to make YIN more accurate when pitch changes?