SoundEngine loop stream sound

[CreativeMD]

Yes!!! Thank you sooo much. It seems to be fixed, everything works just fine: https://youtu.be/Z7GmncqhGpI

Now there is only this random crash, but i don't know if there is a way to fix it.

Again thank you so much for all your spent time and effort! You really helped me, i have been trying to solve this issue for weeks. Now i can finally release! I wish you all the best for your future projects. Thank you.

[CreativeMD]

Hey there, I'm sorry to bother you again, but unfortunately this random crash (which i mentioned earlier) has become quiet annoying.

Just as a reminder I'm talking about this one:

Code: [Select]

[17:36:04] [Thread-11/INFO]: [STDERR]: java.lang.NullPointerException
[17:36:04] [Thread-11/INFO]: [STDERR]: at com.jcraft.jogg.StreamState.packetout(StreamState.java:186)
[17:36:04] [Thread-11/INFO]: [STDERR]: at paulscode.sound.codecs.CodecJOrbis.readBytes(CodecJOrbis.java:575)
[17:36:04] [Thread-11/INFO]: [STDERR]: at paulscode.sound.codecs.CodecJOrbis.read(CodecJOrbis.java:354)
[17:36:04] [Thread-11/INFO]: [STDERR]: at paulscode.sound.Source.stream(Source.java:953)
[17:36:04] [Thread-11/INFO]: [STDERR]: at paulscode.sound.StreamThread.run(StreamThread.java:129)
It causes the sound engine to crash from time to time, so all sounds will be stopped. The sound engine has to be restarted in order to make it working again.

Other mod authors are aware of this issue too, but none of us knows how to fix it. My guess is, that this is a weird thread issue, which is rather unlikely to occur, but will eventually happen if you play the game long enough. Of course the more streaming sounds will be played the more likely it is to happen.

Maybe you can help us out here? Do you have an idea what could cause this crash and how to fix it?

If not, I'm searching for a proper way to catch the exception and try to end the stream properly without causing the engine to crash as well, but unfortunately it does not work out, at least as far as i can tell, since it's rather difficult to reproduce this crash:

Code: [Select]

