# Possible bug in AudioSampleBuffer

**URL:** <https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887>\
**Category:** Audio Plugins\
**Created:** [July 29, 2013, 8:14am UTC](https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887 "2013-07-29T08:14:41Z")\
**Posts on this page:** 6\
**Page:** 1

<div class="post-metadata">

**Author:** ![andy-cytomic](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.juce.com/andy-cytomic/32/1838_2.png) [@andy-cytomic](https://forum.juce.com/u/andy-cytomic)\
**Post date:** [July 29, 2013, 8:14am UTC](https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887/1 "2013-07-29T08:14:41Z")

</div>

I’ve been using the AudioSampleBuffer class as a container for pre-allocated external buffers, so its allocoatedBytes member is zero.

From the header:

```auto
    /** Copies another buffer.

        This buffer will make its own copy of the other's data, unless the buffer was created
        using an external data buffer, in which case boths buffers will just point to the same
        shared block of data.
    */
    AudioSampleBuffer (const AudioSampleBuffer& other) noexcept;
```

It looks like it’s meant to have the behaviour I expect, so I can use copy constructors to other functions and just get the pointers copied, but in the constructor it looks like a deep copy is always done:

```auto
AudioSampleBuffer::AudioSampleBuffer (const AudioSampleBuffer& other) noexcept
  : numChannels (other.numChannels),
    size (other.size)
{
    allocateData();

    for (int i = 0; i < numChannels; ++i)
        FloatVectorOperations::copy (channels[i], other.channels[i], size);
}
```

Should it be more like this?

```auto
AudioSampleBuffer::AudioSampleBuffer (const AudioSampleBuffer& other) noexcept
  : numChannels (other.numChannels),
    size (other.size),
    allocatedBytes (other.allocatedBytes)
{
    if (allocatedBytes == 0 )
    {
        allocateChannels (other.channels, 0);
    }
    else
    {
        allocateData();

        for (int i = 0; i < numChannels; ++i)
            FloatVectorOperations::copy (channels[i], other.channels[i], size);
    }
}
```

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [July 29, 2013, 8:55am UTC](https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887/2 "2013-07-29T08:55:24Z")

</div>

Thanks Andy! Yes… strange that it doesn’t already do that, but I think you’re right - I’ll get on the case and have a good look at that, cheers!

---

<div class="post-metadata">

**Author:** ![andy-cytomic](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.juce.com/andy-cytomic/32/1838_2.png) [@andy-cytomic](https://forum.juce.com/u/andy-cytomic)\
**Post date:** [July 29, 2013, 2:23pm UTC](https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887/3 "2013-07-29T14:23:47Z")

</div>

Also the behaviour of operator= should be consistent as well with what the copy constructor does, and perhaps have a specific duplicate / deep copy and shallow copy / reference call, even keeping the current names of copyFrom and setDataToReferTo but both taking const AudioSampleBuffer& as parameters.

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [July 29, 2013, 3:41pm UTC](https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887/4 "2013-07-29T15:41:16Z")

</div>

Hmm… I’d be reluctant to change that, in case people are relying on the copy operator to actually make a deep copy of the data. I was happy to change the constructor because like you pointed out, the comments clearly say that it won’t make a deep copy, but that’s not the case for the operator=, so a change could break legitimate code. Seems to me that having setDataToReferTo() to do a shallow copy, and operator= to do a deep copy should cover all the use-cases there.

---

<div class="post-metadata">

**Author:** ![andy-cytomic](https://sea2.discourse-cdn.com/flex026/user_avatar/forum.juce.com/andy-cytomic/32/1838_2.png) [@andy-cytomic](https://forum.juce.com/u/andy-cytomic)\
**Post date:** [July 29, 2013, 4:00pm UTC](https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887/5 "2013-07-29T16:00:46Z")

</div>

Having different behaviour for the copy constructor and equals operator is a dangerous game to play, and although everyone may be a c++ expert some may get confused thinking that they have a copy of the data when they do this:

```auto
AudioSampleBuffer b1 (externaldata, numchans, len); // point b1 to some external data
AudioSampleBuffer b2 = b1; // this will call the copy constructor and so make a shallow copy
```

but what they actually need to do is:

```auto
AudioSampleBuffer b1 (externaldata, numchans, len); // point b1 to some external data
AudioSampleBuffer b2;
b2 = b1; // this will call the equals operator and so make a deep copy
```

---

<div class="post-metadata">

**Author:** ![jules](https://avatars.discourse-cdn.com/v4/letter/j/41988e/32.png) [@jules](https://forum.juce.com/u/jules)\
**Post date:** [July 29, 2013, 4:08pm UTC](https://forum.juce.com/t/possible-bug-in-audiosamplebuffer/10887/6 "2013-07-29T16:08:30Z")

</div>

Yes, I can’t argue with that - it’s a tricky choice. I think possibly the only really safe thing to do here would be to remove the operator= and replace it with a method explicitly called “createCopy” or something that explains what it’s going to do, but I’m not keen on doing that either.