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

Feed order and subscriptions groups #2309

Merged
merged 24 commits into from
Mar 14, 2020
Merged

Conversation

mauriciocolli
Copy link
Contributor

@mauriciocolli mauriciocolli commented Apr 25, 2019

  • Sort the items chronologically and by type (live, then upload date in descending order)
  • Let users group their subscriptions
  • Load the feed items in parallel in a background service (like importing works)
  • Show the watching count in live stream items
  • Use Groupie library for easier development of lists

Demonstration

demo.gif

Future improvements (follow ups)

  • Showing the watching state (progress) in the items
  • Migrate other lists to Groupie
  • Make localization changes dynamic
  • [Maybe] Use the new structure YouTube, it uses JSON which is easier to parse and less error prone

Use RSS feed for YouTube

Edit: Added in the latest updates.

One disadvantage that I found is that it doesn't return current live streams, but maybe that's not a huge problem?

The advantages though are:

  • Very lightweight
  • Easier to parse
  • Available for all (most?) channels

I'll think about it a little more the next few days. Any suggestion/idea is welcome.

Loading Strategy Discussion

There's an issue if we just load everything concurrently: overload on the servers of the services. This can results in errors when doing requests and can put some stress on some services, which we may not want to do it.

I haven't decided what strategy to take here, maybe limit how many channel per second/time period, custom rates for each service, etc.

In the current state, a lower concurrent request rate is used. But in the end, we have to balance two things: services servers load and time to load everything.

Waiting for your opinions on this.


Related extractor PR TeamNewPipe/NewPipeExtractor#158
Closes #822, #739, #565, #1752, #1725, #1437, #1946, #1919
Related #1448

@justanidea
Copy link
Contributor

We all love you 😍

@justanidea
Copy link
Contributor

Is it possible that you post an apk test to permit us to send you feedbacks?

@Brimont
Copy link

Brimont commented Apr 26, 2019

How compile this on android studio? I love your work

@mauriciocolli
Copy link
Contributor Author

Thanks!

@justanidea Sure, here you go: org.schabi.newpipe.pr2309.apk.zip

@Brimont The easiest way is checking out the branch that I'm using in this PR (this one at my fork). Then it should be straightforward as opening the project in Android Studio and hitting "Run".

@justanidea
Copy link
Contributor

@mauriciocolli
I tested the apk and this is my opinion :
All this work is so clean and efficient, i cant believe we finally have it on Np and i think im not the only one to be super hyped about it :)
However i have some suggests, to make it even closer to perfection:
(just in my opinion)

  • the order of the feed's icons should be sorted automatically : a too long list of custom feeds makes the icons go outside of the visible field so its harder to access them. But by putting the last icon used by the user on the left of the list, it would make it easier to access.

  • I was also thinking a "Play in background | Play all | Play in Popup" button banner at the top of each customized feed : its more handy if the user desire watching all the videos he missed in one time.

  • With these new feeds, the regular "feed" tab should probably be changed : the icons in the subscription tab should be also visible from the feed tab imo.

  • About the feed refreshing time, its quite long but it doesnt annoy me with my hundred of subs. But what could be done to make it faster in some case is to make possible refreshing manually just a selected feed by the user. Like that, if u just want to refresh your "music" tab, you dont have to also load all your vlogs or any other videos.

  • Making these custom feeds is amazing but it takes a bit of time. So what would be great is the ability to export them with all the other contents (subs, settings, playlists), so the user can change of phone without fearing doing his feeds all over again.

So even if a lot of things could be done i want to remind you how amazing this work had already been, and that a lot of persons are already happy with it :)
Thank you a lot for this time taken for it 🔥

@TobiGr
Copy link
Contributor

TobiGr commented Apr 27, 2019

Welcome back! It is good to hear from you. I absolutely love these changes ❤️

@Stypox
Copy link
Member

Stypox commented Apr 28, 2019

Why are there two car icons?
56865698-14d38a80-69d1-11e9-86f0-1dddfad6aa1d

@Brimont
Copy link

Brimont commented Apr 28, 2019

Exception

  • User Action: ui error
  • Request: App crash, UI failure
  • Content Language: IT
  • Service: none
  • Version: 0.16.0
  • OS: Linux Android 4.4 - 19
Crash log

