show overall duration of videos in playlist

[bg172]

trafficstars

earlier only overall amount of videos was shown in playlist (in both searched online playlist and local playlist). Now overall duration is shown there too - as formatted by existing Localization.concatenateStrings() and Localization.getDurationString().

What is it?

• [ ] Bugfix (user facing)
• [x] Feature (user facing)
• [x] Codebase improvement (dev facing)
• [ ] Meta improvement to the project (dev facing)

Description of the changes in your PR

earlier only overall amount of videos was shown. Now overall duration is shown there too - as formatted by existing Localization.concatenateStrings() and Localization.getDurationString().

APK testing

see https://github.com/TeamNewPipe/NewPipe/pull/6045/checks (artefacts) for the uptodate APK as soon as built by github - tested OK on Xiaomi Mi 9 SE

Due diligence

• [x] I read the contribution guidelines.

[XiangRongLin]

This would need to be done in the extractor, in order to be done properly. And it would only work if the services provide that specific information for us to parse

[bg172]

Hi @XiangRongLin, thank you very much for reviewing. Now reworked to show overall duration only for local lists - they are not so tricky I hope. Can you give an example-link of an infinite list by the way?

[XiangRongLin]

Can you give an example-link of an infinite list by the way?

https://www.youtube.com/watch?v=FfAjtZgLkPQ&list=RDFfAjtZgLkPQ

Its basically youtube.com/watch?v={videoID}&list=RD{videoID}

[bg172]

so now improved enough, tried with that infinite list. Next thing would be to extend the service to deliver the duration, but this service is not in our hands, right?:)

[XiangRongLin]

The service is in our hands https://github.com/TeamNewPipe/NewPipeExtractor

And like i said before, it needs to be fixed there. From a quick glance the duration would increase as more items are loaded. Imagine you are a user and experience that. Would you not be confused?

[bg172]

I am a user, and i am already satisfied with this - i did this implementation for me in the first line;) Most of playlists i see are under 100 items and this seems to be the threshold of the service currently to provide items, i.e. in many cases the duration will be correct, and the longer it is the less important to know its value exactly;) To not confuse users there is this "+" in the end of duration string like 12:20:23+ until exact entire duration is known, meaning it is longer than 12h20m23s. With your info that the service is editable, i consider my PR as kind of an incremental improvement, but not sure when i can make the next incremental step. And concerning its need: large durations e.g. thousands of videos would take also more space in the UI and might cost essential performance, not obvious anybody wants it. What do you think?

[triallax]

I think this could be made as an option disabled by default, with a note that for larger playlists, it might not display the actual duration immediately.

[bg172]

now made it configurable

[bg172]

This would need to be done in the extractor, in order to be done properly. And it would only work if the services provide that specific information for us to parse

Hi @XiangRongLin , if I do this via the extractor I have to return the overall duration in PlaylistInfo which is called per current design only once - when opening the view for a playlist. Because the online-service (e.g. youtube) does not provide playlist-duration it is needed to loop through all items, sum up the duration and provide it via something like PlaylistInfo.getStreamDuration() of extractor - for multipage playlists (>100 videos) it is needed to load all pages which takes too long (about 12s to show a playlist with 1615 videos I had tried - all this time the list itself appears to be loading and no items of it is shown to the user) which makes user experience too bad. The alternative which I implemented - summing up only for the items which are currently shown and considering additional items incrementally as soon as they are added - does not fit current design of extractor usage: the callback adding next page items handleNextItems(final ListExtractor.InfoItemsPage result) cannot read the original PlaylistInfo which would have new getter getStreamDuration() i.e. does not allow to update the duration incrementally.

So either we implement outside of extractor showing only partial duration in case of long playlist but having good performance, or we implement inside of extractor but kill the performance and show the exact duration which nobody needs in long playlists:)

What do you think, am I missing something?

[XiangRongLin]

That is unfortunate that youtube does not provide that information. Since the services (youtube) themself do not provide that information, it would then also not belong inside the extractor.

So either we implement outside of extractor showing only partial duration in case of long playlist but having good performance

I don't personally see much use in this solution, but I also don't like to decide about which features to add and which not. These discussions are best made in an issue and not the PR itself. @Stypox your thoughts on this, i'm slightly against this.

[bg172]

"without opening the playlist" is a good point, let me try.

Concerning units: I made it already without units, it looks like this now: 01:35:17 in case complete or in case incomplete 12:08:09+, but 12:08:09 (∞) is also ok if you confirm, @SameenAhnaf.

About "first 50 or 100 videos": it works already like this out-of-the-box just because this amount of videos the extrator delivers.

[bg172]

hi @SameenAhnaf , I checked about "without opening the playlist", and unfortunately cannot do because e.g. youtube provides info/duration of only 2 first videos on the search-results page with playlists - showing overall duration of just two first videos appears meaningless to me. Only when one opens the playlist, a meaningful amount of info/durations of videos becomes available. Might be possible with a larger redesign perhaps by sending more requests than the user directly triggers, but I cannot do it, sorry.

Showing duration for a channel (when one is open on the screen) I would consider as a new PullRequest, and seems to be possible as I understand the corresponding youtube page for that. But on my phone-screen there seems to be no space where to add duration - even the number of videos is not shown, see Were your screenshots done on some larger device like a tablet? Please suggest.

And some good news: on your screenshot with infinite number of videos no duration is shown I bet because it was not enabled in Settings as requested by @mhmdanas:), I now updated the link to the latest APK, sorry for confusion, can you please check again.

Cool that you've found my feature useful, looking forward to hear from you.

[bg172]

It seems like that we will have to keep the information beside the channel name as you can see for grid view.

This screenshot is a showing search-results page with playlists from youtube, for which youtube does not provide durations - not sure if possible to implement without youtube support but can you create a new feature request?

Thanks once again. But I wonder why this setting should be optional. Maybe, a setting like "Show duration of next unwatched videos" with options "None/All (Watched+Unwatched)/5/10/25/50/100 videos" could be added. This setting is required as none of us won't always have the time to stream hundreds of videos at a sitting.

If "All" is selected for an infinite playlist and duration of first 100 videos is 19:09:08, duration could be shown as "19:09:08+". If a certain value is set (5/10/25/50/100), no '+' icon needs to be shown for infinite playlists. If certain value (5/10/25/50/100) is set for this setting, "Watch history" is disabled, duration of first (5/10/25/50/100) videos will be shown.

Yeah, the dependency on watch-history would be a new advanced feature building upon this PR. And it would depend on the sort-by criteria too, which I was dreaming of as next implementation. Until that this setting is too much, therefore I will remove it now.

[Akv2021]

Any chances of merging this feature ? I think NewPipe can provide an "experimental features" section which enables features like above which might not work for all users but are useful for the smaller audience nonetheless.

[bg172]

Hi @Stypox, @Akv2021, thank you for your comments and attention to this pull-request! Meanwhile I detected there is already a similar feature in NewPipe available (v0.23.1), see videos attached - e.g. in a found list or a favorite list you can see the full duration of the list and remaining list-time over the list if you switch to a next video. Let me know what do you think:)

https://user-images.githubusercontent.com/6170902/179813800-8f8f950e-1e82-4521-a7b3-759f0bf6353a.mp4

https://user-images.githubusercontent.com/6170902/179813828-aae7defd-b6d3-4cb6-8cf4-96d91a465b14.mp4

[Stypox]

I think this PR would be useful anyway, even if a similar feature exists in another place

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

0.0% Coverage
0.0% Duplication

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[opusforlife2]

It feels like the current UI is a bit non-intuitive. Maybe there should be a prefix before it, like "Total duration: 45:03" or something.

[bg172]

It feels like the current UI is a bit non-intuitive. Maybe there should be a prefix before it, like "Total duration: 45:03" or something.

A nice suggestion! I don't know how to update this pull-request here probably because it is marked as closed, but I have an idea how to fulfill this request - see https://github.com/TeamNewPipe/NewPipe/compare/dev...bg172:NewPipe:showOverallDurationInPlaylist . Maybe somebody can take it over in a meaningful way?

[opusforlife2]

@Stypox Does this need a new PR?

[Stypox]

Yes, this should go in a new PR, but I guess it will be a quite simple change, so it can be merged in 0.27.0

[opusforlife2]

@bg172 ^ ^_^

[bg172]

Yes, this should go in a new PR, but I guess it will be a quite simple change, so it can be merged in 0.27.0

https://github.com/TeamNewPipe/NewPipe/pull/10952