Feat/ui custom castproxy #2464

tecteun · 2020-03-20T18:00:38Z

This is an attempt at extending ui/ui.js with an optional external castProxy class.

there are still some open ends, but to start the discussion I started this pull request.

If there are any good idea's on how this should better be implemented, I love to hear them!

googlebot · 2020-03-20T18:00:41Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

joeyparrish · 2020-03-20T19:05:42Z

I have some technical feedback around what looks like some compiler issues, but first let's discuss your goals with this change.

What are the main goals of a custom CastProxy?

tecteun · 2020-03-20T20:57:14Z

I have a project where I would like to use the stock ui, with some minor modifications. One of the modifications being the Chromecast sender implementation.
The main goal for this custom CastProxy is to be able to use a different Chromecast receiver implementation, using the default Shaka build.

I now can use a custom CastProxy sender implementation, using CAF v3 api's. This works fine for me, using this additional argument for shaka.ui.Overlay. Though, I'm struggling a bit on how to make closure compiler cope with this externally defined castproxy, and make it a good general addition to the project.

joeyparrish · 2020-03-20T22:39:51Z

Yeah, the compiler is difficult sometimes. I'll leave you some specific technical feedback on the changes about the compiler. Hopefully we'll be able to help you resolve that.

I'm open to this change generally, but I wonder if you'd be willing to share your CAF v3 implementation with us as well. Does it use CAF on the sender side instead of the older Cast SDK? Does it still expect Shaka Player on the receiver side, or does it use the CAF default receiver?

joeyparrish · 2020-03-20T22:42:06Z

externs/shaka/cast.js

@@ -0,0 +1,56 @@
+


When you define externs, you need to have a block comment containing @externs at the top. Please copy the copyright header and @externs block comment from another extern file.

Without that, this file will not be seen as an "external" interface from the environment, and pieces of the implementation can still be renamed and inlined. I see you are using ['foo'] style to avoid renaming, but inlining can't be defeated in this way.

When you fix the externs header, you should see those issues go away.

joeyparrish · 2020-03-20T22:43:05Z

ui/cast_button.js

