shaka-project/shaka-player

custom preference based criteria

Opened this issue · 5 comments

Have you read the FAQ and checked for duplicate open issues?
Yes

Is your feature request related to a problem? Please describe.

In our shaka fork, we have few changes to the preference based criteria that is too specific to upstream. However, since those changes are inside the preference based criteria, we either have to edit the file in-place or create a new file and edit the build types. In either case, the maintenance isn't ideal.

Describe the solution you'd like

We want to be able to provide shaka with a custom preference based criteria. This should work similarly to the ABR Manager or the Text Display, where a factory function is passed to shaka and shaka uses the factory to create the ABR Manager and Text Display instances. The built-in classes will be used by default.
Additionally, all the options passed to the preference based criteria should be moved to a config object. Having everything be a config option would make it easier for the custom criteria to extend with additional options, since we won't need to add arguments to the constructor and deal with merge conflicts around ordering.

Describe alternatives you've considered

The main two alternatives are to edit the file in-place or use the build system to include a separate file for it at build time. The former makes maintenance a lot worse. The latter has been working, but it still makes it harder to maintain the fork.
Additionally, we could subclass it in the shaka code, but that isn't much different than using a separate file and have the build system include that. It would be nicer to keep the custom criteria separate.

Additional context

Are you planning to send a PR to add it?
Yes, assuming there's buy-in on proceeding with this work.

I'm open to this. Can you give a more concrete proposal? I'd like to see what the factory/constructor interface will look like, and an example of criteria logic. (It doesn't have to be the logic you're using in your app, but something plausible to illustrate the benefit.)

Yeah, the quick answer is that it'll be very similar to the ABR Manager Factory, since moving to the config will make the constructor pretty simple. I'll write something more in-depth up in a bit.

Essentially, we want to filter down to a specific codec under certain conditions. This is part of a legal requirement. The current filters that Shaka provides don't really give us the flexibility we need.

I think the default preferenceBasedCriteriaFactory can be essentially like the abr one:

() => new shaka.media.PreferenceBasedCriteria()

Then usage would be something like:

this.currentAdaptationSetCriteria_ =
    this.config_.preferenceBasedCriteriaFactory();
this.currentAdaptationSetCriteria_.config({
  language: this.config_.preferredAudioLanguage,
  role: this.config_.preferredVariantRole,
  channelCount: this.config_.preferredAudioChannelCount,
  hdrLevel: this.config_.preferredVideoHdrLevel,
  spatialAudio: this.config_.preferSpatialAudio,
  videoLayout: this.config_.preferredVideoLayout,
  audioLabel: this.config_.preferredAudioLabel,
  videoLabel: this.config_.preferredVideoLabel,
  codecSwitchingStrategy: this.config_.mediaSource.codecSwitchingStrategy,
  audioCodec: '',
});

Additionally, if the preference based criteria was moved into a separate file, it would be easier for folks having custom-builds to exclude the built-in one and only include a custom one.

avelad commented

I moved this to a different file is that enough for you or do you still want to make a factory? (1f3af8b)

Appreciate the move to separate files! I think we would still prefer to also have a factory. It'll make things a lot more flexible for us, as we won't necessarily need this custom code to live in our shaka fork. With the switch to a config object for it, it'll make extending much easier.