[Merged] Power Saver feature

Hex

@Zigurana There is not much difference between Boolean and list. That much complexity is fine with me. List allows us to add further options later if necessary. Full allows PS for people who dont care about Description. I use Full all the time. My theme does not even have Description. Why should I have to be restricted to using Minimal Mode?

Remember all video stuff works well with PS. Omx does not even need PS to be disabled. So it makes sense. I would like to default PS to Minimal and allow users to decide if they want more power savings or not. It doesnt matter how much Mins you can squeeze out of this. I am against not having option to not care for description. Just as many users read description, many users dont. I dont want to punish them for that.

pjft

@hex said in Testers needed :: Power Saver features :: PR #172:

My theme does not even have Description.

Should that not be optimizable then in itself, regardless of PS being minimal or max? Same for empty descriptions, or descriptions that fit the container?

Hex

@pjft Well PS trigger is timer based so even if description is not present PS will wait full 12 secs after last event for (Missing) description to scroll. Currently PS and Scrolling is sped up for testing in my branch. Check out the stock for how long the scrolling takes.

If i have to code PS to factor in so many things as Video playing or not, Screensaver working or not, description scrollin or not, It will become too complex to code, manage and make changes to.

Description is scrollable text view and has fixed timeouts for start scrolling and repeat from start. It has no way of knowing how much time it actually scrolls. That depends on the size of text. So if description is too long then scrolling is also time consuming. 10 secs is lower bound to this.

TMNTturtlguy

All of this development is great, and I think the power saving feature is a huge benefit to some, but all of this development including the theme changes/default views, the power saving and others are really complicating a system that used to be very user friendly and easy to use. My opinion is we should keep the power saver options to in or off, or we should make it an optional package. Is there a way that the power saver features are all installed and default set to off or minimum since minimum is probably preferred, and the have the menu be an optional intall? This way basic users don't have all these menus and options. I guarantee there are more users of retropie than there are forum members and a majority of users will not read or research how the tools work and these features may lead to confusion and possibly non-working setups. Again, the features are awesome, but for the users sometimes less is more.

Hex

@TMNTturtlguy There are currently many options that wont make sense to general users. I can add a warning for users who change this option with all details about how each option works, just like Video Screensaver does. I dont think there is a way to make it optional package like you say. I know where you are coming from. May users who have made smaller handhelds with 3.5inch screen cannot even read the text of description. By default i can have Ps default to Min mode. I can although see many use cases for ~~Max~~ Full mode too. I have 3 handhelds that I use and heating is a major issue to tackle. That was the origin of PS. Hence I would like to have the ~~Max~~ Full option. Since many want the description to scroll I will support Min and make it default. Basic users dont mess with options they dont understand. If they do they can surely revert it back. Added with warning msg I think that is sufficient.

pjft

If we believe we will iterate and flesh this out into a more "accurate" solution in the long run, I'd rather start with a binary setting and have "off" be the default (which is the current situation), and "on" be whatever power saving setting you're comfortable with at the moment, and improve it over time.

Prefacing this with "I don't know a lot about how it's implemented", my comment earlier, was not as much thinking about adding edge cases, but thinking about PS as a system-wide thing rather than being centrally controlled.

I apologize for my potential over-simplification, but the way I'm understanding what has happened to solve this problem in terms of implementation is that there is a single key point of energy consumption, which is the animation loop.

So far, you've addressed it by making the render loop be interrupt driven, and added a timeout to it. Is that somewhat correct?

I believe that the render loop being interrupt driven is a key development - no pun intended - and that should be the cornerstone of this.

What I was going for was, rather than have a timer - and by consequence different "min", "max" settings corresponding to different - but arbitrary - timeout values, would effectively attempt to address this in the actual rendering pipeline.

The way I understand this is that GuiComponents have an "update" and a "render" method. Also, the only elements that are animated at the moment are ScrollableContainers, the TextListComponent (not sure if by virtue of the previous one, or in itself) and VideoComponent.

We could consider changing "render" to return a bool value, which by default will be "false" if the element does not need an animation.

And for those 3, you'd just need to update the "render" method to return "true" if being animated or not, and "false" if doesn't need to be animated (meaning:

content of ScrollableContainer needs to scroll;
TextListComponent's currently selected element needs to scroll;
VideoComponent is using VLC.

That way, the cycle would be (pseudo-code):

start:
    bool needsAnimation = render();
    while (needsAnimation)
        // this will return false if all of the rendered components return false;
        needsAnimation = render();

    wait(input);
    updateGuiComponents();
    go to start;

Once again, blatant simplification, and terrible pseudo code.

But if our intention would/could be to head in that direction, then I don't see why we will ever need more than two options - with the ending up being your current PS min, but without the current effects on the user experience. And as such, we can launch for now with just "Off" and "On".

Or maybe it wouldn't even need to be an option in the future - it'd just be optimized by default. :)

My 2 - very likely wrong and overly simplistic - cents. :)

Hex

@pjft What you have mentioned is actually my long term plan. It will take a long time to refactor and test everything. I am interested in implementing this asap while what you mention will be taken up later. I do not have an idea of how rendering currently works so I will have to get familiar with that first.

Another problem with your idea is that SDL has to wait for events, so if we dont render when render is not needed we end up with a busy wait loop which is even worse for power.

Currently stock code looks like this

While(Event.get())
   if Event != null 
        Handle event
   Render()

What you are proposing would work like this (over simplifying)

While(Event.get())
   if Event != null 
        Handle event
   if (Render.needed())
       Render()

Problem would be if both conditions are false it will just be a while loop, which is bad. Having an empty while loop puts the cpu in overdrive.

Another problem with your psudocode is events will not be triggered if rendering is running. ie when you press right on carousel it takes 1 secs to move one system and no event will be consumed till that animation is over. Event.get() needs to be called as fast as possible even while animating.

It is very complicated to redesign rendering to be optimized for PS. And that is time i dont have presently. Hence baby steps.

pjft

@hex great! Then start with just on and off. There will be no need to complicate things if you head in that direction.

As for the code, as I mention, it was pseudo code.

The way to handle the loop when both are false is to sleep until (time - time at start of the loop) is larger than whatever max frame rate we want to enforce. That way we ensure we only run the loop, if both are false, a max of 60 times per second, assuming we want to keep it at 60 fps max. It's a standard frame rate loop controller. Hopefully that's not terrible in terms of performance.

But great to hear that that is where it's headed. Take your time and launch something now. Then iterate.

Hex

@pjft said in Testers needed :: Power Saver features :: PR #172:

@hex great! Then start with just on and off. There will be no need to complicate things if you head in that direction.

Its not that complicated to implement. I shall be submitting final updates soon. Was just interested in checking if others had any better suggestions to the names: None/Min/Full

I know that keeping defaulted to Min will be sufficient for majority of users. Optionally None is also worth using if you are connected to wall socket and have good cooling. Full is for portable setups and there is a whole community for it : sudomod.com

As for the code, as I mention, it was pseudo code.

Yes I know. Your psudo code had nested while loops which wouldnt work and hence i pointed it out. Nothing more than that.

TMNTturtlguy

@hex I have another thought which might not be popular, but I am not sure that these options need to be available in the main build. I don't know if a third ES build is worth the effort to keep updated but it seems to me that havin a RPI1 a RPI2/3 and a RPI-handheld might make sense. This build could be created efficiently to work for handhelds and you could maximize it to the fullest PS settings.

Hex

@TMNTturtlguy I wouldnt vouch for code divergence.

jdrassa

@hex Please correct me if I am wrong, but is the only difference between Minimal, Full, and MAX the time delay between switching from SDL_PollEvent to SDL_WaitEventTimeout?

If so, I have a suggestion.

PowerSaver mode is always on. There are no menu options to enable or configure it.
The default delay before enabling PowerSaver mode is the users configured Screen Saver delay.
If a user wants to change the delay to achieve more power savings, they can modify es_settings.cfg directly to specify a different value. This process can be documented on the RetroPie wiki. This is how the hide splashscreen option works.

