Skip to content

setNext changes - #4

Open
stoyicker wants to merge 2 commits into
mainfrom
jantonio/setnext_updates
Open

setNext changes#4
stoyicker wants to merge 2 commits into
mainfrom
jantonio/setnext_updates

Conversation

@stoyicker

Copy link
Copy Markdown
Contributor

As discussed on Slack, this adds a new event to signify successful completion of setNext calls.

@stoyicker
stoyicker force-pushed the jantonio/setnext_updates branch from d96a6d6 to a110a60 Compare July 10, 2024 13:00
@asendra

asendra commented Jul 15, 2024

Copy link
Copy Markdown

Two small questions that I hope are not dumb.

1. What does it mean successful completion of setNext exactly?

If it means after loading the PlaybackInfo, same as when we do a load, then this new event on iOS might take a while, because we don't start to load the next item it until the current item is fully loaded. (Either fully downloaded because it is an offlined file, or fully cached by the player while streaming it)

If it means as soon as we internally set the self.nextItem property of PlayerEngine, then maybe we could clarify here what can cause the event to not be sent?

On iOS I see 3 possible conditions:

  • Player state is IDLE
  • setNext is called for the same MediaProduct we already have (I guess we could still send the event here..)
  • setNext is called with a nil MediaProduct

2. I consider this event an optional event for the consumers of our SDK. Do we need to clarify that in the spec or is that up to each platform?

@stoyicker

Copy link
Copy Markdown
Contributor Author
  1. My intent was that the meaning of successfully setting next refers to setting the property only. Also, regarding the conditions in which the event fails to be sent, I would think that for the last 2 bullet points you mention it is sent, leaving the IDLE case as the only condition where it does not get sent. I'd rather this not be specified in the documentation as it does not look to me like the value of the information it adds is worth having to update it if we change the internal implementation, but I can live with it.
  2. What does this mean? Are there any events that are not optional for consumers to listen to?

@asendra

asendra commented Jul 15, 2024

Copy link
Copy Markdown
  1. I did not mean to specify it in the documentation, just that we synced on it. But that can be done through other channels.
  2. Probably specific to iOS, but we have some events that are optional and others that are not. For those that are not optional, the Compiler will complain if the consumer does not define the listener to those events. They can of course be no-ops... But this can also be something up to each platform, or not defined at the spec level at least.

The ones that are not optional on iOS are:

  • stateChanged(to state: State)
  • ended(_ mediaProduct: MediaProduct)
  • mediaTransitioned(to mediaProduct: MediaProduct, with playbackContext: PlaybackContext)
  • failed(with error: PlayerError)

@stoyicker

Copy link
Copy Markdown
Contributor Author

I see. All events on Android are optional currently, and changing things to force consumers to treat some specifically would require changing the API from an event stream that the consumer hooks up to to more of a listener which the consumer implements and "installs" into Player, which I'm not sure respects the spec. Let's take this internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants