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

add a screensaver overlay to ui for use with connected displays #140

Merged

Conversation

tysonnorris
Copy link

On connected display, displays an animated html overlay after a configurable timeout.

}
let timer = null;
//current localbrowser user agent is 'Mozilla/5.0 (X11; Linux armv7l) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/13.0 Safari/605.1.15'
let screensaverEnabled = navigator.userAgent.indexOf('(X11; Linux armv7l)') >= 0 && navigator.userAgent.indexOf(' (KHTML, like Gecko) Version/13.0 Safari') >= 0;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The userAgent could be customized to something very specific on hifiberry-os e.g. "hifiberry-localbrowser", but I'm not sure if there is any other current use of userAgent that would break.


</div>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the HTML overlay that shows when the timeout fires. It is a parallax black and white starfield animated from the css in Beocreate2/beo-extensions/ui-settings/ui-settings.css. I will clean up the id/class names to be more pertinent.

@tuomashamalainen
Copy link
Collaborator

Cool stuff, I'll try this out before merging. The important thing is to not have this when the UI is running in any other context, which is something you've also given some thought.

I also think there might be opportunities to bring in an "ambient display" aspect, such as track info at song change.

<div class="screensaver-timeout-slider black show-value"></div>
<div class="symbol" style="-webkit-mask-image: url(extensions/ui-settings/symbols-black/clock.svg); mask-image: url(extensions/ui-settings/symbols-black/clock.svg);"></div>
</div>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this slider should be hidden when the "Use Connected Display" is hidden, but for now it is.

@tuomashamalainen
Copy link
Collaborator

I've reviewed the changes the best I could and got a few suggestions on improvements:

  • For setting the delay, I would use a standard "label + value" menu item instead of a slider (cycle options via a tap similar to a how a high/low-pass filter type is selected in Parametric Equaliser, or alternatively by presenting a menu like in Bluetooth options). I don't believe such fine-grained control over the delay is required – I would instead offer a few sensible options: Never, 1 min, 5 min, 10 min, 15 min for example. This would clean up the menu as follows (mockup):

Screenshot 2021-03-02 at 16 00 01

  • It looks like the mechanism for exiting the screen saver is to listen to several Beobus channels on the server and then sending a stop command to the client. I would handle this primarily client side – moving the mouse over or clicking on the screen saver element could be used to stop it – because the screen saver shouldn't necessarily go away every time there's an update on the channels you listen to (for example, a song change) and you may want to override it at other times. I do think it's a good idea to keep a mechanism to stop it from the server as an alternative, so don't get rid of that pathway.
  • Finally, the client-side screen saver code should be nested inside the "ui_settings" self-invoking function to keep everything scoped.
  • This is extra and more of a personal preference, but I'm not sold on the starfield as something that's "on-brand". There are a few unused animated SVG blobs under /beo-extensions/setup which could well work in a screen saver (maybe placing a few of them onto the screen in random places, letting them pulse a few times, fade out, place elsewhere, fade in, repeat, not all at the same time). But this is something I could cook up myself too, so don't get hung up on it.

Overall I'm grateful that you took the initiative with this idea and I'm certain we can fit it in just nicely with some tweaks 😊

Best,
Tuomas from Bang & Olufsen

@tysonnorris
Copy link
Author

tysonnorris commented Mar 2, 2021

