Feature/byte range streams #127
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ORK IN PROGRESS) This is the first checkin for the implementation of Byte Ranges in Readium SDK. In short, the objective of adding Byte Ranges to Readium SDK is so that there is a way to deal with resources of great size inside an ePUB. For example, you may have an ePUB file with a 1 GB video file in it, and in that case you want to allow the client app to be able to read just chunks of bytes out of that video, and not have to put the entire 1 GB video in memory. This is still a work in progress. There are still code changes that we need to make, plus some more code cleanup that we want to do, before marking this as concluded. Following checkins will take care of that.
…re XXX may for example be woff, as with the Wasteland EPUB3 IDPF sample) Fixed FilterChain bug: reading bytes may reach zero-size, added checks to properly EOF
FIXED obfuscated fonts support for SmokeTestFXL (because the root archive directory is a sibling of META-INF, instead of sub-directory)
…nt filter. unforunately the implementation is incomplete, as RangeFilterContext is never instanciated anywhere, and FilterChainSyncStream::ReadBytes/FilterBytes() with the ByteRange parameter are never used anywhere. also, ByteRangeFilterSyncStream::ReadBytes() with the ByteRange parameter invokes FilterData() with null pointer for source buffer data, so obviously it is not going to return anything :) (as I said, this seems incomplete at this stage...to be continued)
…ealing with byte ranges
This checkin adds the new PassThroughFilter class. This is a very simple class: it just pass along raw bytes without modification. However, despite its simplicity, it can do a number of things for us: - It works as a great sample of how to write a ContentFilter, and also one that deals with byte ranges. - It can be used to debug a stream of bytes, because it can be put in the middle of a filter chain and you can inspect bytes in it. - It is quite easy to modify, therefore it can be used to write logs about, for example, how often a given resource is read. - It can be used to debug the ContentFilter mechanism itself. As I said, feel free to copy and modify this class to do whatever you may need.
Recently, I had to merge the feature/ByteRangeStreams branch with the develop branch. However, I made a small mistake on SourceTree and actually removed this little change from RDPackageResourceConnection.m. So, this checkin just brings it back.
With the changes we recently made on RDPackage, the code now uses SyncContentStreamForItem() (or SyncByteRangeForItem()) in order to get a byte stream for a given resource. The problem, though, is that those methods return a ByteStreamPtr, which was defined by Jim to be a shared_ptr<ByteStream>. This way, in order to be able to store them in the m_byteStreamVector member variable, we need to use a shared_ptr instead of a unique_ptr.
… only, but now we have a "pass through" content filter to play with)
… is requested with byte range support, but its filter does not support it (e.g. obfuscated fonts)
Conflicts: Platform/Apple/RDServices/Main/RDPackage.mm Platform/Apple/RDServices/Main/RDPackageResource.mm
…th pass through content filter (not working, as there is missing buffer-fill code at the moment)
…e filter. commented unused code. to be discussed.
… expand the range to the passed length) iOS: working obfuscated fonts with byte range support (fallback to whole stream, but goes through filter via the correct sync-stream)
…rData (edge case: concrete implementation of Content Filter claims support for ByteRanges but does not have a compatible Context object)
…irective (will later be completely removed, if there is a consensus that this Readium feature is not used in any known application launcher)
…ByteStream[Range])
…ices) This checkin contains three major changes: - First, it creates the new ContentFilter::OperatingMode enum class. Through this enumeration, each subclass of ContentFilter will be able to specify its operating mode, or, more specifically, if it supports byte ranges, if it requires full content, or it if is just a standard ContentFilter. - I also modified how FilterContext objects are created by the ContentFilter class. From now on, the MakeFilterContext() method will check if a ContentFilter class that supports byte ranges is returning a subclass of RangeFilterContext as the result. If not, an exception will be thrown. From now on, subclasses of ContentFilter should override InnerMakeFilterContext() in order to generate a FilterContext object for themselves. - Finally, a piece of work still in progress. We are in the middle of rearchitecting how the current code decides if it should follow the byte range code path or not. There is an initial set of changes around that in this checkin, but this should be majorly reviewed (and modified) in subsequent checkins.
I made a small mistake on my previous checkin - I inverted two function calls. This checkin fixes that.
…, XML well-formed check) see #120 and readium/SDKLauncher-OSX#24
…sted OL elements in navigation document, ReadiumJS handles it fine, ReadiumSDK should as well)
…adding mechanism. the script also decodes the file, and compares the results. this can be used as a basis to test content filter chain + HTTP byte range queries
…er (see contentFilterChainEncode.java for the encoding part). will extract in its own class soon, right now the code is excluded using an #if preprocessor directive. Note the addition of BytesAvailable() for filters: this allows a media-aware deencoder to determine the real decoded size of an asset, rather than using the raw byte length in the HTTP headers (which causes the socket to starve early, as most encoded files are greater in size than their decoded counterpart)
This checkin includes the final chunk of refactoring to support Byte Range streams. Evidently, there may be further checkins as we take feedback on these changes, as well as we fix bugs. But this should be the last piece of redesign in terms of the latest discussions we had around Byte Range streams. To summarize very quickly, this change removes any change previously made in the Cocoa HTTP Server code. It also removes the temporary way that the code had to decide between using Byte Ranges or not. Finally, it also refactors how Byte Streams are built upon for usage by the Cocoa HTTP Server: it first gets the "raw" byte stream, but later switches to the byte streams that can have ContentFilter objects (and that may support byte ranges).
… testing plain old SeekableByteStream (there's a bug in readDataOfLength in RDServices: pending TODO)
After code reviews by Daniel and Shane, I'm checking in this changes. However, notice that this checkin is actually smaller than it looks. One of the things I did in this change is to try to make consistent use of tabs for indentation. In RDService, tabs have been using consistently to make indentations. With the latest checkins made in RDServices, some lines of code had spaces instead for indentations. That was making things look funny in GitHub. So, in this checkin, I tried to switch all the files that I saw with discrepancies to use tabs. Because of that, this checkin is marked as having a lot of changes, but most of the changes are solely from spaces to tabs.
…lter There was an issue when extracting byte ranges when there is no ContentFilter. The problem relied in the fact that, when there is no ContentFilter whatsoever, the byte stream kept by RDPackageResource is a simple ZipFileByteStream. In that case, the RDPackageResource code should just use the SeekableByteStream methods in order to extract a given byte range. The current code is pretty similar to the same fix that Daniel implemented in OS X.
…am instance is determined.
… scenario, yet allow sequential access to ByteStream "ranges", i.e. consecutive chunks)
…red resource sequentially (i.e. 4096 loop buffer), the FilterContext instance carries a re-usable buffer that can be increased if needed. The FilterContext lives for the duration of the HTTP connection that requests a byte range, and therefore survives during the 4096 buffer loop. This bug fix eliminates HTML5 audio playback issues (milliseconds beginning of audio phrase spoken before the actual SMIL clip-begin)
Both Daniel and I have been playing around in the feature/ByteRangeStreams branch, modifying a bunch of things in order to implement the byte ranges. As part of that development effort, we commented out some code, added printfs to do debugging, etc. This checkin cleans up some of that code.
…ncryption, removed corresponding ContentFilter deryption code, removed unnecessary printf() console output. See: https://github.com/readium/readium-sdk/wiki/ContentFilterEncryptedMedia
|
Related OSX Pull Request: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.