Skip to content
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

ContentFilter, non-ByteRange case, no need for BytesAvailable() override, use NeedsCache capability (prefetch media resource, whole byte buffer.length) #163

Closed
danielweck opened this issue Mar 9, 2015 · 5 comments
Assignees

Comments

@danielweck
Copy link
Member

NeedsCache (must be set to truye always, because HTTP byte-range case is never encountered when filter_chain_byte_stream is instantiated):
https://github.com/readium/readium-sdk/blob/develop/ePub3/ePub/filter_chain_byte_stream.cpp#L61

BytesAvailable():
https://github.com/readium/readium-sdk/blob/develop/ePub3/ePub/filter_chain_byte_stream.h#L66

https://docs.google.com/document/d/11NZEzDQhNesEfN6jXZTAihnC7XgFIrCTOT5TVkL0-P4/edit

@nleme
Copy link
Contributor

nleme commented Mar 12, 2015

I wrote a document that describes the whole issue, plus the proposed fix. In addition, it also contains the scenarios where this would come into play, and how the proposed fix would work with it. It is saved as a Google Doc:

https://docs.google.com/document/d/1rB3LO0-aUuFxTbxSrc44gFzvL7zMh3-LFYpUnFskPAU/edit#

@danielweck
Copy link
Member Author

Pretty thorough @nleme (as always!) Many thanks!
I believe you have already started implementation, so I hope you don't mind if I assign you to this issue.

@danielweck
Copy link
Member Author

@nleme
Copy link
Contributor

nleme commented Apr 1, 2015

I made two commits in the feature branch "feature/contentLengthFixes" to implement the fix for this issue. I list the two commits below:

4dd27db

cddeb67

I'll be waiting until release 18 is out the door before merging this code into develop.

@nleme
Copy link
Contributor

nleme commented Apr 3, 2015

I just opened a pull request to merge the changes from "feature/contentLengthFixes" (which were made in the two commits quoted in my previous comment) into develop. The pull request is:

#169

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants