-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[feature request] Add oreo colored media notifications #1660
Comments
Making the non-standard NewPipe background player notification into a standard media player notification would solve this problem and a bunch of others: #1104 #1207 #2540 #1780 #2233 just to list a few. In addition, Android 10 implements a seek bar in the notification itself, which would allow for seeking during playback from the notification. I think this should be a priority feature request/milestone. |
@Stypox I am asking this here cos the issue#2540 was closed. You said:
How do I convert the background queue to a playlist? |
Click on the notification to open the background queue, then click on the "add to playlist" button in the toolbar. @alittlebitofit |
@Stypox Thanks :) |
@Stypox Create a playlist from the background queue would save the audio stream cache for play offline or is it a misunderstanding of me? |
No, offline video caching does not exist in newpipe at the moment. What the button does is just creating a new local playlist containing the current items in the queue. @hardBSDk |
@Stypox Thanks! |
So I looked into this over the weekend and I want to try to implement these new media style notifications, first up I want to clear up a misconception these colored media notifications are possible starting from Android Lollipop so all Android version that we support can use these (except of course the ancient kitkat). I have a question about the scope of this though I hope someone can answer my question:
this is how my current work in progress looks like: So the first notification here is from the vlc media player and then under that is a screenshot app you can ignore that, the one under that are the new colored notifications and below that the current notification. I can make the options of having a repeat and close icon available too when the notification is expanded. (but not when minimised since it would be too much clutter) |
I really like the new notifications, though I am not sure about one thing: since KitKat is not supported, this would imply having to support and mantain two different notification types. An option to revert to the old notification would be a good idea. The optimal thing would be to completely remove the code for the old notification (less code to maintain), but since this can't be done, I suggest enclosing both notifications in a common interface so that they are interchangeable (i.e. there would be no need for |
So I'm still working on this unfortunately I can only work on this on the weekend so my progress is slow. Something I realised recently after testing on Android Nougat, Oreo, Pie and 10 is that the colourization is indeed only possible on Oreo and up I initially assumed that it would be possible on Lollipop and up since Lollipop supports the media style notifications however Android < Oreo will simply ignore the colourization. Given this I will try to make it so that the new notifications will be enabled by default on Oreo and up and on Lollipop and up the current notifications will be the default unless someone enables them in the settings. I have a test build but it's still early in development with some things not working yet, however if someone wants to test it here it is: EDIT: apk got broken by youtube change. I'm going to compile a new version in a few days. Changes in this build:
Temporary Changes in this build:
Known Issues:
|
I'm almost done with this. This weekend I got in all the important features that I wanted in and the notifications now work well on my test devices; the only thing I still need to do is code clean up and I need to add the setting to enable the old notifications. Around next week or the week after I should have my PR ready for this. In the meantime if anyone wants to test the current work in progress here is a test apk: Fixes since v2:
Known Issues: |
@cool-student There is a regression in background player - seeking not works, playback always goes back to the beginning. Screenshot (Oreo 8.0) |
@spvkgn ah yeah sorry about that, I commented out some code that updates the progress, I realised late at night that I commented it out at a place that would make it so that it won't update the backgroundplayeractivity's progress as well which is of course not good. So I'm aware of this issue and I will fix it in the next build. |
I've chased down some bugs and have a new build ready: Changes since v3-1:
Known Issues:
I still need to fix a few small bugs and clean up my code but the PR should come soon. |
@cool-student the thumbnail in the new notification is squished and does not keep the 16:9 ratio. There are two ways to solve this, and each of them implies creating a new drawable based on the original thumbnail:
Another option would be to make the image space bigger, but I don't think this is a good idea since it would reduce space for buttons (and I don't even know if it is possible) |
@Stypox so just for clarification you're not talking about the metadata album art that is shown on lock screen controllers but the image shown in the notifications itself ? @K1rakishou cropped the album art image in #2690 and I undid these changes but it depends on the oem skin how well they look. this is how it looks on my lockscreen controller (and most modern Samsung phones S7 and newer): If it's about the image that is shown in the notifications itself then I have to think about it more, I didn't really consider this honestly and at the moment I just set what I get from the
I don't think this is possible we're not using remote views anymore for the new notifications so we can't do custom stuff like that, buttons are sadly also limited to 3 in compact view and 5 in expanded view otherwise I would have added a shuffle button but sadly it's not possible. edit: maybe it looks different on your phone, can you show me how it looks for you ? |
@cool-student I try to run your code but only get crash on attempting to watch video. The apk works fine. Am I doing something wrong? |
@Stypox to be honest mediastyle notifications don't look so great below Oreo ^^'; @vkhomenk can you check your logcat so I can see what causes the crash, it should have an exception in there. Alternatively you can wait until Monday or Tuesday next week so I can compile a build that has ACRA enabled so that crashes show the crash dialog again. |
@cool-student I mean I get "this was not supposed to happen" screen in the app when attempting to watch video, while apk works fine |
@cool-student Whan you have time, please make a runable release. Thanks |
@vkhomenk Tried reinstalling? It works normally for me. |
@Stypox so I checked what the notification look like on my LG G3 @ Nougat and I compared them to the vlc notifications (they also use mediastyle notifications) turns out they look pretty much the same so we can't improve the image size on the notifications.
about forward and rewind: about the progressbar: @vkhomenk try the new build below. Changes since v4:
|
@cool-student Sure! A toggle to choose between them would be great. My thinking is that with songs and such I generally want to skip tracks. With Youtube videos I generally find myself wanting to rewind. Is it possible to have the setting control which pair of buttons are shown if there are multiple audios in the queue? Because it doesn't make sense to have the skip buttons if only a single audio is playing and the queue is empty. Then it should always show the forward and rewind buttons. No progress bar is not a dealbreaker. My phone is getting LOS 17.1 soon anyway. ;) |
Wow they look nice 😊 And another thing, when the audio is loading, the download symbol is shown, it feels weird because we don't download but only load (technically it's still downloading data, but it's streaming), but I don't know what could be used instead, and if something should be used instead. Also, old notification are being used with popup player (and also with the lockscreen) |
Buggy bebaviour: clicking on the download button blocks the notification (the video stops loading and no button works), and the only way to exit this strange situation is clicking the X button, that resets the notification. |
No no no no no that's not unwanted behaviour at all! When you finish watching a video, you can replay it without downloading anything because it gets cached. The same thing happens with the audio. While the notification stays, the audio stays in the cache, and can be replayed any number of times without incurring additional data costs. This is a feature, not a bug. |
Thank you guys for all the feedback so far.
good catch I'll try to fix it.
your first interpretation was correct the fix is in v6 so I'm confused why it doesn't work for you. Does your device not consider itself to be a part of <= Android N MR1 (API level 25) ? Have you tested it with more than 1 stream ? I could've sworn that it worked on my LG G3 @ Nougat...
as @opusforlife2 explained this is simply the current behaviour I didn't change it. However there is a future request somewhere requesting what you described as an optional feature. I will look at that again when I'm done with this initial PR.
The reason why I added this is that I noticed on one of my old devices the audio would sometimes abruptly stop for a while and I initially didn't know why, so I wanted to indicate the buffering state.
good catch I didn't even know that the popup player used a notification.
I might have phrased this poorly but there is actually a use case for the previous button even if you only have 1 track and that is to restart the track. There was an attempt to add the previous button to the old notifications in that scenario for that reason (#2726) . One more thing that I've been thinking about; I was thinking of removing the close button and replacing it with a deleteintent instead this would enable us to add a shuffle button. The way a delete intent works is that you need to pause the audio playback and then you can swipe away the notification, it's pretty much what the vlc guys are doing and I think we should do this too. What do you think ? |
First, I think the cross button is better, because without it, the user need to know the trick (pause and swipe). And how you will inform him? There are no in-app changelog, no tours… Secondly, you should be able to add the shuffle button even with the cross. Look at spotify, they have like button + controls + cross. |
look at the picture again. there are only 5 buttons. that's also the number we are limited to, we already have |
yeah this is true but vlc is also doing it ... but yeah if people don't like it I'll keep it as is for now and maybe later I'll add a setting for the button layout or something like that |
Thinking further, since there are 5 possible slots, the defaults should be ones which would see the maximum use. I suggest these options:
Since Shuffle and Repeat are queue operations, and not track specific like the 5 above, they could be changed by opening the background player from the notification like normal. I don't recall seeing any app where these options are exposed through the notification, anyway. Edit: Does Android allow long pressing a notification button like you can within an app for a different function than normal? I was thinking it would be neat to have the forward/rewind buttons also function as previous track/next track upon long press. Edit 2: Never mind Edit 1. Any long press anywhere brings up notification options. Google should really add this feature, though. It would make notifications more flexible. |
there are many use cases so it's not easy to decide; for example someone that listens to an eminem playlist would probably want prev and next to skip through tracks that he doesn't like and forward and rewind are useless in this case. (on Android 10 in particular forward and rewind become less important since you can just seek to a position)
being a queue operation doesn't mean much; prev and next are also queue operations but can be quite useful to have depending on the use case. just as a food for thought mx player is using the following buttons for their notifications: edit: slot 0: some explanation: smart just means that we show forward/rewind when there is 1 track and prev/next when there are more, basically what we do now. |
Not for long enough videos. For those, tapping on the seek bar becomes very inaccurate - an annoying exercise in hunting for the correct timestamp - and the buttons become essential.
I knew I was forgetting something important when I wrote the comment! Yes. Previous and next are very essential for multiple items in the queue. Now I think the new defaults should be:
That would be wonderful! :) More user choice is the democratic way of going about things. |
you're right I was thinking about short tracks like music. I think your overall suggestion sounds good so I will use the mx player layout for the next build. There are still a few things that I need to fix so the new "notification slot settings" might not make the next build. As soon as I am done with the code refactoring (only code changes to make the code more readable and better but no functional changes) I will provide the PR. So the next build might just be a bug fix build; tbh I'm a little sad that I'm a bit behind in my personal schedule, I wanted to have the PR up sooner since I might also need to address things from the code review but I just couldn't get it done earlier and I also had some confusing bugs that only happened on Android 10. A list of things I still need to do:
|
Oreo allows media apps to set notification and buttons to match the video thumbnail's accent color,it would be great to have that on newpipe
The text was updated successfully, but these errors were encountered: