Fix Crash When Options Button Pressed, but Options Are Not Available

[ApexArray]

Pressing the back options button during initial video buffering may trigger this code with a group not containing optionsAvailable property, leading to a crash. In my case, the entire Roku device crashed and reset.

Changes
Checks if group.optionsAvailable is valid before we observe it.

Converts another <> invalid check to isValid()

[sevenrats]

why do you feel that isValid() is superior to <> invalid?

[ApexArray]

I feel it’s more intuitive than <> invalid and more friendly to Britghtscript newcomers.

We’re using using a mix of the two right now, but we should probably pick one and standardize. I’ve seen more preference for IsValid() lately, so this is my attempt to help improve uniformity over time.

[1hitsong]

why do you feel that isValid() is superior to <> invalid?

We had a whole discussion (pros/cons) about this in the chat room when I first wrote it. It helps improve readability at a glance, which in turn improves long term maintainability.

The basic idea is removing negatives from conditions as part of our code style.

Ref: https://www.codeproject.com/articles/1073387/robert-s-rules-of-coders-sharp-avoid-negative-cond

[1hitsong]

@ApexArray I'm unable to recreate the issue. The title says it occurs when pressing back, but your code change is inside a handler for pressing options. What are the steps to recreate?

[ApexArray]

@1hitsong you're right, I must have gotten mixed up. It's actually crashing when pressing options button.

To reproduce, start playing any video and immediately press the options button. It's more prevalent in videos that require transcoding since they take longer to buffer initially.

Are you able to reproduce with that?

[1hitsong]

Nope. I can't get this issue to fire. Perhaps Jimi or Neil can.

[jimdogx]

@ApexArray I'm not able to reproduce the crash on 1.6.2 either. I tried movies, 4k movies, 4k shows, tv shows and live tv. I pressed the "*" several times while the video was buffering and did not receive a crash.

[1hitsong]

@ApexArray Are you sure this is the source of the crash? Can you cause the crash on your machine and post the console output?

What reason I ask is I tested this by hardcoding group.optionsAvailable to invalid and everything worked as expected in Unstable.

[ApexArray]

Yep, just got it to fire again. The problem is that that group has no property optionsAvailable.

12-29 18:05:14.501 [beacon.signal] |AppSplashComplete ---------> Duration(987 ms)
------ Running dev 'Jell1yf2i-2n9' 1 8mai:n 0-5-:-1---4
.872 [scrpt.ctx.run.enter] UI: Entering 'Jellyfin', id 'dev'
Unhandled roDeviceInfoEvent:
<Component: roAssociativeArray> =
{
    Mode: "On"
    Mute: false
}
12-29 18:05:16.254 [beacon.signal] |AppLaunchComplete ---------> Pending Render Pass
12-29 18:05:16.284 [beacon.signal] |AppLaunchComplete ---------> Duration(2773 ms)
BrightScript Micro Debugger.
Enter any BrightScript statement, debug commands, or HELP.
Suspending threads...
Thread selected:  1*   pkg:/components/JFScene.brs(14)         if group <> invalid and group.optionsAvailable
Current Function:
006:  function onKeyEvent(key as string, press as boolean) as boolean
007:      if not press then return false
008:  
009:      if key = "back"
010:          m.global.sceneManager.callFunc("popScene")
011:          return true
012:      else if key = "options"
013:          group = m.global.sceneManager.callFunc("getActiveScene")
014:*         if group <> invalid and group.optionsAvailable
015:              group.lastFocus = group.focusedChild
016:              panel = group.findNode("options")
017:              panel.visible = true
018:              panel.findNode("panelList").setFocus(true)
Source Digest(s): 
pkg: dev 1.6.3 8d2470ef Jellyfin
Type Mismatch. Operator "and" can't be applied to "Boolean" and "Invalid". (runtime error &h18) in pkg:/components/JFScene.brs(14)
Backtrace:
#0  Function onkeyevent(key As String, press As Boolean) As Boolean
   file/line: pkg:/components/JFScene.brs(14)
Local Variables:
key              roString (2.1 was String) refcnt=2 val:"options"
press            Boolean val:true
global           Interface:ifGlobal
m                roAssociativeArray refcnt=2 count:2
group            roSGNode:JFVideo refcnt=1
panel            <uninitialized>
Threads:
ID    Location                                Source Code
 0    pkg:/source/Main.brs(72)                msg = wait(0, m.port)
 1*   pkg:/components/JFScene.brs(14)         if group <> invalid and group.optionsAvailable
  *selected
Brightscript Debugger> 

Here's a screenshot of the debugger on crash. Notice the property is absent.

[ApexArray]

It's reliably fired on all my TV Shows and movies so far. I tested with multiple qualities, codecs, transcoding/direct stream and I get the same result every time.

The only difference I can think of at this point is that I'm testing with an older Roku 4200X.

I have a newer Roku 3800X in the living room that I can test this evening.

EDIT: Confirmed the issue is NOT present on my 3800x. JFVideo gets focus right away, so it never hits the affected code in JFScene.

[ApexArray]

Okay, sorry for spamming a bit here but I keep coming across new information.

@1hitsong's test should have reproduced my condition, but it seems that there's more than one kind of "invalid" in Brightscript. I can reproduce the exact output as long as group.optionsAvailable exists in the first place.

For example, pressing options from the home screen:

But I still get the crash if I press it during buffering. Line 14 has no effect in my case (property is not added), and it continues to crash. Apparently an "existing" invalid is different from a "non-existing" invalid.

This means I'm likely pulling a different group than you guys during buffering. I added some debug code to SceneManager.brs:

Printed output during crash

current groups:                 <Component: roArray> =
[
    <Component: roSGNode:Home>
    <Component: roSGNode:JFVideo>
]
Selected group:                 <Component: roSGNode:JFVideo> =
{
    allowOptionsKeyOverride: false
    alwaysShowVideoPlanes: false
    audioFormat: ""
    audioTrack: "2"
    autoPlayAfterSeek: true
    availableAudioTracks: <Component: roArray>
    availableSubtitleTracks: <Component: roArray>
    bifDisplay: <Component: roSGNode:BifDisplay>
    bottomRowButtonFocused: 0
    bottomRowButtons: invalid
    bottomRowButtonSelected: 0
    bufferingBar: <Component: roSGNode:ProgressBar>
    bufferingBarVisibilityAuto: true
    bufferingBarVisibilityBounds: <Component: roAssociativeArray>
    bufferingBarVisibilityHint: false
    bufferingStatus: <Component: roAssociativeArray>
    bufferingTextColor: 0
    capFontPath: ""
    capFontSize: 0
    captionStyle: invalid
    cdnSwitch: invalid
    cgms: "norestriction"
    clipId: 0
    completedStreamInfo: invalid
    content: <Component: roSGNode:ContentNode>
    contentBlocked: false
    contentIndex: 0
    contentIsPlaylist: false
    control: "play"
    controlExt: invalid
    currentAudioTrack: ""
    currentSubtitleTrack: ""
    customTitle: <Component: roInvalid>
    decoderStats: invalid
    disableScreenSaver: false
    downloadedSegment: invalid
    drmHttpAgent: <Component: roHttpAgent>
    duration: 712
    enableDecoderStats: false
    enableLiveAvailabilityWindow: true
    enablePivotNode: true
    enableScreenSaverWhilePlaying: false
    enableThumbnailTilesDuringLive: false
    enableTrickPlay: true
    enableTunerAbsoluteSeek: false
    enableUI: true
    errorCode: 0
    errorInfo: invalid
    errorMsg: ""
    errorStr: ""
    focusedItem: ""
    gain: 0
    globalCaptionMode: "Off"
    height: 0
    interstitialPods: invalid
    interstitialPodsProcessingDone: invalid
    licenseStatus: invalid
    loop: false
    manifestData: invalid
    maxVideoDecodeResolution: <Component: roArray>
    mute: false
    nextContentIndex: -1
    notificationInterval: 0.5
    pauseBufferEnd: 712
    pauseBufferOverflow: false
    pauseBufferPosition: 39
    pauseBufferStart: 0
    pivotNode: <Component: roInvalid>
    playbackActionButtonFocused: 0
    playbackActionButtonFocusedTextColor: -1
    playbackActionButtonFocusedTextFont: <Component: roInvalid>
    playbackActionButtonFocusIndicatorBlendColor: -1
    playbackActionButtonHeight: 0
    playbackActionButtons: invalid
    playbackActionButtonSelected: 0
    playbackActionButtonUnfocusedTextColor: -1
    playbackActionButtonUnfocusedTextFont: <Component: roInvalid>
    playbackSpeed: 1
    playStartInfo: invalid
    position: 0
    positionInfo: invalid
    retrievingBar: <Component: roSGNode:ProgressBar>
    retrievingBarVisibilityAuto: true
    retrievingBarVisibilityBounds: <Component: roAssociativeArray>
    retrievingBarVisibilityHint: false
    retrievingTextColor: 0
    seek: 0
    seekClip: invalid
    seekMode: "default"
    showUI: invalid
    state: "buffering"
    streamInfo: <Component: roAssociativeArray>
    streamingSegment: invalid
    subtitleTrack: ""
    supplementaryAudioVolume: 50
    suppressCaptions: false
    teletextTrack: ""
    thumbnailTiles: invalid
    timedMetaData: invalid
    timedMetaData2: invalid
    ...
}

My JFScene is still in focus during buffering and pulls JFVideo from m.groups, but we're not setting an optionsAvailable in JFVideo. Roku natively handles the options key in Video nodes so it should take over once JFVideo is in focus.

This PR fixes that by checking whether optionsAvailable exists before we evaluate it.

EDIT: Again, I confirmed the issue is NOT present on my 3800x. JFVideo gets focus right away, so it never hits the affected code in JFScene. The issue only occurs on my 4200x.

[1hitsong]

I was able to test this by overriding group without an optionsAvailable property.

Unstable crashes. This PR prevents the crash and doesn't show the options menu.