Hi everyone, 

some advice needed here.
While marking some student work, I came across an issue in diskin/diskin2 that seems to be long standing
and maybe someone might have come across this and dismissed it.

The code I was looking at was reading a stereo file peaking near 0dbfs, but the output from diskin was
double that (and so causing samples out of range). After several checks, I figured the problem was
with diskin. Then I noticed the code was using the same variable for left and right

asig, asig diskin …

so I went to look at the sources and saw that diskin does some summing to these variables as
part of its process. Normally, you would think that it would just overwrite them, but it is
actually using them for some internal buffering. The result is that if the same variable is used,
we get the strange effect of the two file channels being mixed up (and possibly other effects depending
on diskin/diskin2 parameters).

So I am in two minds about this:

1 - it is user error: we should throw an init error.

2 - it is not user error and diskin is not behaving properly.

I don’t actually like the fact the opcode uses the output variable for buffering, but we probably have
similar cases elsewhere. The simplest thing is to treat it as 1 and document it in the manual.
Interestingly it is a similar issue I have encountered with pass-by-reference UDOs 
(see https://github.com/csound/csound/issues/2061)

What do you think?
========================
Prof. Victor Lazzarini
Maynooth University
Ireland

Well since no one answered earlier, I went with option 2, which wasn’t too difficult to implement and also
it allowed me to fix the “inconsistent channels” init error which always bugged me as a half-assed implementation.

https://github.com/csound/csound/pull/2128

I think both would solutions could potentially break code. Solution 2 won’t make anyone’s code fail like
solution 1. It’s also possible to implement solution 1 on top of this (it’s independent of my fix).

========================
Prof. Victor Lazzarini
Maynooth University
Ireland

> On 12 Apr 2025, at 10:25, Rory Walsh  wrote:
> 
> *Warning*
> This email originated from outside of Maynooth University's Mail System. Do not reply, click links or open attachments unless you recognise the sender and know the content is safe.
> I think option 1 is the best, even though there is a chance that this will break existing instruments. 
> 
> On Fri, 11 Apr 2025 at 10:58, Victor Lazzarini <000010b17ddd988e-dmarc-request@listserv.heanet.ie> wrote:
> Hi everyone, 
> 
> some advice needed here.
> While marking some student work, I came across an issue in diskin/diskin2 that seems to be long standing
> and maybe someone might have come across this and dismissed it.
> 
> The code I was looking at was reading a stereo file peaking near 0dbfs, but the output from diskin was
> double that (and so causing samples out of range). After several checks, I figured the problem was
> with diskin. Then I noticed the code was using the same variable for left and right
> 
> asig, asig diskin …
> 
> so I went to look at the sources and saw that diskin does some summing to these variables as
> part of its process. Normally, you would think that it would just overwrite them, but it is
> actually using them for some internal buffering. The result is that if the same variable is used,
> we get the strange effect of the two file channels being mixed up (and possibly other effects depending
> on diskin/diskin2 parameters).
> 
> So I am in two minds about this:
> 
> 1 - it is user error: we should throw an init error.
> 
> 2 - it is not user error and diskin is not behaving properly.
> 
> I don’t actually like the fact the opcode uses the output variable for buffering, but we probably have
> similar cases elsewhere. The simplest thing is to treat it as 1 and document it in the manual.
> Interestingly it is a similar issue I have encountered with pass-by-reference UDOs 
> (see https://github.com/csound/csound/issues/2061)
> 
> What do you think?
> ========================
> Prof. Victor Lazzarini
> Maynooth University
> Ireland
> 

All diskin opcodes use the same code, so a fix applied to one is applied to all..
========================
Prof. Victor Lazzarini
Maynooth University
Ireland

> On 12 Apr 2025, at 10:46, Dr. Richard Boulanger  wrote:
> 
> Hello Victor and Rory,
> 
> The fixes are and will be important.  Others have noticed 'curious' behaviours with the important and incredibly useful diskin family.
> And so, I super appreciate that you found this and are 'fixing' these.  But maybe, you should create additional diskin opcodes so that 'nothing' breaks (diskinA1, diskinA2 or optional argument to the existing code)
> 
> 
> - Dr.B
> 
> Dr. Richard Boulanger
> Professor
> Electronic Production and Design
> Berklee College of Music
> Professional Writing & Technology Division
> 
> 
> On Sat, Apr 12, 2025 at 5:41 AM Victor Lazzarini <000010b17ddd988e-dmarc-request@listserv.heanet.ie> wrote:
> Well since no one answered earlier, I went with option 2, which wasn’t too difficult to implement and also
> it allowed me to fix the “inconsistent channels” init error which always bugged me as a half-assed implementation.
> 
> https://github.com/csound/csound/pull/2128
> 
> I think both would solutions could potentially break code. Solution 2 won’t make anyone’s code fail like
> solution 1. It’s also possible to implement solution 1 on top of this (it’s independent of my fix).
> 
> ========================
> Prof. Victor Lazzarini
> Maynooth University
> Ireland
> 
> > On 12 Apr 2025, at 10:25, Rory Walsh  wrote:
> > 
> > *Warning*
> > This email originated from outside of Maynooth University's Mail System. Do not reply, click links or open attachments unless you recognise the sender and know the content is safe.
> > I think option 1 is the best, even though there is a chance that this will break existing instruments. 
> > 
> > On Fri, 11 Apr 2025 at 10:58, Victor Lazzarini <000010b17ddd988e-dmarc-request@listserv.heanet.ie> wrote:
> > Hi everyone, 
> > 
> > some advice needed here.
> > While marking some student work, I came across an issue in diskin/diskin2 that seems to be long standing
> > and maybe someone might have come across this and dismissed it.
> > 
> > The code I was looking at was reading a stereo file peaking near 0dbfs, but the output from diskin was
> > double that (and so causing samples out of range). After several checks, I figured the problem was
> > with diskin. Then I noticed the code was using the same variable for left and right
> > 
> > asig, asig diskin …
> > 
> > so I went to look at the sources and saw that diskin does some summing to these variables as
> > part of its process. Normally, you would think that it would just overwrite them, but it is
> > actually using them for some internal buffering. The result is that if the same variable is used,
> > we get the strange effect of the two file channels being mixed up (and possibly other effects depending
> > on diskin/diskin2 parameters).
> > 
> > So I am in two minds about this:
> > 
> > 1 - it is user error: we should throw an init error.
> > 
> > 2 - it is not user error and diskin is not behaving properly.
> > 
> > I don’t actually like the fact the opcode uses the output variable for buffering, but we probably have
> > similar cases elsewhere. The simplest thing is to treat it as 1 and document it in the manual.
> > Interestingly it is a similar issue I have encountered with pass-by-reference UDOs 
> > (see https://github.com/csound/csound/issues/2061)
> > 
> > What do you think?
> > ========================
> > Prof. Victor Lazzarini
> > Maynooth University
> > Ireland
> > 
>