Help testing: Random Video Screensaver on Main Branch

[pjft]

Ok, so a final update.

I've just updated the rebased branch with:

• Only two options for VLC screensaver quality (only visible in non-Pi devices): "high" and "fast". Default is "high".
• Added "stretch" option to VLC screensaver as well (so it can keep video proportions). It's "off" by default.
• Option name stays "Random Video" at the moment.

Would appreciate one final round of testing on Linux and the Pi, just to confirm I'm not missing anything.

Thank you!

[meleu]

@pjft I'll try to break test it tonight. ;-)

[Hex]

@pjft same here. Will report back in the evening

[pjft]

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

@pjft I'll try to break test it tonight. ;-)

Why don't you even... -

Thank you both. :)

It's end of day here, so I'll adjust anything missing or messed up tomorrow.

Thanks!

[pjft]

Just updated the branch one final time. Had forgotten one line after merging with the OMX audio changes.

@Hex One small comment: the forced audio init/deinit on every OMX Player instance causes a slight but noticeable delay.

I notice that if I remove the init/deinit code that it works just fine on my end (I'm using "both"), and from my testing it only failed to show any video when using the ALSA:HW:1:0 option, so I'm assuming that it is only necessary if the actual option is this one? Would that be a correct assumption? Or maybe the other ALSA option also needs to be catered for? Or maybe it depends on the ES audio settings as well, and any conflicts between those and OMX ones?

Anyway, my recommendation would probably be to only use the init/deinit instructions explicitly only when we actually need to force it to use the same device, as it causes a slight slowdown when changing/loading videos, but also cuts short the navigation sounds if the theme has them, especially when scrolling quickly (i.e. keeping down/up pressed).

If you tell me that it's only with the ALSA ones (or better, even only with the ALSA:1,0 one) I'm happy to add the conditional statement on my PR, so it only runs that code when it is actually needed.

Alternatively, if it'll be a bit more complicated, you can probably submit a separate PR for that, it's not a big deal but it's one we should perhaps tweak if possible.

Let me know.

Thanks for all the work here as well!

[Hex]

@pjft I will need to test it. Can I let you know later in the day?

[pjft]

@Hex Sure, please. If you want to look into that on your own time as well, and then submit a PR it's perfectly fine - just wanted to mention it in case you would be able to provide an answer off the top of your head, given that you were the one who researched all of that.

It's not urgent nor is it a big deal - in fact, it's right now in the main branch. It's just a small - but noticeable for those who had been using OMX Player - delay in loading the videos and navigating, so while it will be needed for those cases where you do have an external sound card and are competing for the ALSA driver, I'd rather not have it for all the other cases where it's not needed, just that. :)

In fact, we could probably be sneaky about it and just de-init the ES audio completely when:

• the user navigates opens the gamelist, if it is a VideoGameList; and
• if the user is running OMXPlayer and
• the user has the ALSA:HW:1,0 selected (or whatever conflicting options there'll be)

given that, truth be told, if they're on the gamelist and it has videos, it will likely be permanently running OMXPlayer anyway and the audio channel will need to be allocated to it, if it's conflicting.

That would probably save a bit of CPU time in de-initing and re-initing on every single new video being shown.

Obviously you'd need to make sure that you'd re-init the audio when you leave the gamelist (either to the system view, or to a gamelist view that's not a Video one).

Just a thought.

EDIT: as I said, it's end of day for me, so do take your time. I'm in GMT+1 time zone, and tomorrow is a work day.

Thanks!

[meleu]

@pjft My notes about the tests I did this evening (RetroPie on Linux Mint x86).

1. The default option isn't displayed on the menu in the first run (the 2 overlapping triangles look like the star of David :P )

Suggestion, if you're going to provide just 2 options, I suggest to use a boolean ON/OFF option (just like the other options in the VIDEO PLAYER SETTINGS). It could be named SCREENSAVER HIGH RESOLUTION.

2. When running with SCREENSAVER RESOLUTION: FAST and STRETCH VIDEO ON SCREENSAVER [OFF], the video overlaps the theme and part of the theme keeps being displayed at borders:

Also, I noticed that the captions with the name of the game are displayed only for ~5 seconds. The Double Dragon screenshot above has no captions because it was taken after those 5 sec.

3. When running with SCREENSAVER RESOLUTION: FAST option and STRETCH VIDEO ON SCREENSAVER [OFF], the video stretches and again, captions for ~5 seconds.

4. When running with SCREENSAVER RESOLUTION: HIGH there's no difference if STRETCH VIDEO ON SCREENSAVER is ON or OFF and there are no captions at all. IMHO there's no difference between these three modes: RESOLUTION: FAST/STRETCH [ON] and RESOLUTION: HIGH/STRETCH [ON/OFF] (except the captions absence when RESOLUTION: HIGH).

[Hex]

@pjft for the background showing ES you can have a paramter like -b for omx. i dont know for vlc.

You need to edit settings.cpp and set a default for the one with star

[pjft]

@Hex @meleu

Thank you very much for the testing! This is helpful.

The black background is actually my fault - when I added the non-stretch option for VLC and tested it here it works because I do have the black background code running, but it's only being used on the Pi :)

The star in the options is caused because, since I removed most of the options (and renamed them!) you had a now-invalid value in the settings file ("fast" and "high" were new - you probably had "native" or "highest" there). This will not happen on release, as there is effectively a default being set, but only if you don't have a value in your settings file. :) Thanks for catching that though!

As for your missing captions on highest resolution, I do have to say that completely bums me out :( First, then, I will likely have to set the default to "fast" then, and/or rename the options to something more reassuring.

Out of curiosity, though, what is your screen's resolution?

Also, great catch on the "Resolution high" forcing the video to stretch. I will fix that!

Thanks!

[pjft]

@meleu

Thanks for the feedback, and a few more things I forgot to answer.

So I just blind-coded the following:

• I added a ON/OFF option for the captions resolution, and called it something like "USE COMPATIBLE LOW RESOLUTION FOR CAPTIONS". It's set to "ON" by default. People can turn it off but will be aware that, if things don't work, they should revert it back to ON.
• The captions will only render for the first 8 seconds, and the last 4 seconds of the video. This is deliberate so you can have the full video on the screen, but can still know the name of the game at the beginning or the end of the video (like a music video clip from the 80s/90s).
• I also think I fixed the stretch/non-stretch for High resolution (now "OFF" for compatibility in captions). However, as I said, this was a blind coding exercise - the most I got to was confirming it compiled, but I'm not near my Pi. :(

If anyone does have the chance to test it, it'd be great.

Pending questions/answers:

• @meleu : what's your screen resolution? Would like to provide one last shot at fixing the captions at high resolution for you.
• @Hex : if/when you do know the options for ALSA disabling/enabling do let me know if they're just a single condition, and I can add it on my end.

Thanks!

[Hex]

@pjft You can add a conditional statement with string compare to check if it starts with alsa and only then disable and enable it.

[pjft]

@Hex sounds good. Both ALSA options then? And are those the only conflicts to expect? I know you added some more audio options, so just want to make sure that's all that's needed.

Thanks!

[meleu]

@pjft my laptop resolution: 1920x1080.

More notes I would like to make:

• As my video snaps are all low quality, I can't see any difference in video quality between the two resolution options. Does it have difference on your end?

• Maybe SHOW GAME AND SYSTEM NAME ON SCREENSAVER can be simplified to SHOW GAME INFO ON SCREENSAVER.

• I would like to see the game info all the time. I don't think users will wait screensaver to watch the videos as a video clip and wait until it ends... My use case for this feature is: a BBQ in my home and the arcade turned on all the time. When there's no one playing, the screensaver is showing some game's video snaps. When a passerby walks through the arcade machine and enjoys what he's seeing, he'll like to know the name of the game (and probably play).

• Since it's a screensaver, I think the "stretch video" should be on all the time (no black bars on the sides).

Out of curiosity: why don't you create a virtual machine and install Linux?

[Hex]

@pjft AudioManager should be disabled and enabled again only if omxplayer and alsadevice is used. You can put this in your PR

The problem is if the user changes device after video starting then it will not restart the device. I shall take care of this and submit a PR to handle this border case. Please omit this border case handling in your PR. I shall work it out soon.

@meleu I agree on the point that subtitle should be on all the time. There is really no reason to do otherwise. I disagree on the stretch always on. That spoils the aspect of the game expectations. Having it as a user switchable works well as you can do as you want. The goal of screensaver is not to occupy the screen but occupy the interest.

[pjft]

@meleu Darn. 1920x1080 is also the resolution I had tested it on the Pi. No luck, then.

Replying to the notes:

• No, it won't have any difference on the video itself, but it should render the text much sharper. The difference is:
  • One option will render the video in VLC at its resolution (let's say, 320x240) and add captions at 320x240 resolution. The upscaling will then be done on ES, via the texture. The video will be rendered full screen, but the captions will show as upscaled blocky text - as you currently have it. This is less CPU intensive.
  • The other option would force VLC to render the video at the full screen resolution (let's say, 1920x1080), and render the captions at that resolution. The video wouldn't have any perceived difference to the user, but the captions would look sharper as they had been rendered at a much higher resolution. This is more CPU intensive, though.
  • An easy way to see what I'm referring to is to run the screensaver on the Pi, with the game info on. You'll see what I'm going for.
• I like the text change - will use that instead.
• As for the game info, I get where you're coming from. However, just for perspective, the videos will cycle every 30 seconds. As I mentioned, the game name shows during the first 8 seconds, and the last 4 seconds - so, 12 seconds out of the 30 seconds. In the worst case scenario, someone who takes a peek at the video and likes what they see, will have to wait 18 seconds to see the actual game name, though on average the time one would need to wait to see the game name will be 9 seconds. In 1/3 of those occasions they'll look at the video and the name will be showing, though. :) So it doesn't feel like a tremendous amount of time to wait. I added the captions precisely because of that use case, but having them permanently on the screen ended up being somewhat ugly, especially for games with longer titles. I am not keen on making that the default, and I'd hesitate to add an option to make it on all the time just because it's "yet another option" (though it's a possibility), so I'd probably suggest trying it out for a while and if after a few weeks that still bothers you a lot we can definitely change it at that time. Though, thinking about it, I can instead make if a selection of "Never/Normal/Always" instead of ON and OFF if it's something that critical. Let me know.
• On the screensaver and making "stretch video" as the default, I do have to say I hesitate about that. I'm open to suggestions on that default. The main reasons that that is not the case is that, in particular for vertical games, it looks atrocious, and it does not match the actual game when the user chooses to launch it. I'm not that fixed on this particular default - as long as the option is there - so do let me know what your thoughts are and that's a simple change.

@Hex : I'll add that then. Thanks. As for the edge case, it will likely be a very rare occasion - that, truth be told, will be changed as soon as the user changes video - so if fixing that requires a lot of work, I'd not insist on it.

Thanks all!

[meleu]

@pjft one more suggestion: on system view, the "button help" below says for select "RANDOM VIDEO SCREENSAVER". My suggestion: "LAUNCH SCREENSAVER".

[pjft]

@meleu Sounds good.

Any further thoughts on the stretch defaults and subtitles, just so this is closed off?

[pjft]

So, I've added the option to show the game info "Always", "Never" and "Start & End". The default is still "Never", as it can cause problems for some Pi resolutions, unless the user disables overscan in config.txt.

I've left stretch as false by default. It will still cover the full screen, just with black bars, but keep the game's original proportions.

I edited the option and help text as well, and added the ALSA condition.

I need to confirm that this compiles and that all is good (blind coding, yay), but if all is good then that's the last of it.

Thanks all for the inputs and testing! I'll update when I've confirmed it compiles.

[meleu]

@pjft

Any further thoughts on the stretch defaults and subtitles, just so this is closed off?

The vertical screen games example (e.g. DonPachi) convinced me, I'll let it disabled (maybe it should be always disabled and the option removed?).

Regarding subtitles the options you provided are good. I'll choose the "ALWAYS" option.