OutOfMemoryError when writing OGGs

[IJabz]

See https://java.net/jira/browse/JAUDIOTAGGER-449

Looks like OggVorbisTagWriter can OOME when writing an OGG. Here's the stack trace:

at java.lang.OutOfMemoryError.<init>()V (OutOfMemoryError.java:25) at java.nio.HeapByteBuffer.<init>(II)V (HeapByteBuffer.java:39) at java.nio.ByteBuffer.allocate(I)Ljava/nio/ByteBuffer; (ByteBuffer.java:312) at org.jaudiotagger.audio.ogg.OggVorbisTagWriter.writeRemainingPages(ILjava/io/RandomAccessFile;Ljava/io/RandomAccessFile;)V (OggVorbisTagWriter.java:451) at org.jaudiotagger.audio.ogg.OggVorbisTagWriter.replaceSecondPageAndRenumberPageSeqs(Lorg/jaudiotagger/audio/ogg/OggVorbisTagReader$OggVorbisHeaderSizes;IILorg/jaudiotagger/audio/ogg/util/OggPageHeader;Ljava/nio/ByteBuffer;Ljava/io/RandomAccessFile;Ljava/io/RandomAccessFile;)V (OggVorbisTagWriter.java:275) at org.jaudiotagger.audio.ogg.OggVorbisTagWriter.write(Lorg/jaudiotagger/tag/Tag;Ljava/io/RandomAccessFile;Ljava/io/RandomAccessFile;)V (OggVorbisTagWriter.java:132) at org.jaudiotagger.audio.ogg.OggFileWriter.writeTag(Lorg/jaudiotagger/tag/Tag;Ljava/io/RandomAccessFile;Ljava/io/RandomAccessFile;)V (OggFileWriter.java:44) at org.jaudiotagger.audio.generic.AudioFileWriter.write(Lorg/jaudiotagger/audio/AudioFile;)V (AudioFileWriter.java:429) at org.jaudiotagger.audio.AudioFileIO.writeFile(Lorg/jaudiotagger/audio/AudioFile;)V (AudioFileIO.java:334) at org.jaudiotagger.audio.AudioFileIO.write(Lorg/jaudiotagger/audio/AudioFile;)V (AudioFileIO.java:164) at org.jaudiotagger.audio.AudioFile.commit()V (AudioFile.java:115)

In a heap dump I have (available on request) it appears that writeRemainingPages is allocating two large byte buffers. They are initialised to be the length from the final comment page until the end of the file. For large enough OGGs I think this will cause problems:

//TODO there is a risk we wont have enough memory to create these buffers ByteBuffer bb = ByteBuffer.allocate((int)(raf.length() - raf.getFilePointer())); ByteBuffer bbTemp = ByteBuffer.allocate((int)(raf.length() - raf.getFilePointer()));

Note the TODO - that's not my edit! So I suspect this was always known about, but in the absence of a bug I thought I'd file one.

Could the remainder of the file be written chunked up, maybe in VorbisCommentReader.JAUDIOTAGGER_MAX_COMMENT_LENGTH size chunks?

Patch:

Index: src/org/jaudiotagger/audio/ogg/OggVorbisTagWriter.java

--- src/org/jaudiotagger/audio/ogg/OggVorbisTagWriter.java (revision 1059) +++ src/org/jaudiotagger/audio/ogg/OggVorbisTagWriter.java (working copy) @@ -31,6 +31,7 @@ import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.nio.channels.FileChannel; import java.util.List; import java.util.logging.Logger;

@@ -446,13 +447,10 @@ long startAudio = raf.getFilePointer(); long startAudioWritten = rafTemp.getFilePointer();

• //TODO there is a risk we wont have enough memory to create these buffers
• ByteBuffer bb = ByteBuffer.allocate((int)(raf.length() - raf.getFilePointer()));
• ByteBuffer bbTemp = ByteBuffer.allocate((int)(raf.length() - raf.getFilePointer()));
• ByteBuffer bb = raf.getChannel().map(FileChannel.MapMode.READ_WRITE, raf.getFilePointer(), (int)(raf.length() - raf.getFilePointer()));
• // Position the pointer in the correct place in the temp file, but allocate the same space as the original file

• ByteBuffer bbTemp = rafTemp.getChannel().map(FileChannel.MapMode.READ_WRITE, rafTemp.getFilePointer(), (int)(raf.length() - raf.getFilePointer()));

• //Read in the rest of the data into bytebuffer and rewind it to start

• raf.getChannel().read(bb);
• bb.rewind(); while(bb.hasRemaining()) { OggPageHeader nextPage = OggPageHeader.read(bb); @@ -472,9 +470,6 @@ nextPageHeaderBuffer.rewind(); bbTemp.put(nextPageHeaderBuffer); }
• //Now just write as a single IO operation
• bbTemp.rewind();
• rafTemp.getChannel().write(bbTemp); //Check we have written all the data //TODO could we do any other checks to check data written correctly ? if ((raf.length() - startAudio) != (rafTemp.length() - startAudioWritten))