Help testing: Random Video Screensaver on Main Branch

pjft

@TMNTturtlguy sounds good, I'll check. I must have messed something up then.

pjft

@TMNTturtlguy hm.

The code seems to be there though:

https://github.com/pjft/EmulationStation/blob/RetroPie-ScreenSaver-Merge/es-app/src/EmulationStation.h

It's 2.2.1 that you should be seeing.

Could you open that file locally on the Pi and check whether the content is the same?

TMNTturtlguy

@pjft dumb question - but i am not sure where to find that! Your branch is currently in /opt/retropie/supplementary/es_pjft_retropie-screensaver-merge/ on the pi. I don't have a file named EmulationStation.h there.

pjft

Not a dumb question at all. I suppose @meleu would know where the code is compiled to. That's where you'd want to look into.

I've just done a final push to the PR, updating a few things that had gone missing. All seems to be good to go on my end, but more testing is always encouraged and appreciated.

@Hex : the Alsa test works fine, and actually when you change it and exit the options menu the video player is restarted immediately with the new audio option, so no worries about that. Do test it, but it seems to work well.

I do get an error on my end - I believe when set to alsa:hw:1,0 for "ALSA lib pcm_hw.c:1667:(_snd_pcm_hw_open) Invalid value for card".

Unsure if it's because it's not a valid option or if it's because I don't have an external card, but thought you'd like to know.

@TMNTturtlguy one thing you may want to do is to just do it old school style.

https://retropie.org.uk/forum/post/82820

I didn't test those instructions but they should work.

TMNTturtlguy

@pjft said in Help testing: Random Video Screensaver on Main Branch:

@TMNTturtlguy one thing you may want to do is to just do it old school style.
https://retropie.org.uk/forum/post/82820

Yeah, i have to run for a few hours, but i am going to install the main branch es and then install yours, if that doesn't work, i will install it the old way! Thanks

Edit, i am going to try the remove/remove source files from @meleu script and then try to reinstall.

meleu

@TMNTturtlguy the script saves sources at ~/src.

Edit: typo

Hex

@pjft said in Help testing: Random Video Screensaver on Main Branch:

Not a dumb question at all. I suppose @meleu would know where the code is compiled to. That's where you'd want to look into.

I've just done a final push to the PR, updating a few things that had gone missing. All seems to be good to go on my end, but more testing is always encouraged and appreciated.

@Hex : the Alsa test works fine, and actually when you change it and exit the options menu the video player is restarted immediately with the new audio option, so no worries about that. Do test it, but it seems to work well.

I do get an error on my end - I believe when set to alsa:hw:1,0 for "ALSA lib pcm_hw.c:1667:(_snd_pcm_hw_open) Invalid value for card".

Unsure if it's because it's not a valid option or if it's because I don't have an external card, but thought you'd like to know.

@TMNTturtlguy one thing you may want to do is to just do it old school style.

https://retropie.org.uk/forum/post/82820

I didn't test those instructions but they should work.

@pjft Yes if you dont have a second USB cad then it errors out. Its not harmful.

For those who are testing you can manually test it with a keyboard. you dont need the script or anything else

$ git clone URL DIR_NAME
$ cd DIR_NAME
$ mkdir build && cd build
$ cmake ..
$ make -j 2

"-j 2" if using Pi 2/3, omit if pi 1/zero
For pi 1/Zero you have less RAM than necessary to compile, so you need the script.

To run the executable
$ ../emulationstation
This allows you to test without installing and disturbing stock ES

To update build :
$ cd DIR_NAME/build
// Make sure you are on the correct branch
git branch
// Get updates
git pull
// Make same as above
make -j 2

TMNTturtlguy

@Hex Thanks for the info - @meleu script is so fun though!
@pjft after uninstalling and reinstalling i have your current build. Works well, thoughts below.
@meleu I think something broke in the script after the last update? It was working before updating the builds, now you must uninstall the build and reinstall it to get the newest updates.

@pjft - thoughts on the build

The screen saver works really well, as expected
I like the text options, work well.
sounds settings, OMX Player Audio Device - there are a ton of options, most users wont have any idea what all those mean the way they are presented. I think a lot of people will mess with them and the forum will be flooded with complaints and questions.
Video Options - on the 4:3 menu both OMX player options are identical because they get cut of now. I don't know which one i am changing. They both say: USE OMX VIDEO PLAYER (FAST) FOR...
I think we need to shorten them, OMX VIDEO (FAST) PREVIEW and OMX VIDEO (FAST) SCRNSVR

Hex

@pjft I think Video settings should be shifted to Other Settings. Video player for previews and screensaver can be same always so a single setting should work.

pjft

@TMNTturtlguy thanks, good catch on the option length on 4:3! Could you update and check again? I removed "use" and "video". Let's see if it helps!

As for the audio options, they're already on the main branch, @Hex put them together as he has an external sound card so he needed a few options there.

Are you referring to the OMX ones in particular? I think it's a good question and good feedback in general. Instead of having them be literally the option text that's passed to OMX Player it could be something more user friendly, like

HDMI -> HDMI
Alsa:hw:0,0 -> External sound card 1
Alsa:hw:1,0 -> External sound card 2
Local -> headphone jack
Both -> Default

Is something like this (assuming that I have them all correct) helpful? We could then do the option to parameters conversion via code.

Thoughts?

TMNTturtlguy

@pjft I have the girls now for a few hours until they go to bed! Will test in a few hours and let you know, you will probably be getting rest by then!

Yes i was talking about the OMX audio options in particular. I think @hex were fine additions. I think your new suggestion is much better! External Sound Card means something to me! Alsa:hw:0,0 means nothing! I think your list is good if it reads like I think your intentions are, i would order them like this if it was my setup:

Default
HDMI Only
Headphone Only
External Sound Card 1
External Sound Card 2

Does this make sense?

pjft

@Hex The entire video settings options going into the other settings menu?

On the meeting the two video options, I'd rather not. VLC doesn't perform well on the Pi, but there's unfortunately a use case for having it in the gamelist view, which is if you're using a theme with overlays on top of the video. OMX Player doesn't support that. However, it ends up overheating the system after a while, so having it as a screensaver player is far from ideal, unless you have a fan.

If you confirm that the screensaver is running on your pi and the HDMI signal isn't failing, I'll remove the VLC screensaver option altogether. I had just added it as in the previous posts we were considering the possibility that OMX Player would in some configurations not be a valid option because of that, so leaving VLC as a fail safe option was my attempt at that.

I'd love to remove it, though, so if you confirm that things are running for you on the Pi without the game info being displayed, that's good enough for me to remove it! Thanks.

pjft

@TMNTturtlguy I think so, if they are accurate representations of what they mean - I was mostly making informed guesses :)

@Hex - over to you.

Hex

@pjft So I compiled on pi3 and hdmi fails if subtitles are enabled with omxplayer . screensaver with vlc is damn slow.

"""The entire video settings options going into the other settings menu?"""
In my opinion it would be helpful

Regarding the audio options it might be possible to rename them that way but i am directly extracting the preference and sending it to omxplayer so if you rename it omxplayer will receive it as

omxplayer -o "External Sound Card 1"

which will not work at all. I will need to add a lot of code to translate option to parameter. I would rather have it the current way. I agree it is a bit vague but when you go looking for "Splash screen no sound usb" you will become familiar with these parameters. Else you would just need the default one , both.

I can change it if you guys think so. It will not take much time.

TMNTturtlguy

@Hex can you expand on hdmi fails with omx player with titles on? Does the audio, the video or both fail? I am running on pi3 with hdmi to 4:3 monitor and 3.5 jack and it is working. I had 1 instance where I got a black screensaver, after changing options in the menu but then could not reproduce it, it ran correctly every time after that. As for audio settings, as a non computer non coder background the asla or whatever naming means nothing to me. The options are very confusing. Also having these options in this menu and then audio options in the retropie menu will be very confusing as neither place tells me what the setting is doing. We will have users forcing audio in the boot configs and users forcing hdmi or 3.5m in the retropie menu and now we are adding even more options forcing audio from this menu. What happens when i go to retropie and force 3.5 and then go to the start menu and force hdmi?

Not trying to be difficult here, I am just approaching this from the common user trying to build a simple system with no knowledge of the underpinnings.

Hex

@TMNTturtlguy you are right, I will set it up as something simpler later. Since the binary 2.2 is already out there is not much hurry to fix it. I shall submit a PR soon with fixes

Hex

@TMNTturtlguy Regarding the HDMI I have overscan enabled so I am not the right person to work this with. In my opinion asking users to disable overscan and have black stripes just for video screensavers is not done. Disabling the subtitles sounds as a reasonable request to enable this feature

TMNTturtlguy

@Hex @pjft let me install the newest update on both of my builds. I have been testing on my 4:3 today as that is the current build I am working on with theme updates. I will update that to check the menu text. I will then test on my 16:9 with overscan enabled to remove the black bars as @Hex has in his setup. Just as an FYI, my previous testing the last few days have all been on 16:9 and it worked properly. That was prior to all of the options on how to display the text.

meleu

@TMNTturtlguy

@meleu I think something broke in the script after the last update?

Thanks for pointing it out. Actually the script wasn't broken, just a little dumb. :-)

The last changes @pjft made in his repo had conflicts with the previous version and when the script was trying to git pull, it was failing and, with any checking, going ahead with compilation.

I've just fixed and added a checking after the git pull, if it fails (probably due to conflicts) it rm -rf the local repo and then git clone it again (if it fails again, probably it's a connection problem).

Update your script and you'll get the delta version with this fix.

Cheers!

Hex

@meleu rather than rm -rf we can do the following:

git merge --abort && git pull -X theirs

This will preserve the build dir rather that making a clean compile thus saving compile time. All changed files will be recompiled by make. This is a much better solution on failure of git pull

To combine all of theme in one sentence you can have the command as such :
git pull || git merge --abort && git pull -X theirs

The || ensures that git merge --abort && git pull -X theirs only runs if git pull fails
The && ensures that git pull -X theirs only runs if git merge --abort succeeds

The final status will give you a success or fatal error status so its a win.