OK button as Play/Pause

[cexcell]

First OK press now shows Video UI. It's achievable via hidden Roku Video node property showUI, which allows you to override how trickplay should be displayed by specifying parts of the UI to be rendered (like 'titleAndTime', 'trickplayBar', e.t.c). A timer is started every time custom UI is shown to prevent UI from staying forever.

Second OK press while UI is visible pauses the video (pause poster animation is not added yet).

However, since we shouldn't break default OK button actions, we must track whether user is seeking in video UI and if they do, fallback to Video internal OK handler.

Also, auto-formatted VideoPlayer.xml

[iBicha]

Thanks for the PR! This seems to cover ##296 well. I do have some concerns about:

• Using internal state. For example, you set m.top.showUI = { "show": true, "trickPlaybar": true, "titleAndTime": true, "trickplayBackgroundOverlay": true, "focusTrickplayBar": false }. But if Roku introduces a new UI element to the default player, then we will be overriding the showUI with incorrect state
• Handling Live videos. This adds some weird behaviour
  • The OK button pauses the video, but the pause button will not pause the video (a bit strange - not that there's an expectation to be able to pause the video yet #261)
  • Another small side effect that needs to be handled in Live videos pausing, if someone pauses the live stream for say, a few minutes, then resume, they might want to be back at the edge of the stream. But in this case they won't, so they will be forced to be behind. Not that this needs to be covered here, but just to show the edge cases that users will expect to be covered (but are hard to reason about using Roku's internal state)
  • A bug that prevents the video from ever resuming:
  • Start a playing live video - UI is not shown
  • Press OK to show the UI, then press OK again to pause, then press left
  • Now you can press OK or resume as much as you want, the live video won't unpause
• Trickplay thumbnails can be stuck after the UI is hidden (see screenshot). To reproduce:
  • Play a video - UI is not shown
  • On the remote, click right, then immediately click ok
  • The UI is now hidden, except thumbnails
• Another edge case where the UI can remain visible after the timer
  • Play a video - UI is not shown
  • Press ok to show the UI
  • Immediately start going in and out of picture-in-picture mode (press down to shrink, * to full screen) - do this continuously for over 5 seconds (long enough to exceed the timer)
  • Finish at the fullscreen player - the UI (background, video title, and time) is now visible and won't go away
  • Need to pause again to fix the state

It's the edge cases that led me to believe implementing this after a full Video node UI rewrite would be more sane https://github.com/iBicha/playlet/issues/296#issuecomment-1953409980. I suspect there's probably more edge cases that I didn't find yet. It's just going to be hard when complex state is managed by two systems, and one of them is obscure.

But if you can correct the more obvious edge cases, then by all means I'll be here to help push it forward! Let me know what you want to do

Edit: Screenshot of thumbnails stuck but video is playing (screenshot functionality does not capture video, and is set to black, but the video is playing)

[cexcell]

Thank you for your feedback.

For example, you set m.top.showUI = { "show": true, "trickPlaybar": true, "titleAndTime": true, "trickplayBackgroundOverlay": true, "focusTrickplayBar": false }. But if Roku introduces a new UI element to the default player, then we will be overriding the showUI with incorrect state

You're right, that's a major downside of using undocumented fields, but the other option is to use video.trickplayBar, video.bifDisplay and create a custom controller with animations for them. Or like you said, a whole new UI layer that simply ignores standard Roku trickplay ui, but that's a bit out of scope for this change :-) The only protection here is that we can just check if showUI field actually exists, before proceeding with our custom OK handler.

Handling Live videos. This adds some weird behaviour

Let me check those, I've got some weird connectivity issues with my TV in the bedroom. Didn't intend to introduce unwanted behavior :-) Live video should be handled separately

Trickplay thumbnails can be stuck after the UI is hidden (see screenshot). To reproduce:

Yeah, missed that, will fix

Another edge case where the UI can remain visible after the timer

Ooops, let me try to fix that. But I believe if you do that w/o my changes (just leave it to default Roku UI behavior) title and time won't disappear either, I believe that's a Roku Video node bug, but again, I can be wrong.

Quick question: do you want me to add a pause poster splash like Roku has be default when video is paused?

[cexcell]

I hope my latest commit fixes some of the issues. For some reason I cannot load thumbnails, so couldn't verify that :-(

Also, there is an internal Roku UI timer (~6 seconds) that hides UI automatically, so there is an inevitable overlapping, like:

1. Press Play button to pause the video.
2. Press OK to resume it after waiting for about 3 seconds.
3. Press OK to show trickplay UI again.
4. UI will be automatically hidden after ~2 - 3 seconds, because Roku internal timer was still ticking.

That's something that workaround patches like this one won't be able to fix.

[iBicha]

Thanks for making the changes! I think another change related to the live check might be it

But I believe if you do that w/o my changes (just leave it to default Roku UI behavior) title and time won't disappear either, I believe that's a Roku Video node bug, but again, I can be wrong.

I tried on your first version, and saw the bug, then without the change, no bug. But then tried again without your PR, and the UI got stuck. So perhaps it's a bug in the Video node itself, but it's not consistent

For some reason I cannot load thumbnails, so couldn't verify that :-(

Thumbnails only show when using video quality Auto (DASH) - perhaps you have something else in the settings?

Quick question: do you want me to add a pause poster splash like Roku has be default when video is paused?

No need, that's just a cosmetic thing. If we can add this feature without any major bug/side effect, then it's good by me

The new changes seem to do the trick from initial testing

[cexcell]

So perhaps it's a bug in the Video node itself, but it's not consistent

I'm now hiding UI when we do any scaling for video node just to avoid any stale elements, so it shouldn't matter ( at least in theory)

[cexcell]

Thanks for the PR and for addressing all the comments!

You're welcome! Hopefully this doesn't introduce any serious regressions