I think this approach will give the most benefit to all users without changing functionality, while still giving users who want more power savings the ability to do so.

Hex

@jdrassa said in Testers needed :: Power Saver features :: PR #172:

@hex Please correct me if I am wrong, but is the only difference between Minimal, Full, and MAX the time delay between switching from SDL_PollEvent to SDL_WaitEventTimeout?

@jdrassa Very simply, yes. Although there is an enforcer in addition to the counter. Enforcer ensures counter is overridden when PS is disabled and Counter ensures trailing animations after last event. Both are equally important.

If so, I have a suggestion.

PowerSaver mode is always on. There are no menu options to enable or configure it.

Auto scroll doesnt work in that case

The default delay before enabling PowerSaver mode is the users configured Screen Saver delay.

Screensaver delay is at minimum 1m so switching timeout goes from 12 secs to 1m which is not good. Also if video screensaver is enabled PS will never be triggered (generally)

If a user wants to change the delay to achieve more power savings, they can modify es_settings.cfg directly to specify a different value. This process can be documented on the RetroPie wiki. This is how the hide splashscreen option works.

The user base inerested in PS will be more than hide splashscreen guys. Not many users look at the documentation for latest features.

cyperghost

@hex Do you think it would be better to show no PS feature in main menu... so it's disabled by default and all users got a working ES as it looks like now. But create a bash-file within RetroPie menu. There you can choose between PS settings and can tell all advantages and disadvantages of each setting and ES reads out a value in a cfg file stored as you want?

I'm very intereted in the PS feature your know and I personally would enable it by default. Because I have no scrolling filenames, I don't use video preview....
I would also activate it in my Pi0 portable :)

Hex

@cyperghost I dont know if that is possible. Are there any Es settings currently in Retropie-setup?

cyperghost

@hex Well you load Themes for ex. within RetroPie-Menu and activcate them through ES
So yes items for ES are changed through RP-Menu

and yes .sh files can be located to ~/RetroPie/retropiemenu/ and they are visible and executable by default. So why not using this way? You can even create an icon for your set and descripe it in gamelist.xml - very nifty possibilty to create a settings menu :)

Since we know How to: Restart ES via bash script an activation/change of values of ES is also possible :)

So you may create a file names advanced_settings.xml (is there a good place to locate??) and can descripe option there and it could be usefull for future tweaks. For changing file advanced users can do edit manually and/or you write a small bash-script that also explains every option and does a restart automatically after settings were done.

Here you can also find answer of what is ment by EUREKA

Hex

@cyperghost Themes are loaded via Retropie menu but selected using ES alone. I dont think it is a viable solution.

cyperghost

@hex If ES is executed it checks presence/values of advanced_settings.xml one time. Is there is a tag like

<!--
      Hello,
         I am a multi-line XML comment
         I want to tell you a new feature of ES, the powersaver....
        0 = disabled
        1 = min, value Video support works, Scrolling not....
        9 = may the force be with you
               -->

<powersaving>
  <mode>0</mode>
  <timeout>90</timeout>
</powersaving>

or file is missing then PS is disabled.
via batch script and xmlstartlet you can change XML values easily and then force a restart. I think it's a possible solution and you don't need to mess around with explanations pop ups within ES but of course ES must be prepared for the value you've setted in PS-mode.

Of course it's your decision but it will also help to keep ES menu slim but it's still easy configurable by scripting menus.

Hex

@cyperghost I do not want to complicate this further. It has already been decided to have PS enabled by default and a timeout set such that description scrolls. I want to have a setting for people not using/caring about descriptions, hence the Full option. I might think about MAX:EXP option for instant and no carousel users (Might not be implemented). Timeout access shall not be available to user as wrong setting will render ES unusable.

So None/Minimal/Full are three options thought of being implemented for certain. I want your input on changing the names of these modes if necessary. I had name changing suggestions at a later stage while submitting additional animations. I dont want to repeat that. Hence if you have better suggestions for Mode Names, let me know.

Zigurana

@hex
Good idea's all around, even better to come to a decision 😀.
Question: For users, there is no difference between none and minimal, right? Why do we need both?

I would go for default / Improved