[sldev] Bug in LLTextureCacheRemoteWorker::doRead ?

[Philippe Bossut (Merov Linden)]

Hi Carlo,

On Mar 26, 2009, at 8:58 AM, Carlo Wood wrote:
> On Wed, Mar 25, 2009 at 10:31:37PM -0700, Philippe Bossut (Merov  
> Linden) wrote:
>> mOffset is actually the data that's reserved for the header in the
>> formatted image buffer at creation *before* the readFromCache() is
>> invoked (see LLTextureFetchWorker::doWork() in lltexturefetch.cpp).  
>> This
>> quantity is fixed and never changed. What we've read so far is *at  
>> most*
>> TEXTURE_CACHE_ENTRY_SIZE from the header cache.
>
> Before I react to the rest of you mail (thanks!), I really need to
> understand this mOffet better.

Actually, I had a classic "late night email morning after remorse"  
when waking up, thinking that part of my talk had to be wrong or, at  
least, not telling the whole story. Not that I hid anything but rather  
that I don't know the full answer yet and need more time tinkering and  
reading the code.

Clearly, this "mOffset" name is a misnomer and equating it with the  
size of the header (metadata and all I suppose) is not telling the  
whole story... I'll be trying to decipher that today.

> When you say 'header', you seem to refer to something else than
> the header of the image, right? Also jpeg2000 files (and tga files)
> start with a header before the actual pixel data starts.

My understanding is that this "header" contains general image info  
(size, compression scheme, dimension, color model, etc..) and that  
it's valuable to access them fast without opening and decoding the  
whole file. That's the reasoning behind storing this first kilo byte  
of file info into a specific header cache file containing all those  
chunks in one seamless stream.

> So, what is this "header in the formatted image buffer"?
> How is the size of it determined?
> Aren't all files in the cache jpeg 2000 files? Because in that
> case I can't think of a reason for a need of extra data before
> the image(-header).

It seems this header is simply the first kilo byte chunk of data from  
the file and mOffset is the size of the whole metadata chunk in the  
file (could be less or more than 1 kbyte). That's about clear. Also  
the cache does store j2c and tga files but, as far as I can tell, all  
tga files are local files and are loaded by the  
LLTextureCacheLocalFileWorker, not the LLTextureCacheRemoteWorker. The  
code however is written in a general way without making any assumption  
on the file type. It's then possible that the "non-j2c" code path is  
never exercised and could very well be buggy. I'm guessing that our  
investigation might make that clear. (BTW, this is a good reason why  
we should add unit tests to this code, another thing I'm working on  
right now... but that's another story...)

> Right now my picture is this (assuming mOffset < 600):
>
>  TEXTURE_CACHE_ENTRY_SIZE
> <---------------------------><-------------cache file------------>
> <----mOffset----><----JPEG2000 header----><-----pixel data------->
>
> And I have no idea what kind of data goes in the mOffset part
> or how large it is.

Hmmm... You might well be right with that though that would mean that  
LL is adding a different header to j2c files... I'll be investigating  
more this morning and report my findings here.

Cheers,
- Merov