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

When trying to see a video, warn the user if the video has already been downloaded #7360

Open
3 tasks done
brunoais opened this issue Nov 4, 2021 · 7 comments
Open
3 tasks done
Assignees
Labels
feature request Issue is related to a feature in the app

Comments

@brunoais
Copy link

brunoais commented Nov 4, 2021

Checklist

What feature do you want?

When trying to watch a video that has already been downloaded in any resolution and the file, do not start downloading the video and show a dialog letting the user know that there's a downloaded version already. Allowing the user to:

If the file is still present:

  1. Press to ignore (in which the program continues normally)
  2. Press to open the stored video (I.e. Request Android to open to file by broadcasting an intent)
  3. Open the file manager on the directory where the video is (if possible)

If the file is no longer present:

  1. Press to ignore (in which the program continues normally)
  2. Press to go to where the file was downloaded to (opens the file manager)
    (additionally tell the file is no longer where it was downloaded to)

Why do you want this feature?

  1. It's hard to keep track of what videos I have already been downloaded with newpipe when I navigate to see them. The more I download, the harder it is to manage.
  2. It's easier to find the video I want to watch next in newPipe than trying to find using the file manager
  3. I can save bandwidth by just replaying a video I've downloaded than consuming it again when I forget I had downloaded it already.

Ideally (for the user) newPipe would be able to read the files it downloaded from but that has been denied, so I'm creating this proposal as a 2nd best option.

Note: This issue is a followup to #478 being closed.

@brunoais brunoais added the feature request Issue is related to a feature in the app label Nov 4, 2021
@NoSphyrna
Copy link

Hello, we are a group of students and we want to contribute to this issue, can we work on it ?

@XiangRongLin
Copy link
Collaborator

I would vote against implementing this issues as it is described.
Introducing a read from the file system whenever one is starting a video sounds like a very big overhead to me.
This overhead may be observable for users in a longer load time, when only a small part of the users want this feature.

@litetex

@brunoais
Copy link
Author

brunoais commented Nov 23, 2021

@XiangRongLin
That one I need to understand.
As far as I know, in all ext4, FAT32, exFAT and F2FS have very fast read speeds for stats (stat linux syscall, which is also available in the android).

Before I did this suggestion, I experimented with running time stat [file] on a friend's phone (using adb) and, after 5 runs, a stat command took a maximum of 0.03s (counted 0 for system; I'll count 0.01s) (worst being in the SD card).

time stat DCIM | grep "real"    
    0m0.02s real     0m0.02s user     0m0.00s system

That was tested on a budget phone running Android 8

Knowing that it's a very very fast, the only thing left that can be a performance problem is the app's internal DB of downloaded videos.
However, with 50 downloaded videos, the list of downloaded videos opens and gets filled too fast for a human to see.
Given such:

@XiangRongLin, What am I missing here?

@XiangRongLin
Copy link
Collaborator

That was tested on a budget phone running Android 8

Does this also hold for *looks at minSDK* android 4.4 and the corresponding hardware from 2013 or even.
To little market share to matter

all ext4, FAT32, exFAT and F2FS have very fast read speeds for stats

What about all the overhead of doing this out of java/the application? Doing a syscall is definetly fast, but i find it hard to imagine that File.exists() does this.


If it is also that fast on older devices within the application then it's fine for me, even if I don't like it.

Reason for why I don't like it is because it's a synchronous process that completly blocks the user for it's duration from loading the video for a feature that they may not need.
I for myself nealy never download videos.

@brunoais
Copy link
Author

brunoais commented Nov 23, 2021

That was tested on a budget phone running Android 8

Does this also hold for looks at minSDK android 4.4 and the corresponding hardware from 2013 or even. To little market share to matter

I don't know what phone you'd need it tested on. My friend only has Android 8. Whoever still uses Android 4.4, this would work anyway, just slower (I wonder if really significant). Those people would already be used to overall slowness in their phones anyway.

all ext4, FAT32, exFAT and F2FS have very fast read speeds for stats

What about all the overhead of doing this out of java/the application? Doing a syscall is definetly fast, but i find it hard to imagine that File.exists() does this.

I'd need to read the AOSP java to see which of these two options (linux syscalls) it uses but I'm almost certain it is one of these two (one is used in windows, the other one in linux, for java 11 on desktop):

  1. Execute stat C syscall.
  2. Open file for read (open C syscall)

With only 4 hoops in the call stack to reach either. Probably also a tinny little more jvm overhead. I can try making a basic java program and run it on that phone, if you really need an example.

I don't know if you use windows for development but accessing data in a windows filesystem is much more expensive than accessing data in a linux filesystem. So using windows' experience as a baseline for that doesn't work

If it is also that fast on older devices within the application then it's fine for me, even if I don't like it.

Reason for why I don't like it is because it's a synchronous process that completly blocks the user for it's duration from loading the video for a feature that they may not need. I for myself nealy never download videos.

I see what you mean. However, if the feature is done right, there's no reason to even check if the file exists, when it was not downloaded in the first place.
Additionally, if the feature is done right, the load to check if it has been downloaded can be done concurrently to loading the
video. Just the time it takes to made the sliding transition upwards (or whatever other) with the video is enough to:

  1. Spawn a new thread or use a thread from a thread pool.
  2. Locate the video in the database.
    2.1 If it's there and downloaded:
    2.1.1. Test if the video is at downloaded location.
    2.1.2. Show the dialog.

How I tested the performance concept:

echo -e "[40 lines of loremipsum]" > db_test
time (cat db_test | (grep 'Duis aute' && echo "HERE!" || echo "NOT HERE"))

I could have done the grep only but I thought this would make it harder for the code itself.
0m0.13s real 0m0.07s user 0m0.02s system

That is with the very worst kind of database the program would ever get. Reading the whole file looking for those two words.

Reason for why I don't like it is because it's a synchronous process that completely blocks the user for it's duration from loading the video for a feature that they may not need. I for myself nearly never download videos.

That's fine. No dialog for you. If not ever downloaded, no dialog.

@XiangRongLin
Copy link
Collaborator

XiangRongLin commented Nov 23, 2021

@NoSphyrna You guys can give it a try, but I find this issue on the harder (or rather more annoying) side of things, because performance plays a role here.
I'm assuming that you guys are still early on in your studies, so I would recommend starting out without doing it concurrently and seeing if a user can notice the check and then decide if a concurrent solution is needed.


I'm also a student with a part time job so take my advice with a grain of salt.

@brunoais
Copy link
Author

brunoais commented Nov 23, 2021

As a software dev for some years although with little experience with android, I can tell that without using threads, they can't have the slide animation of starting a video at the same time you check for the downloaded file.
However, there's no concurrency here. There's threads but it's not concurrent programming.

This page appears to have all the important parts for the threading, specially the anchored part:
https://developer.android.com/guide/background/threading#communicating-with-the-main-thread

Other than that, there's no extra knowledge required besides what would be required in a synchronous execution.

@NoSphyrna I'm a beginner with Android java API but I know Java very well (ins and outs) so I can give you support in developing this feature. I don't have time right now to do it myself, unfortunately (It's been years since I had student amounts of time) but I have time to do some QA coaching a few times per week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app
Projects
None yet
Development

No branches or pull requests

3 participants