Bad implementation for android Package.getInputStream().skip(long byteCount)

[ClementHard]

The implementation for the InputStream skip method should skip from the current position instead of skipping from the beginning (skip != seek).

InputStream is = mPackage.getInputStream(uri);
Log.d("tag", String.valueOf(dis.read())); // 1
Log.d("tag", String.valueOf(dis.read())); // 2
Log.d("tag", String.valueOf(dis.read())); // 3
Log.d("tag", String.valueOf(dis.read())); // 4

InputStream is = mPackage.getInputStream(uri);
Log.d("tag", String.valueOf(dis.read())); // 1
Log.d("tag", String.valueOf(dis.read())); // 2
Log.d("tag", String.valueOf(dis.skip(1))); 
Log.d("tag", String.valueOf(dis.read())); // 2 but should be 4
Log.d("tag", String.valueOf(dis.read())); // 3 but should be 5

[danielweck]

UPDATE (read below)

[ClementHard]

I use it to stream audio and video. The input stream is passed to the media player that uses skip.

[jccr]

@danielweck
It seems to be only used in one place:
https://github.com/readium/SDKLauncher-Android/blob/develop/SDKLauncher-Android/src/org/readium/sdk/android/launcher/util/EpubServer.java#L175

[danielweck]

Thanks @jccr I suspect @nleme will be looking into this as he is working on porting the cleaned-up + improved ReadiumSDK ByteStream / ContentFilter architecture to Android (iOS / OSX are already up to date, just so you know @Twibit ). Video/audio streaming has been an important aspect of our recent work, with or without encryption. So this issue will definitely need to be checked.

[danielweck]

@Twibit looking at where the skip() method is used, and checking the startFrom variable possible values, it makes sense to use std::ios::beg (not std::ios::cur). I guess the method should be renamed to avoid confusion.
https://github.com/readium/SDKLauncher-Android/blob/develop/SDKLauncher-Android/src/org/readium/sdk/android/launcher/util/EpubServer.java#L175

https://github.com/readium/readium-sdk/blob/develop/Platform/Android/jni/resource_stream.cpp#L118

[ClementHard]

Which method should be renamed ?
The skip() method is from java.io.InputStream and I don't think you can rename it easily.

In my app, I need to be able to read obfuscated audio/video with a custom mechanism. To allow audio/video to be deobfuscated on the fly while streamed, I need to override this InputStream implementation with mine, and the skip() method must skip from the current position, not from the beginning, to be used multiple times.

Have a look at the ByteArrayInputStream.skip() implementation.

[danielweck]

I agree totally, skip() should seek into the stream from the current position. But this is used in the current Android code to seek from the beginning, so the function call must be renamed, and the appropriate seek method implemented.

[danielweck]

Update: there's another issue with the way the HTTP response is built for a byte range request, so we'll probably address the skip() misuse there as well: readium/SDKLauncher-Android#44