Thanks for the feedback.

  • I will update the UI - I don't have DSP on my card, but I see what you mean in the bluetooth "Open for pairing" options dialog, I can make it similar to that.
  • I originally wanted to see artwork changes when new song comes on, even if the screensaver is active and then was thinking that any change to the system state would be useful to reset the screensaver. What do you think about a separate timer for showing song change while the screensaver is active? It would be like: screensaver timeout -> show screensaver - > song change -> show artwork for "song change display timer" amount of time -> back to showing screensaver. With a screen in this case being used as both a reference (now-playing artwork/title) and a user interface (control the system), I think it is hard to apply traditional screensaver behavior like "remove the screensaver only when I want to provide input", if that makes sense.
    EDIT - oh maybe in your previous comment you are getting at embedding the now-playing info into the screensaver display instead of stopping the screensaver completely, which seems like a good idea.
  • Separately on the events, I was trying to get just the now-playing events to cause showing of artwork (by resetting screensaver for now), but using spotify, it seemed like it receives a combination of now-playing and sources events. I can look at this more, there may be some separate issue with spotify+now-playing events, or I may have just done it wrong.
  • I can reorganize the code to be in the ui_settings function.
  • You are referring to this animation right? I can work out something based on that. I think it would also be appealing to eventually have some other options or even configurable settings (colors, speed, etc) to have the animation and style be more personalized - is this something that would be considered? I know it is the opposite of consistent branding, but for something that is ambient background display options, maybe that would be ok.
    image

Will follow up with these changes as I have time, it may be a couple days.

@tuomashamalainen
Copy link
Collaborator

tuomashamalainen commented Mar 2, 2021

Thanks for the feedback.

  • I will update the UI - I don't have DSP on my card, but I see what you mean in the bluetooth "Open for pairing" options dialog, I can make it similar to that.

Cheers. Let me know if you require assistance with it.

  • I originally wanted to see artwork changes when new song comes on, even if the screensaver is active and then was thinking that any change to the system state would be useful to reset the screensaver. What do you think about a separate timer for showing song change while the screensaver is active? It would be like: screensaver timeout -> show screensaver - > song change -> show artwork for "song change display timer" amount of time -> back to showing screensaver. With a screen in this case being used as both a reference (now-playing artwork/title) and a user interface (control the system), I think it is hard to apply traditional screensaver behavior like "remove the screensaver only when I want to provide input", if that makes sense. * Separately on the events, I was trying to get just the now-playing events to cause showing of artwork (by resetting screensaver for now), but using spotify, it seemed like it receives a combination of now-playing and sources events. I can look at this more, there may be some separate issue with spotify+now-playing events, or I may have just done it wrong.

This is kind of what I was referring to with "ambient display", maybe we bring this information directly into the screen saver such that it fades in for a brief period of time. We could also bring colours from the album cover into the screen saver in some fashion. Now Playing is implemented using Vue and it's trivial to add another instance that hooks this data up with the screen saver. I can give this a shot once we get the basics up and running.

  • You are referring to this animation right? I can work out something based on that. I think it would also be appealing to eventually have some other options or even configurable settings (colors, speed, etc) to have the animation and style be more personalized - is this something that would be considered? I know it is the opposite of consistent branding, but for something that is ambient background display options, maybe that would be ok.

Yup that's exactly it. I think customisable options could be something to think about in the future but for now we should aim for one simple solution and see how people are finding it.

Will follow up with these changes as I have time, it may be a couple days.

Cheers, take all the time you need.

@tuomashamalainen
Copy link
Collaborator

Very nice work with the improvements.

Currently the mini-now-playing within the screensaver does not function, from what I can see, but that's because there is no "global" Vue instance. Right now we're just "flirting" with Vue in a bunch of places (Now Playing and Music are written with it) so the whole UI is not yet contained in it. Moving the whole UI onto Vue is something I would like to do at some point, but that time is not now.

For now we could just add a Vue instance in the UI settings client code, outside the self-invoking function, and connect it to the Now Playing data:

var screenSaverInfo = new Vue({
	el: "#screen-saver-element", // Element that houses the <mini-now-playing> component.
	data: nowPlayingData // Plugs in to the existing Now Playing data.
});

This is a quick-and-dirty way to do it, there's a lot of extra markup in the component so for the long term there are better ways to go about it, I can also pitch in on that later.

@tysonnorris
Copy link
Author

Ah right, sorry I had added this to the now-playing-client.js to get it working and forgot to move it.
I wondered about vue usage - I understand the state of it, thanks for the background.

@tysonnorris
Copy link
Author

