Why does auto result in changing Sample values?

[nwolek]

Noticed this behavior while working on issue #35 between commits https://github.com/jamoma/jamoma2/commit/6b8b76fea618b6e528c9a93cb404aaa0537278d5 and https://github.com/jamoma/jamoma2/commit/95f6ba7bab7686484d0f025db562a78ff1080630. Going to work up a test on a separate branch. Making note so I don't forget.

[nwolek]

OK, I hope this test provides a clear demo for @tap. Let me know if you have questions once you get to it. Time for holiday afternoon with the family. Happy thanksgiving!

[tap]

This is a really good question -- understand the answer and you will understand a lot about C++11. I tripped myself up on this a few months ago too, FWIW.

To reason about this, what is the actual type that is deduced by auto in this line of code?

auto out_samples16_1 = my_phasor16();

The answer (from looking at the function operator overload) is SharedSampleBundleGroup. It is not SharedSampleBundleGroup& or SharedSampleBundleGroup* or anything else. So what is a returned is a copy of the phasor's internal SharedSampleBundleGroup.

Does the above seem straightforward? Probably. And that's the problem. Because what is inside of that copy of the SharedSampleBundleGroup is this: std::shared_ptr<SampleBundleGroup>. So our SharedSampleBundleGroup named out_samples16_1 contains a reference to the samples inside the phasor instance. A little further down we have out_samples16_2 -- which internally contains a reference to the same samples.

Thus in the code, when the phasor calculates an additional vector of audio all of these references will see the updated values and not the values that were calculated at the time that the pointer was assigned.

So what to do? Well, we want to copy the values inside of the SharedSampleBundleGroup, not the pointer to those values inside of the SharedSampleBundleGroup. In fact, it is the whole sharing thing that is getting us in trouble, so we just want a SampleBundleGroup. But do we even need the group? It doesn't seem like it. So we just need a plain old SampleBundles. Like this:

https://github.com/jamoma/jamoma2/blob/master/tests/Delay/Delay.cpp#L54-L56

Using auto makes it easier to stumble into this problem, but really the chances of stumbling on this are pretty high even if you explicitly typed the var in which the output is stored. Is this a design flaw in our API? Or is it just the cost of doing business given the features we are trying to implement? How can this be made less confusing?

[tap]

(apologies if my answer was a bit tedious : I'm treating the answering of issues like this as writing documentation, since that's essentially what the issue-history is...)

[nwolek]

I think your response is great! Makes sense and helps clarify what I was observing. You are absolutely right to treat these questions as opportunities for doc writing. I am going to copy the test code into this thread to archive it too. Should it be merged back into master branch with the expectation that the values would compare as not equivalent?

        void testAutoCreatedSampleBundleGroup() {

            // NW: this behavoir was noticed while working on tests for Phasor
            // it appears that sample values in first auto created vector change
            // as each subseqent vector is processed

            Jamoma::Phasor my_phasor16;

            my_phasor16.channelCount = 1;
            my_phasor16.frameCount = 16;

            my_phasor16.sampleRate = 48000;
            my_phasor16.phase = 0.0;
            my_phasor16.frequency = 1.0;

            // process vector 1 and stash a value
            auto out_samples16_1 = my_phasor16();

            Jamoma::Sample stash_value1 = out_samples16_1[0][0][0];

            // process vector 2 and stash a value
            auto out_samples16_2 = my_phasor16();

            // grab same value from first vector, should be the same?
            Jamoma::Sample stash_value2 = out_samples16_1[0][0][0];

            // process vector 3 and stash a value
            auto out_samples16_3 = my_phasor16();

            // grab same value from first vector, should be the same?
            Jamoma::Sample stash_value3 = out_samples16_1[0][0][0];

            // process vector 4 and stash a value
            auto out_samples16_4 = my_phasor16();

            // grab same value from first vector, should be the same?
            Jamoma::Sample stash_value4 = out_samples16_1[0][0][0];

            // I would expect all these to be equal, but they are NOT
            // Maybe I misunderstand what auto does?
            // But it is also possible auto is creating wrong type (i.e. not ImmutableSampleBundleGroup)
            // Let's be sure.
            mTest->TEST_ASSERT("stashed value 1 = 2", mTest->compare(stash_value1, stash_value2));
            mTest->TEST_ASSERT("stashed value 1 = 3", mTest->compare(stash_value1, stash_value3));
            mTest->TEST_ASSERT("stashed value 1 = 4", mTest->compare(stash_value1, stash_value4));
            mTest->TEST_ASSERT("stashed value 2 = 3", mTest->compare(stash_value2, stash_value3));
            mTest->TEST_ASSERT("stashed value 2 = 4", mTest->compare(stash_value2, stash_value4));
            mTest->TEST_ASSERT("stashed value 3 = 4", mTest->compare(stash_value3, stash_value4));

        }

[nwolek]

I think that preserving the test would give us something to point people to as a demo of this behavior.

[tap]

I have no problem merging that. Thanks @nwolek