-
Notifications
You must be signed in to change notification settings - Fork 164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resource streams improved: Filter chain hooks, byte range support #146
Conversation
This checkin contains two small changes: - The first is simply to add to Android.mk the two new files that we created as part of our refactoring of the code for filter chains in Readium SDK. Two new files were created and now they are part of the Android build as well. - Second, in order to let Android compile, GCC does not recognize the "ios::seekdir" enumeration the same way that clang does. Therefore, we have to use "std::ios::beg" instead of "std::ios::seeker::beg" for GCC. I already checked that clang compiles with this change too.
This change tackles the fact that the Launchers need to call Package::GetFilterChainByteStream() in order to get a byte stream that has ContentFilter objects hooked up to it. Without that call, you can get a byte stream, but it does not have any ContentFilter and therefore things like obfuscated fonts don't work. Notice, however, that this change does not allow Android to make use of byte ranges yet.
…te stream with it.
To ease debugging with conflicting file names between that and SDK's package.cpp
This checkin includes some changes that will allow Readium SDK to support byte ranges in Android. In a nutshell, these changes add another native method to the set of native methods associated with the Package class. This new method will receive a range, and it will instantiate a FilterChainByteStreamRange object based upon the request. Then, it will use that FilterChainByteStreamRange to extract a range of bytes from a given resource, and it will return those bytes inside a ByteBuffer. Later, the EpubServer class will put that ByteBuffer inside a ByteArrayInputStream, and it will pass that to the Nano HTTP Server, in order to fulfill a request.
…e a non filter chain capable (raw) input stream. Buffer size can now be specified from above.
…tream (filter chain based ones for now). Add range byte reading support for both stream types.
Conflicts: ePub3/utilities/byte_buffer.cpp
|
||
private static final String TAG = "PackageResource"; | ||
|
||
private static final int BUFFER_SIZE = 4096; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to highlight a fundamental difference between the Android and iOS / OSX launchers (at this stage of ), which we may need to take into consideration when looking into LCP or other DRM encryption schemes.
Right now, the NanoHTTP server pulls binary data from the response's input stream into 16kB buffers:
https://github.com/readium/SDKLauncher-Android/blob/develop/SDKLauncher-Android/src/fi/iki/elonen/NanoHTTPD.java#L622
int BUFFER_SIZE = 16 * 1024;
byte[] buff = new byte[BUFFER_SIZE];
while (pending > 0) {
int read = data.read(buff, 0, ((pending > BUFFER_SIZE) ? BUFFER_SIZE : pending));
if (read <= 0) {
break;
}
outputStream.write(buff, 0, read);
pending -= read;
}
In iOS and OSX, CocoaHTTPServer also feeds HTTP response bytes into its underlying socket (using 256kB or 512kB buffers, depending on the platform), however this is directly correlated to the size of data chunks that incrementally pass through a non-cached Content Filter (filterData() method). For example, an encrypted image file is typically not requested by WebView via HTTP byte-range (just a regular HTTP request), but the actual byte packets would still be fragmented into distinct consecutive chunks (thus why a decryption algorithm must track its state (previous blocks, etc.)).
I see that in the current Android implementation, this correlation is non-existent, because the input stream is initialised with an intermediary pre-fetched byte buffer that represents the full resource:
https://github.com/readium/SDKLauncher-Android/blob/feature/obfuscatedFonts/SDKLauncher-Android/src/org/readium/sdk/android/launcher/util/EpubServer.java#L182
byte[] data = packageResource.readDataFull();
res = new Response(Response.Status.OK, mime, new ByteArrayInputStream(data));
Note that even an HTTP byte-range request fetches the full queried data before passing it into a stream:
https://github.com/readium/SDKLauncher-Android/blob/feature/obfuscatedFonts/SDKLauncher-Android/src/org/readium/sdk/android/launcher/util/EpubServer.java#L177
byte[] data = packageResource.readDataOfLength((int) newLen, (int) startFrom);
res = new Response(Response.Status.PARTIAL_CONTENT, mime, new ByteArrayInputStream(data));
Behind the scenes, the ResourceStream
is indeed initialised with BUFFER_SIZE
, which informs the rate of transfer from the underlying Readium SDK byte stream (which could potentially be bound to a Content Filter filterData()):
https://github.com/readium/readium-sdk/blob/feature/AndroidByteRangeStream/Platform/Android/jni/resource_stream.cpp#L133
std::size_t bytesRead = byteStream->ReadBytes(&tmpBuffer, readLength);
(see readLength
= stream-> getBufferSize()
)
To conclude, this design is totally fine for now. In a future iteration, we might want (or not) to consider an alternative to the aforementioned ByteArrayInputStream
, for example a "real" stream that would pull data incrementally from the underlying Readium SDK byte stream, thereby avoiding the intermediary byte[]
full/range buffer prefetching, and correlating more directly with the 16kB buffer that the HTTP server uses. At any rate, I feel that the 4kB buffer is a bit small, and may be suboptimal with some encryption schemes.
…by my IDE and it looks legit...so here it is.
…ed the Android SDK, via the Manager ... not sure if it is related)
…lls to other functions during HTTP resource fetching
…sing delete[] operator on JNI temporary heap buffers, added log messages to trace HTTP byte requests
…+ types are correctly declared (none of this is strictly necessary, as unsigned int 8 is unsigned chat is jbyte)
…instead of creating one from inside the JNI space (pulls binary data straight from C++ and fills the Java buffer directly) removed inefficient synchronized keyword on ByteStream wrapper, as the critical sections granularity is defined at the application level
Resource streams improved: Filter chain hooks, byte range support
This allows for support of:
This fixes the following issues:
#132 - Reading filter chain aware byte streams should be more clear now (PackageResource helper class)
#103 - This is fixed for non filter chain byte streams (now called raw input streams)
Potentially fixes readium/SDKLauncher-Android#22