AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Allow SkStream::peek() to partially succeed

Browse Source 
If the stream can peek less than requested, peek that amount. Return
the number of bytes peeked.

This simplifies crrev.com/1472123002. For a stream that is smaller than
14 bytes, it can successfully peek, meaning the client will not need to
fall back to read() + rewind(), which may fail if the stream can peek
but not rewind.

This CL revives code from patch set 3 of crrev.com/1044953002, where I
initially introduced peek() (including tests).

Add a test for SkFrontBufferedStream that verifies that peeking does
not make rewind() fail (i.e. by reading past the internal buffer).

BUG=skia:3257

Review URL: https://codereview.chromium.org/1490923005
This commit is contained in:
scroggo 2015-12-07 11:37:13 -08:00 committed by Commit bot
parent 3660d53451
commit d61c384342
4 changed files with 93 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
include/core/SkStream.h
View File

@ -65,17 +65,18 @@ public:
/**
* Attempt to peek at size bytes.
* If this stream supports peeking, and it can peek size bytes, copy size
* bytes into buffer, and return true.
* If the stream does not support peeking, or cannot peek size bytes,
* return false and leave buffer unchanged.
* If this stream supports peeking, copy min(size, peekable bytes) into
* buffer, and return the number of bytes copied.
* If the stream does not support peeking, or cannot peek any bytes,
* return 0 and leave buffer unchanged.
* The stream is guaranteed to be in the same visible state after this
* call, regardless of success or failure.
* @param buffer Must not be NULL. Destination to copy bytes.
* @param buffer Must not be NULL, and must be at least size bytes. Destination
* to copy bytes.
* @param size Number of bytes to copy.
* @return Whether the peek was performed.
* @return The number of bytes peeked/copied.
*/
virtual bool peek(void* /* buffer */, size_t /* size */) const { return false; }
virtual size_t peek(void* /*buffer*/, size_t /*size*/) const { return 0; }
/** Returns true when all the bytes in the stream have been read.
* This may return true early (when there are no more bytes to be read)
@ -325,7 +326,7 @@ public:
size_t read(void* buffer, size_t size) override;
bool isAtEnd() const override;
bool peek(void* buffer, size_t size) const override;
size_t peek(void* buffer, size_t size) const override;
bool rewind() override;
SkMemoryStream* duplicate() const override;

33
src/core/SkStream.cpp
View File

@ -367,18 +367,14 @@ size_t SkMemoryStream::read(void* buffer, size_t size) {
return size;
}
bool SkMemoryStream::peek(void* buffer, size_t size) const {
size_t SkMemoryStream::peek(void* buffer, size_t size) const {
SkASSERT(buffer != nullptr);
const size_t position = fOffset;
if (size > fData->size() - position) {
// The stream is not large enough to satisfy this request.
return false;
}
const size_t currentOffset = fOffset;
SkMemoryStream* nonConstThis = const_cast<SkMemoryStream*>(this);
SkDEBUGCODE(const size_t bytesRead =) nonConstThis->read(buffer, size);
SkASSERT(bytesRead == size);
nonConstThis->fOffset = position;
return true;
const size_t bytesRead = nonConstThis->read(buffer, size);
nonConstThis->fOffset = currentOffset;
return bytesRead;
}
bool SkMemoryStream::isAtEnd() const {
@ -725,25 +721,26 @@ public:
return fOffset == fSize;
}
bool peek(void* buff, size_t size) const override {
size_t peek(void* buff, size_t bytesToPeek) const override {
SkASSERT(buff != nullptr);
if (fOffset + size > fSize) {
return false;
}
bytesToPeek = SkTMin(bytesToPeek, fSize - fOffset);
size_t bytesLeftToPeek = bytesToPeek;
char* buffer = static_cast<char*>(buff);
const SkDynamicMemoryWStream::Block* current = fCurrent;
size_t currentOffset = fCurrentOffset;
while (size) {
while (bytesLeftToPeek) {
SkASSERT(current);
size_t bytesFromCurrent =
SkTMin(current->written() - currentOffset, size);
SkTMin(current->written() - currentOffset, bytesLeftToPeek);
memcpy(buffer, current->start() + currentOffset, bytesFromCurrent);
size -= bytesFromCurrent;
bytesLeftToPeek -= bytesFromCurrent;
buffer += bytesFromCurrent;
current = current->fNext;
currentOffset = 0;
}
return true;
return bytesToPeek;
}
bool rewind() override {

18
src/utils/SkFrontBufferedStream.cpp
View File

@ -16,7 +16,7 @@ public:
size_t read(void* buffer, size_t size) override;
bool peek(void* buffer, size_t size) const override;
size_t peek(void* buffer, size_t size) const override;
bool isAtEnd() const override;
@ -157,18 +157,20 @@ size_t FrontBufferedStream::readDirectlyFromStream(char* dst, size_t size) {
return bytesReadDirectly;
}
bool FrontBufferedStream::peek(void* dst, size_t size) const {
size_t FrontBufferedStream::peek(void* dst, size_t size) const {
// Keep track of the offset so we can return to it.
const size_t start = fOffset;
if (start + size > fBufferSize) {
// This stream is not able to buffer enough.
return false;
if (start >= fBufferSize) {
// This stream is not able to buffer.
return 0;
}
size = SkTMin(size, fBufferSize - start);
FrontBufferedStream* nonConstThis = const_cast<FrontBufferedStream*>(this);
SkDEBUGCODE(const size_t bytesRead =) nonConstThis->read(dst, size);
SkASSERT(bytesRead == size);
const size_t bytesRead = nonConstThis->read(dst, size);
nonConstThis->fOffset = start;
return true;
return bytesRead;
}
size_t FrontBufferedStream::read(void* voidDst, size_t size) {

78
tests/StreamTest.cpp
View File

@ -197,9 +197,9 @@ DEF_TEST(Stream, reporter) {
/**
* Tests peeking and then reading the same amount. The two should provide the
* same results.
* Returns whether the stream could peek.
* Returns the amount successfully read minus the amount successfully peeked.
*/
static bool compare_peek_to_read(skiatest::Reporter* reporter,
static size_t compare_peek_to_read(skiatest::Reporter* reporter,
SkStream* stream, size_t bytesToPeek) {
// The rest of our tests won't be very interesting if bytesToPeek is zero.
REPORTER_ASSERT(reporter, bytesToPeek > 0);
@ -208,9 +208,7 @@ static bool compare_peek_to_read(skiatest::Reporter* reporter,
void* peekPtr = peekStorage.get();
void* readPtr = peekStorage.get();
if (!stream->peek(peekPtr, bytesToPeek)) {
return false;
}
const size_t bytesPeeked = stream->peek(peekPtr, bytesToPeek);
const size_t bytesRead = stream->read(readPtr, bytesToPeek);
// bytesRead should only be less than attempted if the stream is at the
@ -219,21 +217,17 @@ static bool compare_peek_to_read(skiatest::Reporter* reporter,
// peek and read should behave the same, except peek returned to the
// original position, so they read the same data.
REPORTER_ASSERT(reporter, !memcmp(peekPtr, readPtr, bytesRead));
REPORTER_ASSERT(reporter, !memcmp(peekPtr, readPtr, bytesPeeked));
return true;
// A stream should never be able to peek more than it can read.
REPORTER_ASSERT(reporter, bytesRead >= bytesPeeked);
return bytesRead - bytesPeeked;
}
static void test_peeking_stream(skiatest::Reporter* r, SkStream* stream, size_t limit) {
size_t peeked = 0;
static void test_fully_peekable_stream(skiatest::Reporter* r, SkStream* stream, size_t limit) {
for (size_t i = 1; !stream->isAtEnd(); i++) {
const bool couldPeek = compare_peek_to_read(r, stream, i);
if (!couldPeek) {
REPORTER_ASSERT(r, peeked + i > limit);
// No more peeking is supported.
break;
}
peeked += i;
REPORTER_ASSERT(r, compare_peek_to_read(r, stream, i) == 0);
}
}
@ -244,7 +238,51 @@ static void test_peeking_front_buffered_stream(skiatest::Reporter* r,
REPORTER_ASSERT(r, dupe != nullptr);
SkAutoTDelete<SkStream> bufferedStream(SkFrontBufferedStream::Create(dupe, bufferSize));
REPORTER_ASSERT(r, bufferedStream != nullptr);
test_peeking_stream(r, bufferedStream, bufferSize);
size_t peeked = 0;
for (size_t i = 1; !bufferedStream->isAtEnd(); i++) {
const size_t unpeekableBytes = compare_peek_to_read(r, bufferedStream, i);
if (unpeekableBytes > 0) {
// This could not have returned a number greater than i.
REPORTER_ASSERT(r, unpeekableBytes <= i);
// We have reached the end of the buffer. Verify that it was at least
// bufferSize.
REPORTER_ASSERT(r, peeked + i - unpeekableBytes >= bufferSize);
// No more peeking is supported.
break;
}
peeked += i;
}
// Test that attempting to peek beyond the length of the buffer does not prevent rewinding.
bufferedStream.reset(SkFrontBufferedStream::Create(original.duplicate(), bufferSize));
REPORTER_ASSERT(r, bufferedStream != nullptr);
const size_t bytesToPeek = bufferSize + 1;
SkAutoMalloc peekStorage(bytesToPeek);
SkAutoMalloc readStorage(bytesToPeek);
for (size_t start = 0; start <= bufferSize; start++) {
// Skip to the starting point
REPORTER_ASSERT(r, bufferedStream->skip(start) == start);
REPORTER_ASSERT(r, bufferedStream->getPosition() == start);
const size_t bytesPeeked = bufferedStream->peek(peekStorage.get(), bytesToPeek);
if (0 == bytesPeeked) {
// Peeking should only fail completely if we have read beyond the buffer.
REPORTER_ASSERT(r, bufferedStream->getPosition() >= bufferSize);
break;
}
// Only read the amount that was successfully peeked.
const size_t bytesRead = bufferedStream->read(readStorage.get(), bytesPeeked);
REPORTER_ASSERT(r, bytesRead == bytesPeeked);
REPORTER_ASSERT(r, !memcmp(peekStorage.get(), readStorage.get(), bytesPeeked));
// This should be safe to rewind.
REPORTER_ASSERT(r, bufferedStream->rewind());
}
}
// This test uses file system operations that don't work out of the
@ -255,7 +293,7 @@ DEF_TEST(StreamPeek, reporter) {
// Test a memory stream.
const char gAbcs[] = "abcdefghijklmnopqrstuvwxyz";
SkMemoryStream memStream(gAbcs, strlen(gAbcs), false);
test_peeking_stream(reporter, &memStream, memStream.getLength());
test_fully_peekable_stream(reporter, &memStream, memStream.getLength());
// Test an arbitrary file stream. file streams do not support peeking.
SkFILEStream fileStream(GetResourcePath("baby_tux.webp").c_str());
@ -265,7 +303,7 @@ DEF_TEST(StreamPeek, reporter) {
}
SkAutoMalloc storage(fileStream.getLength());
for (size_t i = 1; i < fileStream.getLength(); i++) {
REPORTER_ASSERT(reporter, !fileStream.peek(storage.get(), i));
REPORTER_ASSERT(reporter, fileStream.peek(storage.get(), i) == 0);
}
// Now test some FrontBufferedStreams
@ -293,7 +331,7 @@ static void stream_peek_test(skiatest::Reporter* rep,
SkASSERT(size >= 1);
SkASSERT(size <= sizeof(buffer));
SkASSERT(size + i <= asset->getLength());
if (!asset->peek(buffer, size)) {
if (asset->peek(buffer, size) < size) {
ERRORF(rep, "Peek Failed!");
return;
}