java.lang.NoClassDefFoundError: java.util.Objects
	at org.schabi.newpipe.extractor.localization.Localization.equals(Localization.java:87)
	at org.schabi.newpipe.settings.ContentSettingsFragment.onDestroy(ContentSettingsFragment.java:142)
	at androidx.fragment.app.Fragment.performDestroy(Fragment.java:2699)
	at androidx.fragment.app.FragmentManagerImpl.moveToState(FragmentManager.java:1591)
	at androidx.fragment.app.FragmentManagerImpl$3.onAnimationEnd(FragmentManager.java:1673)
	at android.animation.AnimatorSet$AnimatorSetListener.onAnimationEnd(AnimatorSet.java:765)
	at android.animation.ValueAnimator.endAnimation(ValueAnimator.java:1012)
	at android.animation.ValueAnimator.access$400(ValueAnimator.java:51)
	at android.animation.ValueAnimator$AnimationHandler.doAnimationFrame(ValueAnimator.java:623)
	at android.animation.ValueAnimator$AnimationHandler.run(ValueAnimator.java:639)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:776)
	at android.view.Choreographer.doCallbacks(Choreographer.java:579)
	at android.view.Choreographer.doFrame(Choreographer.java:547)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:762)
	at android.os.Handler.handleCallback(Handler.java:725)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:153)
	at android.app.ActivityThread.main(ActivityThread.java:5297)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:511)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:833)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:600)
	at dalvik.system.NativeStart.main(Native Method)


@Brimont
Copy link

Brimont commented Apr 28, 2019

I have this with your apk, with normal apk i not have problem. Is my Phone?

@mauriciocolli mauriciocolli force-pushed the feed branch 2 times, most recently from 19b8ade to e050a9c Compare April 28, 2019 20:45
@mauriciocolli
Copy link
Contributor Author

@Stypox Oops, fixed and added a test.


@Brimont some classes were trying to use some changes in Java that apparently were not included in your specific ROM (sometimes it doesn't include the latest ones). I removed the use of these changes.

Can you test this version? org.schabi.newpipe.pr2309-2.apk.zip


@justanidea

But by putting the last icon used by the user on the left of the list, it would make it easier to access.

I think the user would lose the ability to remember their positioning in the list, because even a group that they didn't used often would be the first as soon as they accessed it.

What I do think is missing is to let the user reorder the groups.

I was also thinking a "Play in background | Play all | Play in Popup"

the regular "feed" tab should probably be changed

Agreed, changes for a future PR though.

make possible refreshing manually just a selected feed by the user

I thought about doing that, but then the "Last Updated" in every feed would be probably wrong and items would be up-to-date differently, as subscriptions can be in multiple feeds and are updated individually.

I don't know if it's worth removing it entirely or just accept that it would be misleading...

We could also hide the items that were updated/inserted after the last update of the selected feed group, that way we could be more or less consistent thoughout the groups. Maybe?

ability to export them with all the other contents

The groups will be exported like anything else, as they are in the same database.

@Brimont
Copy link

Brimont commented Apr 29, 2019

@mauriciocolli thanks this apk work!
sorry if i tell you this :( but it would not be better in the feeds to put the last video of each channel? and not that puts in order all the videos of each channel for all the existing feeds ( i have 89 channel)
Thanks for your fantastic work i love your work.

@ThatIsAPseudo
Copy link

ThatIsAPseudo commented May 2, 2019

Thank you for this PL that's what we were all waiting for!

Concerning the watching state/progress of items, I think we should be able to mark a video as "watched" from the items list through a single button/menu, because since NewPipe does not use youtube API nor google services the only way that you can track watching state is by recording it within the app itself.

However, NewPipe is not the only/main support for watching youtube videos so the tracking system won't be able to track all of them, so we should have a mean to solve that issue.

Edit : spelling mistake

@juanfra684
Copy link

@mauriciocolli many thanks for your work on this feature. The groups in your second apk are great for users who like to watch the "what's new" section of NewPipe but I miss something. I usually prefer to navigate channel by channel and watch videos, even when the videos are a little old (e.g. 4 months). If I use the group "what's new" feature and create a group for electronics channels, it will hide the old (but still useful) videos quite bellow of the more active channels.

Could you create a new view (or whatever android calls this thing) to show a list of channels in every group?.

Maybe a button at the left of the three points button at the top of the app which allows to change between the view of group "what's new" and the list of channels in the group. With it, you would have a feature for people who wants the "what's new" and people like me who usually prefers the list of channels. The best of both worlds.

In any case, that's just an idea, I will be happy with whatever you decide.

Also, I like ThatIsAPseudo's idea for the watched videos but that probably is better for a separate issue.

@Stypox
Copy link
Member

Stypox commented May 9, 2019

What about an option, chosen on group creation, that specifies whether to show only the first video from the channels or to show all of them (thus putting old ones down in the list)?

@juanfra684
Copy link

The problem is that I don't know how many non-watched videos I've on every channel. Even if you add an option to choose the number of new videos per channel, I would miss some videos.

@jlucfarias
Copy link

@juanfra684 I think your problem with non-watched videos can be fixed with #2288

@justanidea
Copy link
Contributor

Issue : ended streams stay forever in the feed, even after refreshing.

As you can see with this stream of CGP Grey, which now ended 5 days ago

@TobiGr TobiGr mentioned this pull request May 12, 2019
1 task
@justanidea
Copy link
Contributor

Other issue about livestreams: (linked with the previous one)
When a stream is restarted then you have a duplicate of this stream, one working, the other (previous one) doesnt:

(Ambient mix)

@KarupoegPuhh
Copy link

KarupoegPuhh commented May 19, 2019

every time i try to go back from some custom feed, the ui crashes, not sure if this is my custom os's fault (unoffical resurrection remix)

edit: i had system animations disabled, if they'r enabled everything works normal

Exception

  • User Action: ui error
  • Request: App crash, UI failure
  • Content Language: GB
  • Service: none
  • Version: 0.420.69
  • OS: Linux Android 7.1.2 - 25
Crash log

java.lang.IllegalStateException: activity must not be null
	at org.schabi.newpipe.local.feed.FeedFragment.onDestroyOptionsMenu(FeedFragment.kt:114)
	at androidx.fragment.app.FragmentManagerImpl.dispatchCreateOptionsMenu(FragmentManager.java:3335)
	at androidx.fragment.app.FragmentController.dispatchCreateOptionsMenu(FragmentController.java:331)
	at androidx.fragment.app.FragmentActivity.onCreatePanelMenu(FragmentActivity.java:379)
	at androidx.appcompat.view.WindowCallbackWrapper.onCreatePanelMenu(WindowCallbackWrapper.java:94)
	at androidx.appcompat.app.AppCompatDelegateImpl$AppCompatWindowCallback.onCreatePanelMenu(AppCompatDelegateImpl.java:2549)
	at androidx.appcompat.view.WindowCallbackWrapper.onCreatePanelMenu(WindowCallbackWrapper.java:94)
	at androidx.appcompat.app.ToolbarActionBar.populateOptionsMenu(ToolbarActionBar.java:455)
	at androidx.appcompat.app.ToolbarActionBar$1.run(ToolbarActionBar.java:56)
	at android.view.Choreographer$CallbackRecord.run(Choreographer.java:874)
	at android.view.Choreographer.doCallbacks(Choreographer.java:686)
	at android.view.Choreographer.doFrame(Choreographer.java:618)
	at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:860)
	at android.os.Handler.handleCallback(Handler.java:751)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:154)
	at android.app.ActivityThread.main(ActivityThread.java:6236)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:891)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:781)