After replacing the starfield with the animated dots, it works fine, except that on my DSI display, the svg animations stop after some time. I don't think this is critical, but would be interested to know if the same happens to others.

Here is a video of my laptop browser next to my pi display after running for some time - the css transitions/javascript is still working on both, but the animations in the svg only play on my laptop:
https://user-images.githubusercontent.com/1638396/110415086-1860df80-8046-11eb-88dc-456c9cb76d67.mov

Restarting cog service the animations in svg come back for a while, not sure exactly how long - maybe 10-15 minutes. I don't see anything fishy in top:

  PID USER      PR  NI    VIRT    RES  %CPU  %MEM     TIME+ S COMMAND                                                                                                                                                                                        
12893 root      20   0  153.1m  23.6m   0.0   2.6   2:08.50 S /usr/bin/cog 

@tuomashamalainen
Copy link
Collaborator

Cool to see the dots make an appearance. I implemented some tweaks based on these latest changes:

  • The dots gradually come and go one instead of all at once. More tranquil and organic, IMHO.
  • Changed to a more classy fade in/out transition for the screensaver instead of a slide in (functional reasons for the previous choice?).
  • Wrote simplified Now Playing markup within the screensaver instead of using mini-now-playing, visibility controlled via a new notifyLong flag in nowPlayingData.
  • Currently selected option is shown in the screensaver menu item.
  • Changed default delay to 5 min.

Screenshot 2021-03-09 at 16 57 36

I don't have suggestions on the DSI display issue, unfortunately. The SVG animations seem to have a mind of their own even on my Mac, not always triggering, but this might be some WebKit power saving feature.

Unless I'm mistaken (which could be, Git is like black magic to me at times...), I can't add to this PR because you own the fork this comes from, so I could merge now and add my changes afterwards – better ideas?

@tysonnorris
Copy link
Author

Thanks for the help @tuomashamalainen. This all sounds good.
You should be able to push to the PR branch in my fork because I checked the github PR box to "Allow edits by maintainers"
image

To test out changes on desktop (mac if it matters), is there a simple way to run the beocreate2 nodejs app on desktop, without moving around directories like beocreate_essentials? e.g. running node ./beo-system/beo-server.js I get Error: Cannot find module '../beocreate_essentials/communication' etc.

@tuomashamalainen
Copy link
Collaborator

tuomashamalainen commented Mar 9, 2021

Thanks, must be a user error then. I'll try again.
Edit: It might be that I'm experiencing this issue, because the create repo is under an organisation: isaacs/github#1681

Currently the system has not been designed to run outside Raspberry Pi / HiFiBerryOS environment so it's very angry about a lot of stuff if it's not where expected. I did test this a while back and these instructions might still work: #66 (comment)

It's not very simple but if you really want it, it can be done. The GUI itself, as you probably know, doesn't really care where it's displayed – the server is the culprit here.

Personally when I work, I have set up my editor to automatically mirror saved files to the Raspberry Pi so that it doesn't matter much where the actual server runs when testing.

@tysonnorris
Copy link
Author

I haven't seen the issue with allowing edits by maintainers - but I can't remember the last time I used it. I added you as a collaborator on my fork, or if you prefer I can manually shuttle changes from your branch, etc. Whatever is easier for you.

- Fade in and out
- Rotate dots on the screen one by one
- Better now playing data
- Default delay is 5 minutes
- Show currently selected option in the menu
@tuomashamalainen
Copy link
Collaborator

Thanks, after accepting your invitation I managed to push my changes just fine. Give them a go when you've got time, and after that I'll fold this in if you don't have more things to add/change for now.

@tysonnorris
Copy link
Author

Looks good to me - thanks for your help!

@tuomashamalainen
Copy link
Collaborator

No worries, thanks for taking the lead on this. It's a cool initiative that has future potential.

I'll merge this in now.

@tuomashamalainen tuomashamalainen merged commit 80b0968 into bang-olufsen:development Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants