[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Sc-devel] Serious bugs in Buffer :loadCollection and :sendCollection



2008/2/4, Scott Wilson <i@xxxxxxxxxxxxxx>:
>
> On 4 Feb 2008, at 14:49, Dan Stowell wrote:
>
> > Hi -
> >
> > First the mildest bug: Buffer:loadCollection or :sendCollection warns
> > if the collection is bigger than the buffer can hold - but it doesn't
> > take account of multichannel, so it can easily print unwarranted
> > warnings if your buffer is not mono.
> >
> > While looking at this I found more serious problems. sendCollection's
> > second argument should be the start *frame*, but its effect is to set
> > the start *sample*. Watch it go wrong:
> >
> > q = Buffer.alloc(s, 10, 4); // Quadraphonics
> > q.zero;
> > q.plot; // empty, yes? good
> >
> > // "5" here should be the start frame, so this 4-value array should
> > set one full frame
> > q.sendCollection([0.2, 0.4, 0.6, 0.8], 5);
> > // But it's actually the start sample, so the array is spread over
> > 2 frames
> > q.plot;
> >
> > // Try it with loadCollection instead, this time frame-alignment is
> > as expected
> > q.zero
> > q.loadCollection([0.2, 0.4, 0.6, 0.8], 5);
> > q.plot;
> >
> >
> >
> > Oh and another problem. Buffer.sendCollection (i.e. the class-method
> > not the instance-method) mishandles multichannel too:
> >
> > // Let's create a 2-channel buffer, with 6 samples.
> > // Therefore 3 frames, yes?
> > t = Buffer.sendCollection(s, [0.2, 0.4, 0.6, 0.8, 1.0, 0.8], 2);
> > // No:
> > t.plot
> > t.numFrames
> >
> >
> > Patch attached should fix this (run "patch -p4 < patchBuffer.patch"),
> > not committing it yet though since we're in RC stages ;) However,
> > either I'm going mad or these are quite significant problems (and I
> > don't understand why they haven't been found before). Thoughts?
>
> Warning thing is a bug.
>
> startFrame vs. startSample is an error in documentation I think.

Well for sendCollection the documentation, the arg-name and the
analogy with loadCollection all say it should be a frame-index rather
than a sample-index, so I'd argue it's the implementation's fault not
the docs'.


> *sendCollection thing is a bug, definitely.
>
> I have a sinking suspicion that this is all my fault, since I believe
> I added and documented these methods sometime back around 1972. :-(

1972? Makes sense. You must have been distracted by the introduction
of the first scientific hand-held calculator and the nationalisation
of the Iraq Petroleum Company.


> Your patch seems reasonable. Best to stick with startFrame I think.
>
> Believe it or not, probably the reason this has never come up is just
> that nobody has tried to do the exact things that cause the bugs.

Yes, seems weird. Are these serious enough to warrant a new RC? I find
them "serious" because of how commonly buffers are used, but then if
people don't often hit these bugs...?

Dan