@TobiGr
Copy link
Contributor

TobiGr commented Jun 3, 2019

@mauriciocolli Works great! I don't have time to dig into the code now, but let me try to understand your question quickly:

There's an issue if we just load everything concurrently: overload on the servers of the services. This can results in errors when doing requests and can put some stress on some services, which we may not want to do it.

In the current state, a lower concurrent request rate is used. But in the end, we have to balance two things: services servers load and time to load everything.

I think a lower rate ist okay, espacially when we take a look at upcomming services like Peertube etc. which are mostly selfhosted. Maybe, it is possible to have a higher rate on "commercial" / "professionally hosted" servers like YouTube/SoundCloud and a lower on selfhosted. But that's not necessary, I'd say, we can keep the request rate low and when people complain, we can still change it.

@phatloot123
Copy link

phatloot123 commented Jun 19, 2019

I got this crash using your latest apk after I turned the screen off:

Exception

  • User Action: ui error
  • Request: App crash, UI failure
  • Content Language: GB
  • Service: none
  • Version: 0.420.69
  • OS: Linux Android 8.0.0 - 26
Crash log

java.lang.IllegalStateException: Can not perform this action after onSaveInstanceState
	at androidx.fragment.app.FragmentManagerImpl.checkStateLoss(FragmentManager.java:2080)
	at androidx.fragment.app.FragmentManagerImpl.enqueueAction(FragmentManager.java:2106)
	at androidx.fragment.app.BackStackRecord.commitInternal(BackStackRecord.java:683)
	at androidx.fragment.app.BackStackRecord.commit(BackStackRecord.java:637)
	at org.schabi.newpipe.util.NavigationHelper.openChannelFragment(NavigationHelper.java:312)
	at org.schabi.newpipe.local.subscription.SubscriptionFragment$listenerChannelItem$1.selected(SubscriptionFragment.kt:283)
	at org.schabi.newpipe.local.subscription.SubscriptionFragment$listenerChannelItem$1.selected(SubscriptionFragment.kt:282)
	at org.schabi.newpipe.local.subscription.item.ChannelItem$bind$$inlined$run$lambda$1.onClick(ChannelItem.kt:41)
	at android.view.View.performClick(View.java:6897)
	at android.view.View$PerformClick.run(View.java:26104)
	at android.os.Handler.handleCallback(Handler.java:789)
	at android.os.Handler.dispatchMessage(Handler.java:98)
	at android.os.Looper.loop(Looper.java:164)
	at android.app.ActivityThread.main(ActivityThread.java:6944)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)


@Kommynct
Copy link

Kommynct commented Jun 19, 2019

This might be unrelated but is there any way posts by the YouTuber could be intermingled with the videos?

That'd be feature parity with subscriptions in the actual app.

@B0pol
Copy link
Member

B0pol commented Mar 14, 2020

I just noticed you introduced %s seconds plural string. It's already in NewPipe (but not when you submitter your PR) and is already translated in many languages, with title dynamic_seek_duration_description.

We definitely should address this problem before the next Weblate sync because people shouldn't translate twice seconds plural strings.

The logical solution would be delete seconds and rename dynamic_seek_duration_description into seconds, but the problem with this solution is that Weblate will 100% mess up in the next sync.
Then I think the best solution, but hackish would be to use dynamic_seek_duration_description instead of seconds as name of plural string.

@Stypox
Copy link
Member

Stypox commented Mar 15, 2020

I realized the layout subscription_header.xml was not removed, even though it is unused.

@tmbgreaves
Copy link

Massive thanks @mauriciocolli for your work on this, can't wait to see it released now. I have had a bounty on this for some time now and would be very pleased if you claimed it.

@mauriciocolli
Copy link
Contributor Author

@tmbgreaves Thanks!

About the bounty: it seems like #739 was sort of replaced by #1448 and all the backers posted their bounties to the first issue, however, one (@earboxer) left his comment on the former, while two (you and @bendem) left their comments on the latter.

It seems like you're fine with the results of this one, and by the message that @bendem left, he only refers to the first issue (and @earboxer only commented on that one).

Did I misunderstood something, or the bounty claim would be valid in this case?

@mauriciocolli mauriciocolli deleted the feed branch March 17, 2020 01:11
@earboxer
Copy link

This looks good, and I think I will be happy once the update gets on F-Droid.

@Stypox Stypox mentioned this pull request Mar 17, 2020
@tmbgreaves
Copy link

@mauriciocolli I think I moved my bounty from one issue to the other at some point, perhaps? In either case I think you can just 'claim bounty' on that issue which I would be very happy to authorise. I was already a regular NewPIpe user but the apks you have been posting up have been a massive improvement in usability, exactly what was needed to complete this app for me. Just waiting on it to arrive in FDroid now!

@Stypox Stypox mentioned this pull request Mar 22, 2020
@B0pol
Copy link
Member

B0pol commented Mar 22, 2020

I've a question

I've read this line

But when I use fast mode, I still have duration. Then how do you gather it?

@Stypox
Copy link
Member

Stypox commented Mar 22, 2020

If you have already opened the stream, its basic information was saved in the database. So when an item in the feed is missing a field, it tries to obtain it from db

This was referenced Mar 25, 2020
@bendem
Copy link

bendem commented Mar 27, 2020

Sorry for not accepting the bounty release, I was waiting for the PR to hit FDroid to be able to test it.
Is there an APK somewhere I can download since it doesn't seem like it's coming?

@wb9688
Copy link
Contributor

wb9688 commented Mar 27, 2020

@bendem: See #3266. Anyway, v0.19.0 will likely be released on Sunday, and then it could take ±3-7 days for it to be on F-Droid.

@bendem
Copy link

bendem commented Mar 27, 2020

I just found it yeah. I was enable to import my subscriptions from the old version, but I could import it from youtube to test at least. This is great work really!

@wb9688
Copy link
Contributor

wb9688 commented Mar 27, 2020

@bendem: How did you try to export/import it? In Settings or in the Subscriptions tab at Home? Those are entirely different things. If the latter, that's strange as it should just work. If the first, I remember there being some issues with database imports and exports. Either way, what exactly doesn't work? Do you get the crash reporter?

@bendem
Copy link

bendem commented Mar 29, 2020

No worries, I found my way around, I just came up on the subscription screen and there was a button to import from youtube and a button to import from previous export, so I guessed the export option from the previous version was it.

I used the import thing in the settings instead, worked fine.

@lu4p
Copy link

lu4p commented Apr 1, 2020

This is awesome thanks a lot.

@Wilker-uwu
Copy link

i finally finished taking some hours to read this. i love you all 😻

there is one suggestion which i think should probably be for another pr but idk what to do tbh.
i actually just want to know if the feed will keep older videos while uploading. because the feed is limited to a certain point, believe that keeping older entries (with option to remove these, and the same option if you stop following a channel) would be really useful for the purposes of catching-up after not watching for some time.

thank you a lot for this ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deep review required Maintainer must double check/test/review this due to changes in API or architecture feature request Issue is related to a feature in the app
Projects
None yet