Weirdness when saving partial thumbnails


#1

Hello!

If you’ve been following my adventures, you know that I cache AudioThumbnail for audio files.

I recently turned on code to cache partially completely thumbnails - but this doesn’t seem to work quite right. What appears to happen is that the thumbnail fills up fine, I seem to write it fine, but then when I read it again I get the same partial thumbnail I initially stored instead of more complete thumbnail I saved…

I’d feel it was something I was doing wrong, but I definitely call write on the completed thumbnail and I can see the file update on my drive (though it’s much smaller than the correct thumbnail).

If I change my code to simply only store thumbnails that are complete, I don’t get this.

Ideas?


#2

Can’t see any obvious code-paths that lead to that kind of behaviour… Are you using an AudioThumbnailCache?


#3

Well, I have an AudioThumbnailCache instance because the code requires it, but I don’t use it to store or recall, directly calling thumbnail_.saveTo() and thumbnail_.loadFrom().

Don’t spend any time on this - my testers didn’t like the cached partial thumbnails anyway, they were confused when a partial thumbnail appeared for a large file they’d only partially loaded. The code could be re-enabled in a second, so when I get some time I’ll trace through and figure out what’s going on, it shouldn’t be too hard (the file created is much smaller so clearly a lot just isn’t being written…)


#4

Ok… If you do want me to take a look, maybe send me some example code and I’ll trace it through.


#5

OK, I figured out what the issue is, and the Subject: is somewhat confusing: it should really be “AudioThumbnail::isFullyLoaded() gives incorrect results if the thumbnail is not loaded sequentially”.

Here’s the code:bool AudioThumbnail::isFullyLoaded() const throw() { return numSamplesFinished >= totalSamples - samplesPerThumbSample; }

Here’s what’s going on! In the most common use case, you simply fill the thumbnail from start to finish, save it, and dust your hands off. In my use case, I have large files that might be on a CD and the thumbnail generation is of secondary importance to playing - so if you click in the waveform somewhere, I instantly jump there and start accumulating a thumbnail.

Unfortunately, numSamplesFinished isn’t correctly named if you take my use case into account - it should really be called “lastSampleThatsBeenFilledIn”, but if you aren’t filling in continuously, filling the last sample doesn’t mean they’re all filled.

Now I know the issue I can work around it in my code fairly easily. It’s not so clear how to fix it in the Juce code anyway…


#6

Yes… that whole class makes my head hurt. Does this help?

[code]void AudioThumbnail::setLevels (const MinMaxValue* const* values, int thumbIndex, int numChans, int numValues)
{
const ScopedLock sl (lock);

for (int i = jmin (numChans, channels.size()); --i >= 0;)
    channels.getUnchecked(i)->write (values[i], thumbIndex, numValues);

const int64 start = thumbIndex * (int64) samplesPerThumbSample;
const int64 end = (thumbIndex + numValues) * (int64) samplesPerThumbSample;

if (numSamplesFinished >= start && end > numSamplesFinished)
    numSamplesFinished = end;

totalSamples = jmax (numSamplesFinished, totalSamples);
window->invalidate();
sendChangeMessage();

}
[/code]

…this change should mean that when you write blobs of data that aren’t contiguous, they won’t be counted towards the value of numSamplesFinished.


#7

I’ve rather bypassed that code now - but it’s not clear that that will work because I only fill in each pixel once. So if “when you write blobs of data that aren’t contiguous, they won’t be counted towards the value of numSamplesFinished,” then if I rely on this then I will never end up writing the file…

Since I’m feeding the thumbnail display myself anyway, it was trivial to simply tell my thumbnail to write itself, rather than itself detecting that it needs writing. I suspect most other people who aren’t using Juce’s lovely facilities to automatically fill the thumbnail will do the same as I do…

As for “the class making your head hurt” :smiley: this happens to me all the time in stuff I wrote, where a class gets too large. I have a generic strategy for splitting such monsters up which I believe will work well for this class - it’s to pull off methods into separate functions. In this case, were I you :smiley: (not that you need my advice) I’d pull the structure marking what’s filled out into a separate “dumb” POD data structure that has a few functions you can call on it, and create a single accessor to read the POD data structure from the main class.

You basically have this already with your ThumbData (but that’s currently a class, and private to boot!) I’d make that instead a simple struct, perhaps have another one including
{code]struct ThumbnailDescription {
int32 samplesPerThumbSample;
int64 totalSamples, numSamplesFinished;
int32 numChannels;
double sampleRate;
};[/code] and then pull off the various methods that manipulate these two into pure functions - perhaps in separate files. As an additional bonus, most if not all of those “friend” declarations would vanish, and you’d pass those structs to, say, LevelDataSource…

You can tell I’m trying to avoid starting my week’s work. :smiley: Have a good one!