private byte[] readBytes()
    {
        if( !initialized( GET, XXX ) )
            return null;

        if( endOfStream( GET, XXX ) )
            return null;

        if( convertedBuffer == null )
            convertedBuffer = new byte[ convertedBufferSize ];
        byte[] returnBuffer = null;

        float[][] pcmf;
        int samples, bout, ptr, mono, val, i, j;

        switch( joggSyncState.pageout( joggPage ) )
        {
            case( 0 ):
            case( -1 ):
                break;
            default:
            {
                joggStreamState.pagein( joggPage );
                if( joggPage.granulepos() == 0 )
                {
                    endOfStream( SET, true );
                    return null;
                }
               
                try{
                processPackets: while( true )
                {
                    switch( joggStreamState.packetout( joggPacket ) )
                    {
                        case( 0 ):
                            break processPackets;
                        case( -1 ):
                            break;
                        default:
                        {
                            if( jorbisBlock.synthesis( joggPacket ) == 0 )
                                jorbisDspState.synthesis_blockin( jorbisBlock );

                            while( ( samples=jorbisDspState.synthesis_pcmout(
                                                     pcmInfo, pcmIndex ) ) > 0 )
                            {
                                pcmf = pcmInfo[0];
                                bout = ( samples < convertedBufferSize ?
                                                samples : convertedBufferSize );
                                for( i = 0; i < jorbisInfo.channels; i++ )
                                {
                                    ptr = i * 2;
                                    mono = pcmIndex[i];
                                    for( j = 0; j < bout; j++ )
                                    {
                                        val = (int) ( pcmf[i][mono + j] *
                                                                       32767. );
                                        if( val > 32767 )
                                            val = 32767;
                                        if( val < -32768 )
                                            val = -32768;
                                        if( val < 0 )
                                            val = val | 0x8000;
                                        convertedBuffer[ptr] = (byte) (val);
                                        convertedBuffer[ptr+1] =
                                                               (byte) (val>>>8);
                                        ptr += 2 * (jorbisInfo.channels);
                                    }
                                }
                                jorbisDspState.synthesis_read( bout );

                                returnBuffer = appendByteArrays( returnBuffer,
                                               convertedBuffer,
                                               2 * jorbisInfo.channels * bout );
                            }
                        }
                    }
                }
               
                }catch(Exception e){
                e.printStackTrace();
                endOfStream( SET, true );
                return null;
                }

                if( joggPage.eos() != 0 )
                    endOfStream( SET, true );
            }
        }
Is "endOfStream( SET, true );" a proper way of stopping the sound or will it cause future issues?

[Paul]

Do you know what specifically is null when that exception is thrown?  If lucky, a null check might fix the problem.  Besides a thread issue, possibly other causes might be an empty buffer or index out of bounds problem (i.e. some corner case in the read process).  Maybe try updating JOrbis if there is a more recent version.

If you can't solve it, setting end of stream should be fine I think.

[CreativeMD]

The newest version of JOrbis is 0.0.17 and from 2008, i'm pretty sure you are already using the newest version.

The crash itself takes place here:

Code: [Select]

public int packetout(Packet op){

    /* The last part of decode. We have the stream broken into packet
       segments.  Now we need to group them into packets (or return the
       out of sync markers) */

    int ptr=lacing_returned;

    if(lacing_packet<=ptr){
      return (0);
    }

    if((lacing_vals[ptr]&0x400)!=0){ //<- this line is causing the exception
      /* We lost sync here; let the app know */
      lacing_returned++;

      /* we need to tell the codec there's a gap; it might need to
         handle previous packet dependencies. */
      packetno++;
      return (-1);
    }

    /* Gather the whole packet. We'll have no holes or a partial packet */
    {
      int size=lacing_vals[ptr]&0xff;
      int bytes=0;

      op.packet_base=body_data;
      op.packet=body_returned;
      op.e_o_s=lacing_vals[ptr]&0x200; /* last packet of the stream? */
      op.b_o_s=lacing_vals[ptr]&0x100; /* first packet of the stream? */
      bytes+=size;

      while(size==255){
        int val=lacing_vals[++ptr];
        size=val&0xff;
        if((val&0x200)!=0)
          op.e_o_s=0x200;
        bytes+=size;
      }

      op.packetno=packetno;
      op.granulepos=granule_vals[ptr];
      op.bytes=bytes;

      body_returned+=bytes;

      lacing_returned=ptr+1;
    }
    packetno++;
    return (1);
  }
I think catching the exception is the best way to do it right know. Future tests will tell if that fixes the crash of the sound engine or not.

As i said earlier, my guess it's a threading issue. The codec will cleanup everything, but still tries to continue to stream. (maybe once he runs out of streaming channels).

Thanks again for your help, if you ever run across a solution for this problem just let me know . I really appreciate your work and your spent time. Have a good day.

EDIT
Am I allowed to upload a modified version of your CodecJOrbis on github (open source)? I would also mark all changes i made.

[Paul]

No problem with uploading modified code.. the license is basically a "do whatever you want" license (only restriction really is that if you happen to give me attribution, be sure to mention the website here).

[CreativeMD]

Finally someone could send me the complete log. Catching the exception is not enough unfortunately, since the sound engine seems to crash nonetheless:

Code: [Select]

[12:35:48] [Thread-23/INFO] [STDERR/]: [com.creativemd.ambientsounds.CodecJOrbisFixed:readBytes:626]: java.lang.NullPointerException
[12:35:48] [Thread-23/INFO] [STDERR/]: [com.creativemd.ambientsounds.CodecJOrbisFixed:readBytes:626]: at com.creativemd.ambientsounds.CodecJOrbisFixed.readBytes(CodecJOrbisFixed.java:587)
[12:35:48] [Thread-23/INFO] [STDERR/]: [com.creativemd.ambientsounds.CodecJOrbisFixed:readBytes:626]: at com.creativemd.ambientsounds.CodecJOrbisFixed.read(CodecJOrbisFixed.java:356)
[12:35:48] [Thread-23/INFO] [STDERR/]: [com.creativemd.ambientsounds.CodecJOrbisFixed:readBytes:626]: at paulscode.sound.Source.stream(Source.java:953)
[12:35:48] [Thread-23/INFO] [STDERR/]: [com.creativemd.ambientsounds.CodecJOrbisFixed:readBytes:626]: at paulscode.sound.StreamThread.run(StreamThread.java:129)
[12:35:48] [Thread-23/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: java.lang.NullPointerException
[12:35:48] [Thread-23/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: at paulscode.sound.Source.stream(Source.java:962)
[12:35:48] [Thread-23/INFO] [STDERR/]: [java.lang.ThreadGroup:uncaughtException:1052]: at paulscode.sound.StreamThread.run(StreamThread.java:129)
This is the code causing the second exception:

Code: [Select]

buffer = codec.read();
if( buffer != null )
{
   if( buffer.audioData != null )
       channel.queueBuffer( buffer.audioData );
   buffer.cleanup();
    buffer = null;
   return true;
}
else if( codec.endOfStream() ) //<-- codec seems to be null

It looks like the method cleanup() is called while the sound is still streaming. It is the only method which sets the "codec" to null.

With that knowledge in mind i found some fishy looking code in my mod:

Code: [Select]

        if(!system.playing(sound.systemName))
        {
        if(sound.hasBeenAdded)
        {
            if(sound.repeat && AmbientSounds.debugging)
            {
            System.out.println("Unexpected ending sound " + sound.getSoundLocation() + " " + sound.systemName);
            }
            sound.playing = false;
            if(library != null)
            library.removeSource(sound.systemName); //<-- not synchronized
            else
            System.out.println("No library found. Something went wrong!");
                iterator.remove();
        }
            }else{
            sound.hasBeenAdded = true;
            }

I thought it would be necessary to remove the source after is has been played. This call is not synchronized so potentially it could be the cause of the issue, but i doubt it since it only remove a source which is not playing anymore and the issue is also there if my mod is not installed. I will do some testing nonetheless ...

Maybe you have another idea? It looks like something is cleaning up the source from another thread.

[Paul]

Good catch.  I'll play around with it to see if I can find the synchronization issue.  I have a feeling it is a race condition between the streaming thread and command queue thread.

[CreativeMD]

Any news so far?

[CreativeMD]

Ok, i have taken a look at it myself and as far as i can tell there is no synchronization for that. So here is my suggestion for implementing it:

In Source.java

Code: [Select]

public boolean removed = false;

In Library.java

Code: [Select]

    public void removeSource( String sourcename )
    {
        Source mySource = sourceMap.get( sourcename );
        if( mySource != null )
        {
            if(mySource.toStream )
                mySource.removed = true;
            else
                mySource.cleanup(); // end the source, free memory
        }
        sourceMap.remove( sourcename );
    }
In StreamThread.java - run() :

Code: [Select]

                        [...]
                        if( src == null )
                        {
                            iter.remove();
                        }
                        else if( src.removed )
                        {
                            src.cleanup();
                            iter.remove();
                        }
                        else if( src.stopped() )
                        {
                            if( !src.rawDataStream )
                                iter.remove();
                        }
                        [...]


[Paul]

I think a better way to handle this would be for the stream thread to place an entry into the command queue, so that it is synchronized with other commands.  Sorry, I've been busy with projects around the house and haven't had a lot of time for programming.  I'll try and make some time to work on this.

[CreativeMD]

Hey there,

it's been a while. In the meantime i have decided to try to solve this issue on my own. Forge (the tool to mod in minecraft) allows a mod to change class files while the game is loading. So i implemented the changes i showed you above myself.

If you want to you can take a look at the actual transforming code here:
https://github.com/CreativeMD/AmbientSounds/blob/1.11/src/main/java/com/creativemd/ambientsounds/soundfix/SoundFixTransformer.java
https://github.com/CreativeMD/AmbientSounds/blob/1.11/src/main/java/com/creativemd/ambientsounds/soundfix/SoundFixMethods.java

So yes, basically this solves the issue. It never occurred again (at least no so far). Again thanks for all your help and support. I hope you will eventually find some time to update the engine and to fix this issue properly.

[MadMartian]

I can also confirm a synchronization issue.  A previous call to removeSource is wiping the codec field while the StreamThread is busy preloading it.  I managed to catch this happening with the help of logging.

Since I am also Minecraft modding, I think I will take-up CreativeMD's fix, it's essentially what I had in mind (and dreaded the thought of having to resort to).

[CreativeMD]

I can also confirm a synchronization issue.  A previous call to removeSource is wiping the codec field while the StreamThread is busy preloading it.  I managed to catch this happening with the help of logging.

Since I am also Minecraft modding, I think I will take-up CreativeMD's fix, it's essentially what I had in mind (and dreaded the thought of having to resort to).

I will try to merge it into forge, so it's fixed for everybody.

[CreativeMD]

The pr for forge: https://github.com/MinecraftForge/MinecraftForge/pull/4765

[CreativeMD]

I think a better way to handle this would be for the stream thread to place an entry into the command queue, so that it is synchronized with other commands.  Sorry, I've been busy with projects around the house and haven't had a lot of time for programming.  I'll try and make some time to work on this.

What exactly do you mean? Should the StreamThread add a commend queue to cleanup the source or are you suggesting a completely different way?