HISE icon indicating copy to clipboard operation
HISE copied to clipboard

StreamingSamplerSound::FileReader::readFromDisk and MemoryMappedReader

Open mathieugarcia opened this issue 2 years ago • 0 comments

Hi!

I found a case where StreamingSamplerSound::FileReader::readFromDisk would read blank data as the MemoryMappedReader only contains a section outside the StreamingSamplerSound start/end positions.

Conditions to be met to trigger the bug:

  • Preload is disabled (-1)
  • setDeactivatePoolSearch() is set to true on the SamplePool, to be able to use different sample and loop start/end position with the same sample (otherwise, these sample attributes are shared).

In that case, when updating the sample start/end, some StreamingSamplerSound's will not load anything when filling the internal buffers:

In StreamingSoundSound.cpp, method StreamingSamplerSound::FileReader::readFromDisk(), around line 1244:

if (memoryReader != nullptr && memoryReader->getMappedSection().contains(Range<int64>(readerPosition, readerPosition + numSamples)))

Only part of the sample is mapped into memory, thus, in some case, the readerPosition will not be within the requested read range.

The StreamingSamplerSound::FileReader::readFromDisk method has a bool useMemoryMappedReader parameter, but it is always set to true when updating the in-memory sample in StreamingSamplerSound::setPreloadSize, which is generally called after updating the sample start/end positions (from lengthChanged()).

By enforcing useMemoryMappedReader = false; worked as a very temporary workaround.

So, by simply mapping the requested range prior to reading, in case it's outside the request range, works. Plus, as an extra fallback, if all of the above fails, the normalReader will be used.

Here's the diff:

  if (!isMonolithic() && useMemoryMappedReader && memoryReader != nullptr)
  {
    Range<int64> range { readerPosition, readerPosition + numSamples };
    
    // When the requested range isn't mapped, map it first
    if (!memoryReader->getMappedSection().contains(range))
    {
      ScopedReadLock sl(fileAccessLock);
      
      auto ret = memoryReader->mapSectionOfFile(range);
      jassert(ret && memoryReader->getMappedSection().contains(range));
    }
    
    if (memoryReader->getMappedSection().contains(range))
    {
      ScopedReadLock sl(fileAccessLock);
      
      if (buffer.isFloatingPoint())
        memoryReader->read(buffer.getFloatBufferForFileReader(), startSample, numSamples, readerPosition, true, true);
      else
        jassertfalse;
    }
    else if (normalReader != nullptr && buffer.isFloatingPoint())
    {
      ScopedReadLock sl(fileAccessLock);
      
      // The MemoryMappedReader couldn't be used this time so try with the regular reader
      normalReader->read(buffer.getFloatBufferForFileReader(), startSample, numSamples, readerPosition, true, true);
    }
    else
    {
      jassertfalse;
    }
  }

If this is an acceptable fix then I can submit a pull request.

Thanks! Mathieu.

mathieugarcia avatar Jun 14 '22 08:06 mathieugarcia