@@ -39,7 +39,7 @@ shaka.ui.CastButton = class extends shaka.ui.Element {
  constructor(parent, controls) {
    super(parent, controls);

-    /** @private {shaka.cast.CastProxy} */
+    /** @private {shaka.cast.CastProxy|shaka.extern.CastProxy} */


Since shaka.cast.CastProxy is an implementation of shaka.extern.CastProxy, you should just use the base type here.

joeyparrish · 2020-03-20T22:43:27Z

ui/cast_button.js

@@ -94,12 +94,12 @@ shaka.ui.CastButton = class extends shaka.ui.Element {

    /** @private */
  async onCastClick_() {
-    if (this.castProxy_.isCasting()) {
-      this.castProxy_.suggestDisconnect();
+    if (this.castProxy_['isCasting']()) {


I believe these changes to bracket notation can all be reverted once you fix the externs file.

joeyparrish · 2020-03-20T22:43:52Z

ui/controls.js

    video, player, this.config_.castReceiverAppId);

  /** @private {boolean} */
  this.castAllowed_ = true;

  /** @private {HTMLMediaElement} */
-  this.video_ = this.castProxy_.getVideo();
+  this.video_ = this.castProxy_['getVideo']();


Same here. Try reverting these changes once the externs are fixed.

joeyparrish · 2020-03-20T22:44:39Z

ui/controls.js

@@ -675,11 +676,15 @@ shaka.ui.Controls.prototype.addControlsContainer_ = function() {
  this.videoContainer_.appendChild(this.controlsContainer_);

  this.eventManager_.listen(this.controlsContainer_, 'touchstart', (e) => {
-    this.onContainerTouch_(e);
+    if (!e.defaultPrevented) {


This looks like it's probably a useful change independent of the Cast changes. What's the use case? Would you be willing to split that into a separate PR?

joeyparrish · 2020-04-20T17:39:42Z

I've just noticed that you're developing against the v2.5.x branch. We normally only accept PRs against master. Cherry-picks to a release branch are always done by the team after carefully considering the risk of a change and any backward-compatibility issues it might create. This does not seem like something we would cherry-pick to an older branch.

If you'd like to continue this PR, please rebase against master and address the feedback you've received so far. Thanks!

tecteun · 2020-04-20T19:06:08Z

Thanks for the explanation, I'll probably handle this rebase and changes the coming week! I couldn't find any time to finish this MR due to the unexpected chain of events. My productivity plummeted, because, you know, kids ;)

joeyparrish · 2020-04-20T19:41:05Z

No worries! We understand, and there's no urgency from our perspective.

googlebot · 2020-04-22T12:21:34Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

… calls, in case of chromecast

avelad · 2022-05-18T09:48:01Z

Soon, we are going to work on #4214 so for now I don't think we're going to add this. If you have any suggestion tell me.

Teun van Roovert added 2 commits March 14, 2020 00:09

Enable custom castproxy in shaka.ui.Overlay

b43b098

Allow preventing default for container click / touch

478d3d2

joeyparrish requested changes Mar 20, 2020

View reviewed changes

Teun van Roovert added 2 commits April 20, 2020 13:23

ui seekbar: remember paused state when seeking

be91e05

ui seekbar: seek to end needs offset, so events are still triggered

c356903

tecteun force-pushed the feat/ui-custom-castproxy branch from 4629f54 to c356903 Compare April 20, 2020 14:00

ui seekbar: fix empty seek bar / glitching when seeking

9f7f537

tecteun force-pushed the feat/ui-custom-castproxy branch from 9ae6a64 to 9f7f537 Compare April 20, 2020 14:47

Merge branch 'v2.5.x' into feat/ui-custom-castproxy

af66d00

tecteun force-pushed the feat/ui-custom-castproxy branch 3 times, most recently from b43a60c to af66d00 Compare April 22, 2020 12:21

tecteun force-pushed the feat/ui-custom-castproxy branch from 0d369eb to af66d00 Compare July 21, 2020 15:42

tecteun added 3 commits July 21, 2020 18:43

add shaka.extern.Player definition to avoid mangling proxied function…

a1de44d

… calls, in case of chromecast

add more missing shaka.extern.Player methods

9a0f69c

fix debug build

0b513f1

avelad closed this May 18, 2022

github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/ui custom castproxy #2464

Feat/ui custom castproxy #2464

tecteun commented Mar 20, 2020

googlebot commented Mar 20, 2020

joeyparrish commented Mar 20, 2020

tecteun commented Mar 20, 2020

joeyparrish commented Mar 20, 2020

joeyparrish Mar 20, 2020

joeyparrish Mar 20, 2020

joeyparrish Mar 20, 2020

joeyparrish Mar 20, 2020

joeyparrish Mar 20, 2020

joeyparrish commented Apr 20, 2020

tecteun commented Apr 20, 2020

joeyparrish commented Apr 20, 2020

googlebot commented Apr 22, 2020

avelad commented May 18, 2022

Feat/ui custom castproxy #2464

Feat/ui custom castproxy #2464

Conversation

tecteun commented Mar 20, 2020

googlebot commented Mar 20, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

joeyparrish commented Mar 20, 2020

tecteun commented Mar 20, 2020

joeyparrish commented Mar 20, 2020

joeyparrish Mar 20, 2020

Choose a reason for hiding this comment

joeyparrish Mar 20, 2020

Choose a reason for hiding this comment

joeyparrish Mar 20, 2020

Choose a reason for hiding this comment

joeyparrish Mar 20, 2020

Choose a reason for hiding this comment

joeyparrish Mar 20, 2020

Choose a reason for hiding this comment

joeyparrish commented Apr 20, 2020

tecteun commented Apr 20, 2020

joeyparrish commented Apr 20, 2020

googlebot commented Apr 22, 2020

avelad commented May 18, 2022