-
Notifications
You must be signed in to change notification settings - Fork 4
Playback Mode Field #87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: fp/subview-metrics
Are you sure you want to change the base?
Conversation
… into fp/playback-mode
… into fp/playback-mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one change. Hopefully simple.
Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall - does this even work? If so, how, what am I missing about how Roku works?
second, I'm okay with just telling Roku customers to set playbackMode = { mode: 'string', data: JSONstring }
rather than having to make them use player_playback_mode_data
src/mux-analytics.brs
Outdated
end if | ||
props.player_playback_mode = playbackMode.mode | ||
|
||
if playbackMode.player_playback_mode_data = Invalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this what we want to specify as the field name, @daytime-em ? It feels a little bit verbose, and I'm not really against it, but if we have playbackMode.mode
and playbackMode.data
wouldn't that work for us?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks for taking the changes!
I responded to the player_playabck_mode_data
thing on Slack. I'm good with it, but if we change to data
, it's still a ✅ from me
This PR adds a new field called
playback_mode
which accepts an assocarrayThis field is handled in the
playbackModeHandler
, this method does the following:mode
is set, if not, it warns the user and returnsplayer_playback_mode_data
is set, if not, it warns the user and returnsplayer_playback_mode_data
is parse-able JSON, if not, it warns the user and returnsplaybackmodechange
containingmode
,player_playback_mode_data
,m._cumulativePlayingTime
andm._totalAdWatchTime
To check if a string is parse-able JSON, the
ParseJson
utility function was used, to check out how it works, here's the documentation