openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.84k stars 2.56k forks source link

AVEngine - issue with soundPlayerExample #7931

Closed dimitre closed 1 month ago

dimitre commented 2 months ago

if you click any sample and wait until it stops it works OK if you click again before the first sound is stopped it crashes in the first line of stop function:

- (void)stop {

    __typeof(self) __weak weak_self = self;

    if(_isSessionInterrupted || _isConfigChangePending){
        dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.3f), dispatch_get_main_queue(), ^{
            [weak_self stop];
        });
        return;
    }

    if([self isValid]) {
        [self.soundPlayer stop];
    }
    _bIsPlaying = NO;

    [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(playloop) object: self.soundPlayer];
    [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(stop) object: self.soundPlayer];

    self.startedSampleOffset = 0;
}
artificiel commented 2 months ago

is that iOS? (on macOS OF_NO_FMOD=1 still gives errors with some AVAudioSession things being unavailable)

how can I confirm I'm really getting the AVEngine back end?

dimitre commented 2 months ago

Not iOS, it is macOS. I've #ifdef out out the parts that were erroring, I think they are specific to iOS

dimitre commented 2 months ago

to test this I've copied the example to another project, adding this on App.xcconfig

LIB_FMOD=""
OF_NO_FMOD=1
USER_PREPROCESSOR_DEFINITIONS="OF_NO_FMOD=1"

the line LIB_FMOD one was unexpected, but this was needed to make it work you can know it is using AVEngine in two different ways: one is reproduce the error and XCode will lead you to __typeof(self) __weak weak_self = self; or you can inspect the .app folder and certify there is no fmod there

ofTheo commented 2 months ago

Hmm. That's weird, the multi play stuff used to work well. Will try and reproduce with the nightlies.

artificiel commented 2 months ago

hmm the state machine of that class is hard to follow with the threads, delays etc especially when multi-play, I don't get where how the superposed instances are tracked?

anyhow here it does not crash easily (M1, 14.4.1). I have to click a lot (like 100 clicks in 12 seconds) then I get the above weak self crash.

adding this to ofAVEngineSoundPlayer.mm stops the crashes:

- (void)stop {
    if (!_bIsPlaying) {
        NSLog(@"stop() called before play()");
        return;
    }

but it feels like a "patch" -- maybe the design needs a bit more robustness.

ofTheo commented 2 months ago

@dimitre I can reproduce here - its crashing here, where the multiplayer objects that are done playing are deleted ( ie the old vector of multiplayer objects gets replaced with the new one ).

I get this message: objc[26616]: Cannot form weak reference to instance (0x60000002c2d0) of class AVEnginePlayer. It is possible that this object was over-released, or is in the process of deallocation.

image

I remember stress testing this pretty hard and multiplay used to work, but maybe the ARC changes mean something is getting deleted twice.

dimitre commented 2 months ago

Nice! I think @artificiel patch can be applied so things work OK for now. cc: @2bbb

artificiel commented 2 months ago

yes _bIsPlaying being false it indicates the sound is not playing or -- presumably what happens in the crash --not done getting initialized, so should not be deallocated yet.

in which case my patch means "dropping a request" for a sound to play (because play calls stop); ideally things would be locked and pass through once the sound becomes stoppable, presumably a buffer size later or something like that. the patch is good to confirm the problem is there (stopping a sound that's not playing), but the real solution might be in redesign.

but as mentioned the reentrancy features of that class eludes my quick analysis.

dimitre commented 2 months ago

Yes, please submit a PR we can use a comment with //FIXME: mentioning the needed redesign

dimitre commented 2 months ago

@artificiel tested here and working. no crashes

artificiel commented 2 months ago

ok I can look more but I believe @ofTheo wrote that might be more efficient to take a look at it in light of the info here?

also an observation: the crash don't occur on click, but at the end of the sound. it seems there is something going on deallocation that gets somehow invalidated during the lifetime of the dispatch_after. (maybe the runtime crash error message is not totally correct)

artificiel commented 2 months ago

in other words: image

ofTheo commented 2 months ago

wow - @artificiel , beat me to it! :D

this also works for me:

image

but I am not sure if this hides the main bug, but possibly keeps it for the dispatch ( when audio outputs are changed mid playback ).

artificiel commented 2 months ago

I'm not sure what becomes self in the dispatch — is it implicitly captured in the block? maybe even (don't schedule a stop if not playing):

image

anyhow the fix is better, and considering the difficulties of getting audio session/config changes right (even some pro audio software don't react that well while processing is undergoing) maybe a low-priority problem for OF.

(also, the #ifdef'd stuff to get to compile on Mac should also be committed?)

ofTheo commented 2 months ago

Thanks @artificiel - yeah I notice that now it doesn't work anymore when unplugging headphones.

I did have it working where it paused/stopped all audio and recreated it for the new engine and picked up where it left off. I basically had it working better than some of Apple's own software which would just stop the audio on device switch.

But I'll look into that separately.

Thanks for tracking this issue down!! I can do a PR for the fix, unless you prefer to do so.

artificiel commented 2 months ago

please do the PR!