Another case where only the first channel of a multi-channel Inst is audible. #163

Closed
rogerallen opened this Issue Oct 9, 2012 · 9 comments

Projects

None yet

3 participants

@rogerallen
Member

This appears to be a variant on Issue #94.

I'm using a pretty recent 0.8.0-SNAPSHOT on a Mac. I extended Kevin's testcases from Issue #94 to show that while it works for his case, it fails with the new definst testcase.

I'd expect test-synth1 and test-inst1 to sound the same, but the 2nd channel is missing on test-inst1.

My use-case is more complex than this, but I tried to reduce to the simplest repro.

(use 'overtone.live)

(def lo-freq 200)
(def hi-freq 300)

;; original testcases
(defsynth test-synth [] (out 0 (sin-osc [lo-freq hi-freq])))

(definst test-inst [] (sin-osc [lo-freq hi-freq]))

;; new testcases showing the failure
;; this one works
(defsynth test-synth1 []
  (let [s2 [(sin-osc lo-freq)
            (sin-osc hi-freq)]]
    (out 0 s2)))

;; this does not create a stereo instrument
(definst test-inst1 []
  (let [s2 [(sin-osc lo-freq)
            (sin-osc hi-freq)]]
    s2))

(comment
  "Try these with headphones on."

  "Both tones are audible; lo-freq on the left; hi-freq right."
  (test-synth)
  (stop)

  "FIXED: Both tones are audible; lo-freq on the left; hi-freq right."
  (test-inst)
  (stop)

  "Both tones are audible; lo-freq on the left; hi-freq right."
  (test-synth1)
  (stop)

  "BROKEN: One tone is audible; lo-freq in the center."
  (test-inst1)
  (stop)

  )
@rogerallen
Member

Woah, this is breaking my noodle...

I was trying to experiment with the following code. [before adding this code, be sure you have reproduced the problem above and are satisfied that it fails like I am claiming]

(definst test-inst2 []
  (let [s2 [(sin-osc lo-freq)
            (sin-osc hi-freq)]]
    (pan2 s2 0.0 1.0)))

I thought that adding the pan2 might make a difference, but it did not sound any different when I did

 (test-inst2)
 (stop)

BUT, now go back and try out

  (test-inst1)
  (stop)

for me, the (test-inst1) now works! (it sounds like (test-synth1)

How can defining a new instrument cause an earlier instrument to change? Very wacky things are afoot.

@samaaron
Member

Hi Roger,

I would advise you don't use definst whilst you are learning things. It does a bunch of magic which ( I think) is non-obvious to someone just getting started. definst adds both a pan2 and out ugen to the synth design if they're not there. In the case of your test-inst1, the secretly added pan2 ugen only takes one input and makes that stereo, in which case only the first of the array of signals in s2 is passed through to the out ugen and hence to the soundcard.

One thing to note is that if you pass a vector of signals, Overtone will perform what's called 'multi-channel expansion' and essentially duplicate the signal channels for each element of the vector. When the out ugen receives a number of channels, it will automatically place each channel on a subsequent bus starting at the supplied value. This is why passing a vector of two signals in s2 to the out ugen in test-synth1 works.

All of this is expected behaviour - I'm sorry that it's non-obvious and therefore poorly documented/communicated.

@samaaron samaaron closed this Oct 12, 2012
@rogerallen
Member

Okay, I thought that the work done on #94 would apply to this case. My confusion seems to come from not understanding the difference between (sin-osc [400 600]) and [(sin-osc 400) (sin-osc 600)]. They must be different beasts under the hood.

I would think that a fully stereo instrument without the pan2 would be useful for Overtone...maybe that should be explicit in the definst macro if it is difficult to infer. (def-stereo-inst?)

But, as you point out, I can get past this with defsynth...will do.

@neatonk
Member
neatonk commented Oct 14, 2012

Hi Roger,

With multichannel expansion (sin-osc [400 600]) should be equivalent to [(sin-osc 400) (sin-osc 600)] so your confusion is completely understandable. I'll echo Sam's advice here and recommend using of defsynth over definst anytime you're not taking advantage of the more advanced signal routing and fx chaining that definst provides.

That said, I had a look at the issue today and found that (definst [] [(sin-osc 400) (sin-osc 600)]) is being wrongly identified as a mono inst and being assigned to a mono bus as a result. This is responsible for both the missing channel and it's strange reappearance after defining another inst.

I'm working on a fix for this now.

@neatonk neatonk reopened this Oct 14, 2012
@neatonk neatonk added a commit that closed this issue Oct 14, 2012
@neatonk neatonk Fix #163 Another case where only the first channel of a multi-channel…
… Inst is audible.

* Thanks to Roger Allen for catching this bug.
98cb062
@neatonk neatonk closed this in 98cb062 Oct 14, 2012
@rogerallen
Member

Thanks, Kevin!

@rogerallen
Member

Oh, I think there may be an issue with that fix, Kevin. With your changelist, if I

(use 'overtone.inst.synth)

then I get:

No more ids! Unable to allocate a sequence of ids of length: 12
  [Thrown class java.lang.Exception]

Restarts:
 0: [QUIT] Quit to the SLIME top level

Backtrace:
  0: allocator.clj:51 overtone.sc.machinery.allocator/find-gap
  1: allocator.clj:55 overtone.sc.machinery.allocator/find-gap
  2: allocator.clj:55 overtone.sc.machinery.allocator/find-gap
  3: allocator.clj:55 overtone.sc.machinery.allocator/find-gap
 ...

If I revert back to the previous changelist

git checkout 998bc0f0e063bf5a51450e544e203e86f0c29d94

I don't get the error.

@neatonk neatonk added a commit that referenced this issue Oct 14, 2012
@neatonk neatonk Revert "Fix #163 Another case where only the first channel of a multi…
…-channel Inst is audible."

This change caused an excessive number of id's to be allocated when
compiling some of the insts that are packaged with overtone.

This reverts commit 98cb062.

reopen #163
12ec0a1
@neatonk
Member
neatonk commented Oct 14, 2012

Thanks Roger. I should have some time to look at this again today, I've reverted the change for now.

@neatonk neatonk reopened this Oct 14, 2012
@neatonk neatonk was assigned Oct 14, 2012
@neatonk neatonk added a commit that closed this issue Oct 14, 2012
@neatonk neatonk Actually Fix #163 9568b27
@neatonk neatonk closed this in 9568b27 Oct 14, 2012
@neatonk
Member
neatonk commented Oct 14, 2012

That should do it Roger. It took some digging to figure out, but it was still just a one line change!

@rogerallen
Member

Fantastic. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment