Skip to content
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

Breaking changes for Shaka Player v4 #3188

Closed
joeyparrish opened this issue Mar 1, 2021 · 13 comments
Closed

Breaking changes for Shaka Player v4 #3188

joeyparrish opened this issue Mar 1, 2021 · 13 comments
Labels
status: archived Archived and locked; will not be updated type: announcement An announcement from the team; generally pinned to the top

Comments

@joeyparrish
Copy link
Member

joeyparrish commented Mar 1, 2021

Hi everyone,

Now that we are following semantic versioning more strictly (since v3.0.0), we are not making any breaking changes until v4. This issue is to collect ideas and discuss the sorts of breaking changes we could make in v4. We don't have a timeline for Shaka Player v4, but I would expect to bump the major version some time in the next 12 months (end of Q1 2022).

@joeyparrish joeyparrish added type: announcement An announcement from the team; generally pinned to the top flag: bot ignore Asks CI bots that maintain issues to ignore this issue labels Mar 1, 2021
@joeyparrish joeyparrish pinned this issue Mar 1, 2021
@joeyparrish
Copy link
Member Author

To start, I would like to rethink gap-jumping.

The API changes for this would be minor (a few changes to the configuration fields only), but would be breaking changes. I intend to publish a doc in the near future detailing my ideas on this, but in short, I would like to get rid of the "small/large" gap concept, to always jump gaps by default, and to completely rethink the behavior and triggers of gap-jumping. I also think it would benefit from combining with stall detection.

@joeyparrish
Copy link
Member Author

I've also been thinking about dropping CastProxy and shaka.cast and integrating the UI directly with the Cast Application Framework v3.

In short, the way we're offering these Cast APIs predates CAF and is not the way the Cast team would have us do things. By proxying the full Shaka Player API across the Cast message bus as we do today, we have a complete mirroring of every method from sender to receiver. The downsides are that we send a lot more data than CAF would, and we are unable to access touchscreen capabilities on the receiver (which are available only to CAF).

By dropping the entire shaka.cast namespace and connecting our UI features to the CAF SDK instead of through CastProxy, the cast system becomes more performant (less data sent back and forth) and aligns much more nicely to the preferred way of doing things. We get also get touchscreen support, and receiver apps get simpler to build (and don't have to involve Shaka Player directly).

One downside is that the UI elements would then all have to have conditionals to deal with cast. If local, call player.foo(), else call some CAF method. Or some elements would not work with cast and would hide themselves when we are in cast mode.

Another downside is that anyone who is currently using shaka.cast but not using our UI would have to build their own CAF integration to upgrade to Shaka v4. (It could be that most people are ignoring shaka.cast and doing things the recommended way according to public Cast docs, so this might not matter much.)

@joeyparrish
Copy link
Member Author

Another breaking change we should consider is dropping our abuse of Closure Compiler's "externs" to prevent renaming.

If we stop using public fields (as we do in SegmentReference) and record-type interfaces (as we do in the types consumed and produced by plugin interfaces), we could move everything to classes implemented by us in the library, with exported getters/setters/builders. Plugins that produce output would use a builder or setters provided by us, rather than returning plain objects that follow an interface. Plugins that consume input would use getters provided by us to access the things they need.

This would be a breaking change for all of those plugin interfaces, but would simplify the way we interact with application plugins, reduce a certain kind of compiler issue that we tend not to catch, and eliminate a whole class of compiler-renaming bugs. Externs would be strictly for what they were meant for: external interfaces in the execution environment.

@dulmandakh
Copy link
Contributor

How about transitioning to modern JS build tools like DevTools did https://www.youtube.com/watch?v=BHogHiiyuQk?

@joeyparrish
Copy link
Member Author

@dulmandakh, in the overview from that video, I see four migrations referenced:

  1. Custom modules -> ES modules
  2. Closure Compiler -> TypeScript
  3. Python build system -> Rollup
  4. Custom Components -> Web Components

None of those are a great fit for what we're discussing here, which are breaking API changes.

We don't have custom components, though offering a Web Component for Shaka Player is a great idea and deserves its own feature request. (I couldn't find one.)

The other issues all touch on the Closure Compiler, which today provides our module system, type safety, and what Rollup offers (bundling, dead code elimination). So none of those other things are happening without moving from Closure to TypeScript.

TypeScript may be in our future, but we're not considering it right now because of the absolutely massive effort that would be required for very little immediate benefit.

Thanks!

@chrisfillmore
Copy link
Contributor

(I've changed jobs and don't work with Shaka anymore, but I used Shaka almost daily for about 3 years at my previous job. Also some of my knowledge may be out of date.)

IMO the biggest pain point is that the library has few usable abstractions. When working on an actual consumer video product, the following situations come up frequently:

  • My company wants to support some device which runs an HTML5 environment (this could be anything from a new browser to a refrigerator or a car), so we go through testing our product, then if it doesn't work out of the box we test with bare Shaka, etc, and troubleshoot the vendor's MSE/EME implementation. This leads to situations where the solution is "we can get this to work with a bunch of hacks". I always tried my best to find a sensible upstream solution, but it was not always possible.
  • In a similar vein, my company supports some device which rolled out an update that breaks something, and I need to develop a workaround in application code which I can deploy tomorrow, instead of waiting weeks/months for a fix from the vendor.
  • My company's product is deployed to multiple device platforms, and the behaviour of the player on e.g. Roku or Android is different than (perhaps incompatible with) the behaviour of Shaka, and those platforms have more users, or my company has a special business arrangement, or doesn't have an arrangement, so I need to adapt our web client because it's less risky. (Risk aversion sadly drives a lot of these sorts of decisions.) I can't tell you how many times I escalated issues within my company like, "Can we please use this feature in DASH to solve this problem I have on web clients", and the response I got was "do Roku and Android support it?"
  • My company wants to understand and control the player's networking behaviour. I would prefer to delegate network behaviour back to our own stack, but unfortunately NetworkingEngine is not injectable. So now I have two networking stacks, one of which I can only control via configuration.
  • My company wants to add support for some feature into the player (things like: hot channel switching, preload, extended buffer cacheing). Shaka does not expose any hooks to add support for these kinds of things. (For example, I was able to develop a feature for ExoPlayer which enabled pausing/rewinding live TV by implementing their ParsingLoadable interface. Such a feature is valuable to my company even if there's no official spec saying how clients and servers should precisely behave. Being able to develop this extension externally and inject it means that I can tailor it to my particular business needs. I don't have to solve the general problem.)

As a fully integrated product, Shaka works beautifully, and the Shaka team has always been very supportive, which I appreciate. And I see that the tradeoff of exposing new hooks, points of entry, and injection points, is that client code can get themselves into a lot more trouble, and it's harder to help them when something goes wrong. But on the other hand, it means that many of your users are likely forking Shaka internally, to add support for things they need (this was the case with us). Which means Shaka is possibly missing out on contributions, interesting or innovative use cases, and new device support.

If you're now wondering what behaviour I would like to be able to override with my own implementations, the answer is all of it. :)

@caridley
Copy link
Contributor

Please think about application migration strategy. There will almost certainly be some period of time where our application will need work with both the new and old versions. Perhaps a facade implementing the old interface could be proved if there are to be major changes to the interface.

@valotvince
Copy link
Contributor

We could set com.microsoft.playready.recommendation keySystem per default instead of com.microsoft.playready in dash.keySystemsByURI

@caridley
Copy link
Contributor

I think Shaka should move on from period flattening. It may have simplified some Shaka internals and improved reliability for some scenarios, but it has made it impossible to optimize support for selecting tracks based on track attributes in individual periods and compromised the accuracy of stream bitrate information used for ABR. I think we need to restore the ability to made streaming decisions based on period specific information.

@avelad
Copy link
Collaborator

avelad commented Feb 18, 2022

@joeyparrish since there is a change that already requires a 4.0 ( #3964 )can we remove the castproxy integration in this release? (I volunteer to do it if you wish)

@joeyparrish
Copy link
Member Author

I am not against removing shaka.cast, but only if we have something to replace it with. Getting rid of our custom Cast weirdness would be good, but I don't want to lose casting as a feature.

Ideally, we could use a generic receiver and integration between the Shaka UI and CAF as a sender. (Although I hear styled receivers can't do DRM, which may be an issue.) I think this is something we'll probably work on at some point in 2022, especially now that we're under the Cast team at Google. But I'm happy to have you explore that now if you're interested.

@joeyparrish
Copy link
Member Author

@caridley, I respect your opinion, and I recognize that period-flattening has been unpopular at times and had a rocky start. But "moving on" from period-flattening could be a ton of work, and cause more instability during the transition.

For my team, I'm going to prioritize improving and stabilizing HLS before taking apart DASH again. But if you want to explore this, modify shaka.extern.Manifest, do a proof-of-concept, etc, please feel free. I'd be happy to discuss your findings and plan that transition with you if it seems likely to succeed.

@joeyparrish
Copy link
Member Author

Since v4 is coming sooner than we anticipated, we won't be implementing these ideas in v4:

  • Ripping out and rewriting gap-jumping logic
  • Removing CastProxy
  • Migrating "extern" interfaces for plugins into the library
  • TypeScript migration
  • New build system
  • Replacement NetworkingEngine for deeper control of networking
  • Changing default PlayReady keysystem ID

I'm still open to all of them, though, in a future major release (v5+). We just don't have time for these if we want to get out support for containerless/packed/raw audio streams and latency improvement in HLS, which will require a v4 release.

These breaking changes will appear in v4:

  • Packed audio support will require us to drop support for HLS on Tizen 2, Tizen 3, and WebOS 3, which lack support for MediaSource sequence mode.
  • Player's addTextTrack() method has been replaced by addTextTrackAsync(), which returns a Promise.
  • Offline storage's getStoreInProgress() method has been removed, because concurrent downloads are supported.
  • Configuration changes:
    • Configuration for manifest.dash.defaultPresentationDelay has been moved to manifest.defaultPresentationDelay.
    • Configurations and methods which take factories must use plain factory functions, not constructors:
      • Config textDisplayFactory
      • Config abrFactory
      • Method shaka.Player.setAdManagerFactory()
      • Method shaka.media.ManifestParser.registerParserByExtension()
      • Method shaka.media.ManifestParser.registerParserByMime()
      • Method shaka.text.TextEngine.registerParser()
  • Plugin changes:
    • UI element plugins will be required to have a release() method.
    • ABR Manager plugins will be required to have a playbackRateChanged() method.
    • Text parser plugins will need to generate cues with the lineBreak flag instead of the spacer flag.
  • Manifest changes:
    • The SegmentIndex async method destroy() has been replaced by the synchronous method release().
    • The SegmentIndex method merge() will become private; use mergeAndEvict() instead.
    • The SegmentIndex method seek() has been replaced by getIteratorForTime().
  • Utility changes:
    • Uint8ArrayUtils.equal() has been replaced by BufferUtils.equal().

As for the other suggestions for breaking changes and future improvements that may require them, I will file new issues for each. Thanks, everyone! I appreciate your feedback on all of this.

joeyparrish pushed a commit that referenced this issue Apr 13, 2022
We get rid of the "small/large" gap concept, to always jump gaps by default

Related to issue #3188 (comment)
@joeyparrish joeyparrish removed the flag: bot ignore Asks CI bots that maintain issues to ignore this issue label May 4, 2022
@joeyparrish joeyparrish unpinned this issue May 24, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 3, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: announcement An announcement from the team; generally pinned to the top
Projects
None yet
Development

No branches or pull requests

6 participants