reading buffer~ whose contents might change

Jun 2, 2008 at 7:08pm

reading buffer~ whose contents might change

Hi,

I’m writing an extension that reads two buffer~ objects. During testing, I run the extension while manually replacing the contents of the buffers from different sound files, resulting in the length of the buffers changing. I’ve also observed the *pointer* to the buffer~ samples (b_samples) change. I assume that replacement of the buffer~ contents from a file is not happening from the performance thread. This means that at any point during the execution of my perform function, the buffer~ referenced by my code can change, invalidating certain assumptions my code makes. I suspect that several crashes I’ve had are due to this.

Among the fixes I’ve tried are reliance on the state of the b_valid and b_inuse flags of the buffer~ objects. If b_valid is false, I don’t do any dsp during that perform call. I save and restore the state of the b_inuse flag, as shown in the index~ SDK example, and set this flag while doing my dsp.

Judging from “buffer.h,” if I avoid reading the buffer while b_valid is false, then I shouldn’t have any trouble. However, it’s easy to imagine this happening…

1. check b_valid
2. it’s true, so start my dsp process
3. b_valid set to false by another thread during my processing
4. length of buffer shortens, or b_samples pointer changes
5. crash, as my processing code relies on invalid assumptions

One way around this would be for my perform function to set b_valid to false before doing dsp and restore it afterward, assuming that all other objects using the buffer would stay away from it while b_valid is false. Seems kind of heavy-handed for an extension that is only reading the buffer, though. Plus, you could see a similar situation from the other function’s point of view (b_valid set to false after it’s already begun replacing buffer).

Another workaround would be for my processing code to have direct access to the crucial buffer fields (b_samples, b_frames, b_nchans), so that any update to them would be noticed in time to protect against a crash. But since my code comprises several large-ish C++ classes, I’m reluctant to do it this way.

Do I have to use the global critical region here? My processing code is fairly compute-intensive, so I was hoping for a lockless solution.

Any ideas?

Thanks,
John

#38212
Jun 7, 2008 at 2:45pm

> Judging from “buffer.h” …

this is symptomatic of the sdk’s sparse documentation… too many times i’ve had to go hunting through header files trying to guess at how things work. eventually i found it somewhat useful to run doxygen on the headers to make a nice html index of all the functions and types, etc… but even that is only helpful to a certain extent. there are still a lot of things that need to be guessed at… the dictionary stuff… the linked list stuff, some of the client-server stuff… and so on.
i’m really really hoping that the next sdk that comes out will be better documented…
sorry i can’t help with the buffer problems, but i’ve had some difficulty there myself.

#132907
Jun 8, 2008 at 4:26pm

Thanks, thezer0ist. I agree, it’d be great if the Max 5 SDK would clarify some of these more obscure things.

About my problem, normally I wouldn’t care so much about the theoretical possibility of this threading issue causing a crash. Right now, my extension is running well, and I haven’t seen any crashes since handling the b_valid flag in the way I’ve seen in other extensions. But I’m sure the first time it will crash will be when it’s used during a performance!

I’d at least like someone who understands critical regions well in Max/MSP to weigh in on whether this is the way to solve my problem. You’d have to have access to the source code for buffer~ to say so with confidence, I think.

-John

#132908
Jun 15, 2008 at 3:46pm

Quote: John Gibson wrote on Mon, 02 June 2008 13:08
—————————————————-

>> Do I have to use the global critical region here? My processing code is fairly compute-intensive, so I was hoping for a lockless solution.

You shouldn’t need to do this.

>>I’ve also observed the *pointer* to the buffer~ samples (b_samples) change. I assume that replacement of the buffer~ contents from a file is not happening from the performance thread. This means that at any point during the execution of my perform function, the buffer~ referenced by my code can change, invalidating certain assumptions my code makes. I suspect that several crashes I’ve had are due to this.

Actually – although another thread may be called at the same time (in a multiprocessor situtation and given the correct DSP Status settings, the audio thread shouldn’t be interrupted otherwise) – if you set b_inuse then no other external should alter a buffer~s length or contents (I’ve verified this behaviour in a very ad-hoc way when I accidentally set it permanently and was then unable to replace the contents of the buffer).

Of course a third party external could ignore this, but all c74 externals should behave correctly.

>> I save and restore the state of the b_inuse flag, as shown in the index~ SDK example, and set this flag while doing my dsp.

This is the correct procedure asfaik and should cover you. Any crashes you experience after correctly implementing this should be due to other things….

Regards,

Alex

#132909
Jun 16, 2008 at 7:47pm

Quote: AlexHarker wrote on Sun, 15 June 2008 09:46
—————————————————-
> Quote: John Gibson wrote on Mon, 02 June 2008 13:08
> —————————————————-
>
> >> Do I have to use the global critical region here? My processing code is fairly compute-intensive, so I was hoping for a lockless solution.
>
> You shouldn’t need to do this.
>
> >>I’ve also observed the *pointer* to the buffer~ samples (b_samples) change. I assume that replacement of the buffer~ contents from a file is not happening from the performance thread. This means that at any point during the execution of my perform function, the buffer~ referenced by my code can change, invalidating certain assumptions my code makes. I suspect that several crashes I’ve had are due to this.
>
> Actually – although another thread may be called at the same time (in a multiprocessor situtation and given the correct DSP Status settings, the audio thread shouldn’t be interrupted otherwise) – if you set b_inuse then no other external should alter a buffer~s length or contents (I’ve verified this behaviour in a very ad-hoc way when I accidentally set it permanently and was then unable to replace the contents of the buffer).

Thanks for your input, Alex. What I have now does seem to be working reliably. But your answer makes me wonder…

Should I check the b_inuse flag and skip processing if it’s set? Currently I don’t do that (and neither do other code examples I’ve seen). I merely set it to say to other objects, “hey, I’m using this now.” I *do* skip processing if the b_valid flag is set, though.

But if b_inuse is the critical flag, it seems that buffer~ could already be in the process of altering its contents before I set b_inuse. So setting that would not stop the buffer~ contents from changing, and this would invalidate assumptions made in my dsp routine.

I can see a similar situation with my use of the b_valid flag, which I check but never set.

Wouldn’t it be better if there were one flag that you both check and set? As in…

if (!inuse) { inuse = 1; process(); inuse = 0; }
else don’t do any dsp

-John

#132910
Jun 16, 2008 at 10:29pm

On 16 juin 08, at 21:47, John Gibson wrote:

> Thanks for your input, Alex. What I have now does seem to be working
> reliably. But your answer makes me wonder…
>
> Should I check the b_inuse flag and skip processing if it’s set?
> Currently I don’t do that (and neither do other code examples I’ve
> seen). I merely set it to say to other objects, “hey, I’m using
> this now.” I *do* skip processing if the b_valid flag is set, though.
>
> But if b_inuse is the critical flag, it seems that buffer~ could
> already be in the process of altering its contents before I set
> b_inuse. So setting that would not stop the buffer~ contents from
> changing, and this would invalidate assumptions made in my dsp
> routine.
>
> I can see a similar situation with my use of the b_valid flag, which
> I check but never set.

Hey,

I didn’t follow the thread properly but you definitely need to check
the b_valid and the b_inuse. In the future Max 5 SDK, you’ll see that
there are macro to increment/decrement b_inuse properly, in a thread
safe way.

ej

#132911
Jun 16, 2008 at 11:01pm

Quote: Emmanuel Jourdan wrote on Mon, 16 June 2008 16:29
—————————————————-
> On 16 juin 08, at 21:47, John Gibson wrote:

> I didn’t follow the thread properly but you definitely need to check
> the b_valid and the b_inuse. In the future Max 5 SDK, you’ll see that
> there are macro to increment/decrement b_inuse properly, in a thread
> safe way.

In Max 4 I believe you can assume that audio processing cannot be interrupted by scheduler processing (only the other way round) and that NO audio processing is multithreaded. I also believe that all scheduler based buffer operations that make the buffer invalid / in use will set b_valid to false. Hence this stuff should be enough (after all there’s nothing else except this approach in the SDK / examples / all the buffer-based code I’ve seen).

However, the new max 5 poly~ object means that you must assume that all your dsp code needs to be properly threadsafe (as audio processing may be multithreaded). This requires the kind of system ej describes, and it would seem that the c74 guys have implemente this in the (as yet) unreleased 5 SDK. So, if full 5 compatibility is an issue then you may need to wait. Also asfaik the situation regarding reading/writing should be different, as reading from the buffer shouldn’t block multithreaded processes from reading (that would seem inefficient) – in a situation where writing is involved this may be necessary… I guess the release of the 5 SDK will make it clear how the official max externals are dealing with all this..

Regards,

Alex

#132912
Jun 18, 2008 at 3:03am

Quote: Emmanuel Jourdan wrote on Mon, 16 June 2008 16:29
—————————————————-

> Hey,
>
> I didn’t follow the thread properly but you definitely need to check
> the b_valid and the b_inuse. In the future Max 5 SDK, you’ll see that
> there are macro to increment/decrement b_inuse properly, in a thread
> safe way.
>
> ej

Thanks for the Max5 tip, ej. I look forward to having a cleaner way of doing this.

-John

#132913
Jun 18, 2008 at 3:08am

Quote: AlexHarker wrote on Mon, 16 June 2008 17:01
—————————————————-

> In Max 4 I believe you can assume that audio processing cannot be interrupted by scheduler processing (only the other way round) and that NO audio processing is multithreaded.

Ah, so that means my current approach is fine for Max4. Thanks for the reassurance. ;-)

> I also believe that all scheduler based buffer operations that make the buffer invalid / in use will set b_valid to false. Hence this stuff should be enough (after all there’s nothing else except this approach in the SDK / examples / all the buffer-based code I’ve seen).
>
> However, the new max 5 poly~ object means that you must assume that all your dsp code needs to be properly threadsafe (as audio processing may be multithreaded). This requires the kind of system ej describes, and it would seem that the c74 guys have implemente this in the (as yet) unreleased 5 SDK. So, if full 5 compatibility is an issue then you may need to wait.

I will certainly want to take full advantage of whatever new APIs there are for handling thread safety in Max5. I had read somewhere about Max5 poly~ threading and figured that this would introduce a new wrinkle.

> Also asfaik the situation regarding reading/writing should be different, as reading from the buffer shouldn’t block multithreaded processes from reading (that would seem inefficient) – in a situation where writing is involved this may be necessary…

Yes, I agree with all this.

> I guess the release of the 5 SDK will make it clear how the official max externals are dealing with all this..

Let’s hope!

Thanks for your advice.

-John

#132914

You must be logged in to reply to this topic.