[Webkit-unassigned] [Bug 33268] make JPEG image dcoder read segmented SharedBuffer

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33268





--- Comment #3 from Peter Kasting <pkasting at google.com>  2010-01-06 14:13:52 PST ---
(From update of attachment 45993)
Other than the style nits the bot pointed out, my comments:

> +unsigned SharedBuffer::extract(char* buffer, unsigned bufferLength, unsigned from)

"from" (and to some degree "buffer") are a little vague.  How about:

unsigned SharedBuffer::copyToContiguousBuffer(char* destBuffer, size_t
bufferLength, size_t readOffset)

> +class SharedBufferDataWalker {

Not totally obvious what this class does, maybe you can write a brief comment
above it?

> +        unsigned lastOffset = -1;
> +        Vector<unsigned char, 8192> buffer;

Is it a potential perf issue that we can decode at most 8K at a time?

> +        SharedBufferDataWalker segment;

"segment" is kind of odd as a name... why not "dataWalker" or something?

> +        while(ture) {

Won't compile (typo)

> +            if (readOffset == lastOffset && m_bufferLength == data.size())
> +                break;
> +
> +            unsigned segmentLength = 0;
> +            if (readOffset == lastOffset) {

Could just move the first conditional here inside the second conditional.

> +                // Need more data to proceed
> +                if (buffer.isEmpty()) {
> +                    segment.set(data, readOffset);
> +                    segmentLength = segment.length();
> +                    buffer.append(segment.data(), segmentLength);
> +                }
> +                segment.set(data, readOffset + buffer.size());
> +                segmentLength = segment.length();
> +                buffer.append(segment.data(), segmentLength);
> +                m_info.src->bytes_in_buffer += segmentLength;
> +                m_info.src->next_input_byte = (JOCTET*)buffer.data();
> +                m_bufferLength = readOffset + buffer.size();
> +            } else {
> +                if (!buffer.isEmpty())
> +                    buffer.clear();
> +
> +                lastOffset = readOffset;
> +
> +                segment.set(data, readOffset);
> +                segmentLength = segment.length();
> +
> +                unsigned totalSize = readOffset + segmentLength;
> +                m_info.src->bytes_in_buffer += totalSize - m_bufferLength;
> +                m_info.src->next_input_byte = (JOCTET*)segment.data();
> +
> +                // If we still have bytes to skip, try to skip those now.
> +                if (m_bytesToSkip)
> +                    skipBytes(m_bytesToSkip);
> +
> +                m_bufferLength = totalSize;
> +            }

I confess, I can't totally follow all this even after peering through it a few
times.  There are too many different buffers and lengths.  Maybe some block
comment somewhere explaining the algorithm, I dunno.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.