Skip to content

Conversation

@messmerd
Copy link
Member

@messmerd messmerd commented Jul 22, 2025

After some experience using the new audio buffer views in #7459 and #7858, it became clear that there was some useful quality-of-life functionality still missing (especially in InterleavedBufferView).

This PR adds that extra functionality we found useful.

EDIT: It looks like some of the commit messages got messed up due to me using backticks from the command-line.

@messmerd messmerd requested a review from sakertooth July 22, 2025 23:40
Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick review, mostly looks fine.

@sakertooth
Copy link
Contributor

I was making an AudioLayoutConverter class for #7568. I ended up needing to implement 4 separate functions, with the only differences being whether it's planar, interleaved, or both planar or both interleaved. I was considering a AudioBufferView that is essentially abstracting both planar and interleaved formats, but I was concerned about the performance and if there is a better way.

@sakertooth
Copy link
Contributor

I could template it I guess.

@messmerd
Copy link
Member Author

messmerd commented Aug 4, 2025

@sakertooth How about this:

template<class T, SampleType U, proc_ch_t channels = DynamicChannelCount>
concept AnyBufferView = std::convertible_to<T, InterleavedBufferView<U, channels>>
    || std::convertible_to<T, PlanarBufferView<U, channels>>;

You could use it like this:

void foo(AnyBufferView<const float, 2> auto in, AnyBufferView<float, 2> auto out)
{
}

Or without using the abbreviated function template syntax:

template<AnyBufferView<const float, 2> In, AnyBufferView<float, 2> Out>
void foo(In in, Out out)
{
}

EDIT: For ease of use, InterleavedBufferView and PlanarBufferView might need to define a static constexpr value or method that indicates whether it is interleaved or not:

void foo(AnyBufferView<const float, 2> auto buffer)
{
    if constexpr (decltype(buffer)::interleaved())
    {
        // `buffer` is interleaved
    }
    else
    {
        // `buffer` is planar
    }
}

Copy link
Contributor

@sakertooth sakertooth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't done any testing but the changes look good as of right now. You can merge if you feel its ready.

@sakertooth
Copy link
Contributor

Thought about it some more, and I think I prefer AudioBufferView rather than AnyBufferView. The audio part still gives context that this is a view over an audio buffer.

@messmerd messmerd merged commit dabfe6f into LMMS:master Aug 6, 2025
11 checks passed
@messmerd messmerd deleted the audio-buffer-view-improvements branch August 6, 2025 18:05
@sakertooth
Copy link
Contributor

Apologies, I should've tested this PR in something like #7858 before approving. I think there was a slight error here: Shouldn't it be possible to convert from InterleavedBufferView<float, 2> to InterleavedBufferView<const float>?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants