[Ready] Play Queue

[karyogamy]

Play Queue is now feature complete. No future updates or bugfixes will be pushed to this branch, unless there is a feature breaking bug or requested change. Continued development will go on a separate branch: play-queue-enhancement.

Features:

• Gapless playback for both audio and video play queues
• Play queue controls: Select, Move, Remove, Enqueue, Shuffle
• Play queue menu for background and popup video player (click on notification)
• VLC-style play queue for main video player
• Enabled tunneling for HDR (only if both device AND codec is capable)
• Enabled integration with system-wide sound effects (DSP)
• Persistent quality selection in player
• Hold button to enqueue stream on detail page

Start a play queue by going into any playlist fragments:

A quick overview of the design (final):

Because NewPipe allows multiple players to co-exist as services, each player contains its own play queue. This also means all players now handles only playlists and single streams are now playlists of one item.

Each play queue keeps track of the queue and current index state and broadcast any changes to the states. Play queue has different implementations that expose methods to fetch more items if incomplete.

MediaSourceManager is used to react to the changes in play queue and reflect them on Exoplayer's dynamic playlist. With the new dynamic concatenating media source provided by the recent Exoplayer update, streams are now seemless during transition (if buffered).

By baking StreamInfoItem into a custom MediaSource, we now have much better control over loading StreamInfo and Exoplayer's dynamic playlist. This is done so each MediaSource can be individually prepared on-demand through calls from MediaSourceManager. This reduces the amount of bandwidth when loading while still enjoying the gapless capability from ExoPlayer.

Current tasks:

• Waiting for code review and merge

Reserved as separate PR:

• Local Playlist
• Watch history entry for playlist streams
• Fix intent too large when passing play queue
• Pitch dropdown to main video player

[mauriciocolli]

Fix intent too large when passing play queues

I think we'll have to pass as a file instead (file path, of course).

And the popup need the notification, as it is the only mean to close it.

---

The UI has some bugs (flashing some elements, jumps and others don't work), but nothing that we can't fix.
The core is pretty nice, I like it, I'll try to review a little more in depth when I have more time.

In the mean time, good work!

[karyogamy]

Thanks,

for passing large intents, I was thinking we can create a singleton for key-value store and use rxjava to clear entries on an interval.

As for the popup, would it be better if all the player controls are in the same place? I didn't even realize popup creates a notification until you mentioned that =P

[theScrabi]

I've tried it for a bit, and I found that first the background player may now need a proper UI in the app itself. Having only the notification for controlling the music, and now also the play list is a bit to much.

We need a proper UI for Music only now.

Also there should be buttons in the main player (as well as in the notification of the popup player) for switching to the next/previous video.

Also the notification for the popup player is not visible anymore.

[karyogamy]

The current plan is to have the background music notification to launch its own play queue activity (which also contains the playback controls).

The UI part will be worked on after the play queue core is complete. Currently this PR is to show a proof of concept of the design and exoplayer's gapless playback with dynamic playlists.

I'll add back the notification for popup soon. Done.

[theScrabi]

The current plan is to have the background music notification to launch its own play queue activity (which also contains the playback controls).

Thats ok I guess :)

[mauriciocolli]

For passing large intents, I was thinking we can create a singleton for key-value store and us...

For passing data, maybe this would work, but to retain in the case of the user leaving the app it won't do very well (consider the case of a large list that a bundle can't handle). I created the StateSaver for this purpose, it should be flexible enough to allow us to use it.

Maybe you want to add that this part is broken (user exiting and coming back) in the todo list, just for the record.

[karyogamy]

Just fixed pause and resume as these don't require saving the states.

However, it is a good idea to have some ways to restore the states when gc decides to wipe the activity. This will probably be another PR though, as the current main player (master branch) do not support this yet (onCreate finishes the activity when no intent is found).

[karyogamy]

@theScrabi @mauriciocolli

I have recently revamped the quality selection with ExoPlayer's build-in track selector. However, I realized this change may conflict with the architecture on how video streams are passed from the Extractor. Please go over the change and tell me if we should stick with the old quality switching mechanism.

Also, there are also some problems with using track selections where Extractor media sources (mp4) have unknown framerate. This is solved by using DASH sources, but I have not seen a video with DASH source coming from the Extractor. Is this feature implemented yet?

With the StreamInfo baked into Exoplayer's media source, I think the play queue is in a state where I am comfortable to leave it and start working on the UI. Will probably start by this weekend if there is no other changes pending.

Do you think local playlist should be reserved as a separate PR? I believe this feature can be done independently of a working play queue at the moment.

[mauriciocolli]

The track selector will clash with the extractor, as the video can have different resolution than there is in the extractor. Even the mimetype can be different, although the mimetype that the extractor's MediaFormat have (the container mimetype) is more for general application, like intents.

And we can already see the effects of this in the quality selector.

This is solved by using DASH sources

I think it's not a choice here because we pretend to support more services, and we don't really want that restriction.

Do you think local playlist should be reserved as a separate PR?

Yes, there it's too much changes already here and in other pull requests too.

---

And sorry for the delay and this superficial review, I'll try to review and merge it later on.

[karyogamy]

Please review, but don't merge yet. I want to get some more documentations in first.

Also, we might be able to map the tracks in track selector with the metadata from audiostreams, since track selector seems to keep the order of how MergingMediaSource is built.

[karyogamy]

I have just added a rudimentary UI for background player, which can be accessed by clicking on the background player notification.
Currently, this UI is still in rough shape. I will start improving it (i.e. adding variable speed and pitch controls, quality selection, list manipulations, thumbnail, descriptions and better adapter update) after my exam period is over.

Please feel free to test and provide any suggestions.

@mauriciocolli
I've made some changes to the quality menu building so that the track selections now match the sorted streams. However, the problem with frame rates still persist as the track selector can't differentiate between tracks with the same resolution but different frame rates. How do you think abou this? Can we just do what Youtube does and not provide the 30fps version if the 60fps exists? (We could roll our own track selector, but there is no good way of passing the container information into the mp4 extraction process, other than rolling our own extractor, AFAIK)

[theScrabi]

In case someone is wondering how that new background player UI looks like:

[karyogamy]

Play queue is now feature complete. Feel free to review and test the code as there will be no more change except bugfixes.

Here are some screenshots:

New hidden feature (add to current queue): you can go into a detail page and long press background or popup to add to their respective queue (if they are playing).

Please reply below if you have any suggestions or bug reports.

[theScrabi]

Nice one tho don't make it a hidden feature. We need an ui element which clearly depicts this option.

[karyogamy]

1. Fixed. I've also taken the liberty to add a bullet point to the front of the selected item, similar to the Youtube app.

2. Fixed, at least according to my own taste =D Though please take a look at the light-themed version of this queue and tell me if there is too much white.

3. Fixed the flashing and tested on my other phone running sdk 16. For further improvement on queueing, though, we should have a discussion later to find a UI that provides good UX for this function.

[theScrabi]

All right I'm convinced :)
Nnnice job.

[theScrabi]

Forgot to squash again. Sorry. -.-

[TobiGr]

I am a little late on this, but clicking on the notification causes a crash like this. I tested it on Android 5.0.1 and 6.0:

java.lang.RuntimeException: Error receiving broadcast Intent { act=org.schabi.newpipe.player.BackgroundPlayer.OPEN_CONTROLS flg=0x10000010 bnds=[12,114][528,306] } in org.schabi.newpipe.player.BasePlayer$1@206e3ba
	at android.app.LoadedApk$ReceiverDispatcher$Args.run(LoadedApk.java:891)
	at android.os.Handler.handleCallback(Handler.java:746)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:148)
	at android.app.ActivityThread.main(ActivityThread.java:5443)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:728)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:618)
Caused by: android.util.AndroidRuntimeException: Calling startActivity() from outside of an Activity  context requires the FLAG_ACTIVITY_NEW_TASK flag. Is this really what you want?
	at android.app.ContextImpl.startActivity(ContextImpl.java:672)
	at android.app.ContextImpl.startActivity(ContextImpl.java:659)
	at android.content.ContextWrapper.startActivity(ContextWrapper.java:331)
	at org.schabi.newpipe.player.BackgroundPlayer.openControl(BackgroundPlayer.java:139)
	at org.schabi.newpipe.player.BackgroundPlayer$BasePlayerImpl.onBroadcastReceived(BackgroundPlayer.java:472)
	at org.schabi.newpipe.player.BasePlayer$1.onReceive(BasePlayer.java:157)
	at android.app.LoadedApk$ReceiverDispatcher$Args.run(LoadedApk.java:881)
	... 7 more
	android.util.AndroidRuntimeException: Calling startActivity() from outside of an Activity  context requires the FLAG_ACTIVITY_NEW_TASK flag. Is this really what you want?
	at android.app.ContextImpl.startActivity(ContextImpl.java:672)
	at android.app.ContextImpl.startActivity(ContextImpl.java:659)
	at android.content.ContextWrapper.startActivity(ContextWrapper.java:331)
	at org.schabi.newpipe.player.BackgroundPlayer.openControl(BackgroundPlayer.java:139)
	at org.schabi.newpipe.player.BackgroundPlayer$BasePlayerImpl.onBroadcastReceived(BackgroundPlayer.java:472)
	at org.schabi.newpipe.player.BasePlayer$1.onReceive(BasePlayer.java:157)
	at android.app.LoadedApk$ReceiverDispatcher$Args.run(LoadedApk.java:881)
	at android.os.Handler.handleCallback(Handler.java:746)
	at android.os.Handler.dispatchMessage(Handler.java:95)
	at android.os.Looper.loop(Looper.java:148)
	at android.app.ActivityThread.main(ActivityThread.java:5443)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:728)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:618)

[theScrabi]

Confirmed. Happens to android 6.0 as well.
I'm confused, this shouldn't even work on andorid 7.1
... Would be the first time I/we found a bug in the andorid implementation :P