Autoload next video

[ghost]

@kelson42 commented on Dec 17, 2019, 3:09 PM UTC:

I thought this would be a nice feature and by testing the ZIM about Mali I discovered that it was somehow working halfway. At the end of the video it seems that new one is played... but the HTML page does not change... it is buggy!

This issue was moved by kelson42 from openzim/youtube#60.

[ghost]

@rgaudin commented on Dec 17, 2019, 3:12 PM UTC:

Can you be more specific. Each article page is completely independant of the others. So it should not magically start playing another video. Can you please give instructions to reproduce ? Also, is this a bug report (what's the bug?) or a feature request?

[ghost]

@kelson42 commented on Dec 17, 2019, 3:19 PM UTC:

1 - Open ZIM file about Mali 2 - Click on video about la révisoin constitutionelle 3 - Seek to the end

The video reaches the end (end the audio start with "Bonjour à tous pour cette deuxième video sur la monnaie"... the video seems stuck for a few seconds... and then somewhoe the video moves ahead too (with the right content).. but the video progress bar is still stuck at 100% and the HTML does not have changed.

[ghost]

@rgaudin commented on Dec 17, 2019, 3:23 PM UTC:

OK thanks ; can you reproduce it with kiwix-serve ?

[ghost]

@kelson42 commented on Dec 17, 2019, 3:25 PM UTC:

Work fines with Kiwix Serve, see http://library.kiwix.org/mali-pour-les-nuls_fr_all/A/3mn_contre_la_revision_constitutionnelle.html

[ghost]

@rgaudin commented on Dec 17, 2019, 3:27 PM UTC:

Then have you considered this might be an android bug? Source code references a single video file…

[ghost]

@kelson42 commented on Dec 17, 2019, 3:33 PM UTC:

@rgaudin No, but I might be wrong ;) @macgills Possible?

[ghost]

@macgills commented on Dec 17, 2019, 4:01 PM UTC:

Have you looked at kiwix-serve on your phone? I have absolutely no idea

[ghost]

@kelson42 commented on Dec 17, 2019, 4:07 PM UTC:

@macgills yes, works fine. Can you reproduce the issue?

[ghost]

@macgills commented on Dec 17, 2019, 4:15 PM UTC:

No, it is different. If I play the video then after the video ends it seems like it plays some audio but quickly stops

[mgautierfr]

From a small investigation :

From https://github.com/kiwix/kiwix-android/blob/eaee39867c96d12b225a2b61a2d064b3dd72164c/core/src/main/java/org/kiwix/kiwixmobile/core/reader/ZimFileReader.kt#L151-L157, it seems that we simply create a ParcelFileDescriptor from a file descriptor seeked at the right offset. However, we don't give any information about the size of the content to read.

And from https://developer.android.com/reference/android/os/ParcelFileDescriptor it doesn't even seems possible.

Maybe there is a better solution, but the one I see for now is to implement a custom child of ParcelFileDescriptor that reimplement the method writeToParcel(https://developer.android.com/reference/android/os/Parcelable.html#writeToParcel(android.os.Parcel,%20int) and ensure that we don't read content in the zim file after the end of the video.

[macgills]

writeToParcel is just a way of android passing objects between processes, marshalling/unmarshalling to a Parcel. We would have to do something else that I am not entirely sure of, I'll need to investigate the ParcelFileDescriptor a bit more

[mgautierfr]

ParcelFileDescriptor seems to be a class encapsulating a file descriptor (or a pipe or .. whatever can be streamed). We probably pass it to the android player. Then the player use writeToParcel to get the content of the stream (the video content in our case) to play it.

The problem is probably that ParcelFileDescriptor read the fd until the end of the (zim) file. If the content after the video is something that the player cannot read, the player stop (with some kind of invalid data error). But if the content is another video, the player start to play it as the ParcelFileDescriptor keep giving it data (instead of doing as if we reach the end of the file)

[macgills]

It is not reading anything by uing writeToParcel, that is a method from the Parcelable interface

/**
 * Interface for classes whose instances can be written to
 * and restored from a {@link Parcel}.  Classes implementing the Parcelable
 * interface must also have a non-null static field called <code>CREATOR</code>
 * of a type that implements the {@link Parcelable.Creator} interface.
 * /

We pass the parcelFileDescriptor to the ContentProvider when the contentProvider asks for it via the Webview. The fileDescriptor must be parcelable because contentProviders can share data between processes. How the video actually gets played I have no idea

[kelson42]

@macgills @mgautierfr The libzim knows how big is the video/audio. Would that helps if somehow we could tell the player to not go over a certain size? Is that possible at all?

[macgills]

I don't see how right now, the entire thing is set up to read a file and we are telling it to read a portion of a file. We can write the file out of the zim temporarily and then pass along the pointer to that file but I have concerns over the speed of that

[rgaudin]

Not to mention the size issue given video files are usually large…

I wonder how stat() works and what it returns from that fd though the fact that we saw next video being played automatically indicates it might not be using stat results.

Would be interesting to know what triggers EOF given we don't provide that information yet it works as expected in most cases.

[macgills]

The theory is that the adjacent bytes are incompatible with the player and cause an exception but if 2 video files are sequential in memory it starts playing the next one

[macgills]

We extract the fileDescriptor from a RandomAccessFile which we preform a seek on with the offset, RandomAccessFile also does have a setLength, I wonder if this would be respected

[kelson42]

We extract the fileDescriptor from a RandomAccessFile which we preform a seek on with the offset, RandomAccessFile also does have a setLength, I wonder if this would be respected

We should check this. I believe the length of the RandomAccessFile might not be set at all.

[macgills]

To do that I would need to know the length, I can run a quick experiment with a hardcoded length and see if the video stops

[kelson42]

If you struggle, give me a url and zim file and i will tell you the size.

[kelson42]

Here are the details for the ZIM about Mali: zimdump.txt

[rgaudin]

Interestingly, there's no two videos next to each other in this example. I believe that's the one you encountered the overlap in.

[macgills]

setLength will truncate the zim file so is not feasible to use

[mgautierfr]

As I said, I think we should try to create our custom ParcelFileDescriptor (inheriting the android's ParcelFileDescriptor) to store the size.

It seems that this the method writeToParcel is then called by "android" to fill a parcel with a chunk of the data to read.(and play). If we use our custom class, we can use the size to stop filling the parcel and behave as if we are at the end of the file.

I've not tested this, but it is my main (and only) idea.

[macgills]

And as I have said @mgautierfr that is not what a Parcel is. Parcelable is how android passes data between processes, this ParcelFileDescriptor allows the fileDescriptor to be passed between processes by giving instructions on how to write a fileDescriptor to a parcel and how to read a fileDescriptor from a parcel. It does nothing with what the fileDescriptor points to.

We may have more luck with getting rid of the ContentProvider altogether and overriding shouldInterceptRequest in or WebViewClient so we can return a WebResourceResponse which can take an InputStream as a constructor parameter so we can actually have some control over how to supply the data and no longer lock ourselves to a FileDescriptor. I wasn't aware of such a solution until earlier today when researching the youscribe problem, I don't know if it has been attempted prior but I would say it warrants investigation

[mgautierfr]

And as I have said @mgautierfr that is not what a Parcel is. Parcelable is how android passes data between processes, this ParcelFileDescriptor allows the fileDescriptor to be passed between processes by giving instructions on how to write a fileDescriptor to a parcel and how to read a fileDescriptor from a parcel. It does nothing with what the fileDescriptor points to.

Ho, I didn't understand your previous comment. Sorry.

So, maybe by overriding https://developer.android.com/reference/android/os/ParcelFileDescriptor.html#getStatSize() ? I don't know how the fd is send to the videoPlayer using the parcel but, if are lucky it also pass the size of the video using getStatSize to know it. If we change it to the real size of the article (or offset+size), maybe it is enough. Else we would have to try your another lead. I don't know what is best or easier to test.

[macgills]

Overriding getStatSize does seem more promising, I can run quick experiments to see if it does work.

I do think that ultimately if we can pass InputStreams to the WebView that would be superior on a lot of counts as it better reflects the reailty of what we are doing ie just chucking bytes at it, not actual files which FileDescriptors represent

[macgills]

Overriding getStatSize did not produce a result. I hardcoded the size as 1_000L and content played regardless.

Also I believe after some light investigation that what we are recreating is an AssetFileDescriptor and if we follow my proposal (and it actually works) we will be feeding in an AssetFileDescriptor.AutoCloseInputStream to the WebResourceResponse

[kelson42]

@macgills Two questions:

• You confirm that using InputStreams fixes the problem because you have tested?
• Do you need something from the libkiwix to get things working with an InputStreams?

[macgills]

@kelson42 I do not confirm InputStreams fix the problem though I would very much believe it does, I have not been able to verify thus far, it requires a bit of time investment. To create an AssetFileDescriptor I would need filename, offset & size so the only thing missing from kiwixlib is size

[macgills]

It certainly sounds feasible, this article makes it sound quite robust

[kelson42]

the only thing missing from kiwixlib is size

@macgills Can you please link here to a libkiwix ticket requesting this?

[macgills]

https://github.com/kiwix/kiwix-lib/issues/327 @kelson42 done

[macgills]

@kelson42 I confirm that using inputStreams works, I have hardcoded the size and videos stop playing. I will experiment with using getContentPart to get an accurate size

[macgills]

works like a charm, #1769 is a duplicate of this issue, will close it

[macgills]

@kelson42 I am considering this blocked until kiwixlib offers a way for us to retrieve the size of an asset as a long

[mgautierfr]

FYI, this is done here https://github.com/kiwix/kiwix-lib/pull/328

[macgills]

@mgautierfr excellent! Is it published?

[mgautierfr]

Yes it is in release 9.0.0 (and version 9.0.1 integrate the change about the offset stored in a long instead of a int)

[kelson42]

@mgautierfr thx

[macgills]

@mgautierfr I am not seeing the method getArticleSize

[mgautierfr]

My bad. I've totally mixed up the things. The function is in master but not yet released. It will be available soon.

[macgills]

Cool, just let me know on release so I can update my draft PR to Ready For Review with the latest library.

[kelson42]

New kiwixlib has been published https://bintray.com/kiwix/kiwix/kiwixlib/

[macgills]

After the library upgrade the zim file reading is far more unstable. I might make a separate PR to just upgrade kiwixlib on develop and see if it is affecting the contentprovider too

[macgills]

Hmmm seems to be working fine on develop. Curious. Will investigate more tomorrow

[kelson42]

New kiwix-lib is now available https://bintray.com/kiwix/kiwix/kiwixlib/9.1.1

[macgills]

@kelson42 @mgautierfr I am still having an issue with the new api for fetching size by url

2020-04-20 16:27:07.693 org.kiwix.kiwixmobile E/kiwix: Unable to get size for url : zim://content/I/videos/qbO7_ivMldc/video.webm
2020-04-20 16:27:07.693 org.kiwix.kiwixmobile E/kiwix: std::exception

[macgills]

Ah I see my error, hold on, testing a fix