BUG in pspAudioLib

[sakya]

Hi! :)

I think I found a bug in pspAudioLib.
This code just init and then end the audio lib but after two init-end the init fails (cannot create callback threads).

Code: Select all

int main(){
    int count = 0;
	pspDebugScreenInit();
	SetupCallbacks();

    pspDebugScreenPrintf("Eboot test\n");
    pspDebugScreenPrintf("Press X to run pspAudioInit and pspAudioEnd\n");

    SceCtrlData pad;
    while(runningFlag){
        sceCtrlReadBufferPositive(&pad, 1);
        if (pad.Buttons & PSP_CTRL_CROSS){
            pspDebugScreenSetXY(0, 5);
            pspDebugScreenPrintf("Count: %i\n", ++count);
            if (pspAudioInit())
                pspDebugScreenPrintf("pspAudioInit ERROR!!!\n");
            else
                pspAudioEnd();
        }
        sceKernelDelayThread(200000);
    }
	sceKernelExitGame();
    return 0;
}

The problem (I think) is that pspAudioEnd isn't really deleting threads (so when the maximum is reached the init fails).

I fixed this by just adding a "flag" in this structure

Code: Select all

typedef struct {
  int threadhandle;
  int threadactive;   <-- THIS
  int handle;
  int volumeleft;
  int volumeright;
  pspAudioCallback_t callback;
  void *pdata;
&#125; psp_audio_channelinfo;

I changed the AudioChannelCallback to manage the new flag:

Code: Select all

static int AudioChannelThread(int args, void *argp)
{
        volatile int bufidx=0;
        int channel=*(int *)argp;

        AudioStatus&#91;channel&#93;.threadactive = 1;      <-- HERE
        while &#40;audio_terminate==0&#41; &#123;
                void *bufptr=&audio_sndbuf&#91;channel&#93;&#91;bufidx&#93;;
                pspAudioCallback_t callback;
                callback=AudioStatus&#91;channel&#93;.callback;
                if &#40;callback&#41; &#123;
                        callback&#40;bufptr, PSP_NUM_AUDIO_SAMPLES, AudioStatus&#91;channel&#93;.pdata&#41;;
                &#125; else &#123;
                        unsigned int *ptr=bufptr;
                        int i;
                        for &#40;i=0; i<PSP_NUM_AUDIO_SAMPLES; ++i&#41; *&#40;ptr++&#41;=0;
                &#125;
                pspAudioOutBlocking&#40;channel,AudioStatus&#91;channel&#93;.volumeleft,AudioStatus&#91;channel&#93;.volumeright,bufptr&#41;;
                bufidx=&#40;bufidx?0&#58;1&#41;;
        &#125;
        AudioStatus&#91;channel&#93;.threadactive = 0;        <-- HERE
        sceKernelExitThread&#40;0&#41;;
        return 0;
&#125;

and waited for this flag in pspAudioEnd before sceKernelDeleteThread:

Code: Select all

void pspAudioEnd()
{
        int i;
        audio_ready=0;
        audio_terminate=1;

        for &#40;i=0; i<PSP_NUM_AUDIO_CHANNELS; i++&#41; &#123;
                if &#40;AudioStatus&#91;i&#93;.threadhandle != -1&#41; &#123;
                        //sceKernelWaitThreadEnd&#40;AudioStatus&#91;i&#93;.threadhandle,NULL&#41;;
                        while &#40;AudioStatus&#91;i&#93;.threadactive&#41;    <-- HERE
                            sceKernelDelayThread&#40;100000&#41;;
                        sceKernelDeleteThread&#40;AudioStatus&#91;i&#93;.threadhandle&#41;;
                &#125;
                AudioStatus&#91;i&#93;.threadhandle = -1;
        &#125;

        for &#40;i=0; i<PSP_NUM_AUDIO_CHANNELS; i++&#41; &#123;
                if &#40;AudioStatus&#91;i&#93;.handle != -1&#41; &#123;
                        sceAudioChRelease&#40;AudioStatus&#91;i&#93;.handle&#41;;
                        AudioStatus&#91;i&#93;.handle = -1;
                &#125;
        &#125;
&#125;

There's surely a more elegant way to fix this bug. :)

EDIT: I see now this commented line:

Code: Select all

sceKernelWaitThreadEnd(AudioStatus[i].threadhandle,NULL);

If this does what I think (wait for the thread to end) it could fix the problem (maybe I'll try later).
Why is commented?
I dind't mention that I found the bug on 3.52M33-u4, with a kernel 3.x eboot.

Ciaooo
Sakya

[NineByNine]

Apologies for resurrection of this ancient thread, but I found the same behaviour recently. It's only likely to cause trouble if you start and stop the pspaudiolib stuff a couple times, but it does cause trouble in this case. Full explanation: the pspAudioEnd() code sets the flag that is supposed to allow the audio thread to terminate gracefully (audio_terminate), but doesn't actually wait for the audio threads to exit, so the calls to sceKernelDeleteThread() fail (silently; the returns aren't checked), and eventually, if you start and stop the lib a few times, the dead threads pile up and calls to create new threads start to fail.

I'm guessing the call to sceKernelWaitThreadEnd(...) was commented out because if someone supplies a buggy callback which would itself hang the audio thread(s), and then called pspAudioEnd() to shutdown, the thread calling pspAudioEnd() would hang in the call. But it's a nuisance that piles up dead threads if you leave the call out.

I note there's also a pspAudioEndPre() defined, presumably for this purpose. Calling this first, then sleeping the calling thread (to allow the audio threads slice time to terminate gracefully), and *then* calling pspAudioEnd() should also work... But note it's not quite guaranteed; you have to count on the audio threads actually getting scheduled, and getting out of the way in time for your call to pspAudioEnd(). The sceKernelWaitThreadEnd() from pspAudioEnd() is much safer from that point of view (tho', of course, a risk with regard to callbacks hanging).

My solution was to create a second call in the lib (pspAudioEnd_patient()) that does wait for the thread exit. Works fine for my app. Just a suggestion.

[J.F.]

Or you could just start the audio lib at the start of your app and stop it at the end... you know, like EVERYONE else does. ;)

[NineByNine]

Or you could just start the audio lib at the start of your app and stop it at the end... you know, like EVERYONE else does. ;)

... Or, y'know, the SDK could contain code that cleans up after itself properly. Like anyone would expect.

Seriously, the reason I do it that way is only because attempts to reset the sample frequency (to use the built in rates the sce calls support) once the lib was up and running failed. So I modified the lib to allow me to set the sample frequency at init; that works; init with the rate you want, shut down, init again for another stream with a different rate; it's all good that way, provided you make sure those threads get put away properly. I do expect to revisit the issue, see what I can do about resetting the frequency without tearing down and restarting the thread; I don't like that either. But still, I did find that issue doing that. So here's yer notice, for what it's worth to you.

[J.F.]

Or you could just start the audio lib at the start of your app and stop it at the end... you know, like EVERYONE else does. ;)

... Or, y'know, the SDK could contain code that cleans up after itself properly. Like anyone would expect.

Seriously, the reason I do it that way is only because attempts to reset the sample frequency (to use the built in rates the sce calls support) once the lib was up and running failed. So I modified the lib to allow me to set the sample frequency at init; that works; init with the rate you want, shut down, init again for another stream with a different rate; it's all good that way, provided you make sure those threads get put away properly. I do expect to revisit the issue, see what I can do about resetting the frequency without tearing down and restarting the thread; I don't like that either. But still, I did find that issue doing that. So here's yer notice, for what it's worth to you.

Or you can use the same method to set the sample rate that sakya uses in LightMP3. Seriously, it sounds like you should be using his code instead of the audiolib anyway. Too many people use the audiolib who don't really need to.

[nemesis]

ok.. this crap took me hours of bugsearching where everything was fine.. thanks to sakya for this hint.. jeez why isn't this commited yet?

[jimparis]

Because you haven't provided a proper patch in the "Patch Submissions" forum.