Support Media Session title in demo #689

beaufortfrancois · 2017-02-09T10:06:54Z

With the brand new Media Session API, we can now customize media notifications by providing metadata for the media the web app is playing. It also allows us to handle media related events such as seeking or track changing which may come from notifications or media keys.

So here's a first stab to support the Media Session in the shaka demo.
WDYT?

mounirlamouri · 2017-02-09T10:12:24Z

demo/asset_section.js

+    if (playPromise !== undefined) {
+      // If video plays successfully, set media session title.
+      playPromise.then(function() {
+        navigator.mediaSession.metadata.title = asset.name;


Would it make sense to have something like:

var playPromise = shakaDemo.video_.play(); if (playPromise !== undefined && 'mediaSession' in navigator) { playPromise.then(function() { navigator.mediaSession.metadata = new MediaMetadata({ title: asset.name }); }); }

I was thinking about future use of the play promise so that people don't have to refactor the Media Session part and could simply add their code when play promise is resolved. Does that make sense?

Should the play promise check be first then?

You're absolutely right. I've updated patch.

ismena · 2017-02-09T18:02:18Z

demo/asset_section.js

-    // Reset the media session.
-    navigator.mediaSession.metadata = new MediaMetadata();
-    if (playPromise !== undefined) {
+  if (playPromise !== undefined) {


Thanks for the PR! Small nit: we generally avoid using '===' operator in the project unless there's a good reason to have it instead of just '=='.

beaufortfrancois · 2017-02-10T13:58:25Z

@ismena I've addressed your feedback.

ismena · 2017-02-10T17:47:34Z

@beaufortfrancois Cool, let me run it through our test bot.

shaka-bot · 2017-02-10T17:49:25Z

Testing in progress...

shaka-bot · 2017-02-10T17:50:54Z

Failure:

+ echo START-BUILD
START-BUILD
+ ./build/all.py
175 files checked, no errors found.

   Config loaded: /var/lib/jenkins/jobs/Manual PR Test (local-tests)/workspace/.htmlhintrc

   Config loaded: /var/lib/jenkins/jobs/Manual PR Test (local-tests)/workspace/.htmlhintrc

   Config loaded: /var/lib/jenkins/jobs/Manual PR Test (local-tests)/workspace/.htmlhintrc

Scanned 3 files, no errors found (23 ms).
/var/lib/jenkins/jobs/Manual PR Test (local-tests)/workspace/demo/asset_section.js:203: ERROR - variable MediaMetadata is undeclared
      navigator.mediaSession.metadata = new MediaMetadata();
                                            ^

1 error(s), 0 warning(s)
Build failed
Generating Closure dependencies...
Running Closure linter...
Running htmlhint...
Checking that the build files are complete...
Checking the tests for type errors...
Build step 'Execute shell' marked build as failure

TheModMaker · 2017-02-10T18:33:15Z

This is a new API and our compiler doesn't know about the types involved. You will need to add an additional file to the externs folder. Simply copy one of them and modify it to describe the new APIs (you don't need to include everything, just what you use).

You can also run build/all.py to check that it works locally.

beaufortfrancois · 2017-02-13T15:23:24Z

I've added missing externs.js files in aa693a9 but it looks like tests are still failing for the play promise:

Checking the tests for type errors...


./externs/mediaelements.js:29: ERROR - variable HTMLMediaElement.prototype.play redefined, original definition at externs.zip//html5.js:1700
HTMLMediaElement.prototype.play = function() {};
^

1 error(s), 0 warning(s)
Build failed

I'd appreciate some help there.

TheModMaker · 2017-02-13T17:25:02Z

externs/mediaelements.js

+/**
+ * @override
+ */
+HTMLMediaElement.prototype.play = function() {};


This file is not needed. This is already known by the compiler (the error is about duplicate definitions).

TheModMaker · 2017-02-13T17:30:20Z

demo/asset_section.js

+      // If video plays successfully, set media session title.
+      playPromise.then(function() {
+        navigator.mediaSession.metadata.title = asset.name;
+      });


How about doing this after player.load() resolves instead?

TheModMaker · 2017-02-13T17:31:45Z

externs/mediasession.js

+
+
+/**
+ * @constructor


Replace with:

/** @type {MediaMetadata} */ Navigator.prototype.mediaSession;

TheModMaker · 2017-02-13T17:33:40Z

externs/mediasession.js

+ */
+var MediaMetadata = function() {};
+
+


You'll need a line like:

/** @type {string} */ MediaMetadata.prototype.title;

TheModMaker · 2017-02-13T17:34:59Z

externs/mediasession.js

+
+/**
+ * @fileoverview Externs for MediaSession based on
+ * {@link https://goo.gl/8QS094i Editor's Draft, 12 January 2017}


Double check this URL, it doesn't work for me.

beaufortfrancois · 2017-02-14T12:08:14Z

@TheModMaker Done! Thanks ;)

shaka-bot · 2017-02-14T17:45:28Z

Testing in progress...

shaka-bot · 2017-02-14T17:54:50Z

All tests passed!

TheModMaker · 2017-02-14T18:15:59Z

Thanks for the PR and thanks for introducing this to us. This will be helpful if we decide to add more UI elements to our demo app.

beaufortfrancois · 2017-02-15T09:17:35Z

Yeah! Please let me know when it's live @TheModMaker!

TheModMaker · 2017-02-15T17:32:20Z

It is already live on the nightly site: https://nightly-dot-shaka-player-demo.appspot.com/demo/. It will appear in the normal demo once we release v2.1.

Support Media Session title in demo

862abe5

mounirlamouri reviewed Feb 9, 2017

View reviewed changes

Check play promise first

78c9186

ismena reviewed Feb 9, 2017

View reviewed changes

Address @ismena feedback

6e0c806

Add missing externs.js files

aa693a9

TheModMaker suggested changes Feb 13, 2017

View reviewed changes

Address TheModMaker@ feedback

3c78af6

TheModMaker approved these changes Feb 14, 2017

View reviewed changes

TheModMaker merged commit bab557d into shaka-project:master Feb 14, 2017

beaufortfrancois deleted the addMediaSession branch February 15, 2017 09:15

TheModMaker pushed a commit that referenced this pull request Feb 24, 2017

Support Media Session title in demo (#689)

db2b0d1

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

Support Media Session title in demo #689

Support Media Session title in demo #689

beaufortfrancois commented Feb 9, 2017 •

edited

Loading

mounirlamouri Feb 9, 2017

beaufortfrancois Feb 9, 2017

mounirlamouri Feb 9, 2017

beaufortfrancois Feb 9, 2017

ismena Feb 9, 2017

beaufortfrancois Feb 10, 2017

beaufortfrancois commented Feb 10, 2017

ismena commented Feb 10, 2017

shaka-bot commented Feb 10, 2017

shaka-bot commented Feb 10, 2017

TheModMaker commented Feb 10, 2017

beaufortfrancois commented Feb 13, 2017

TheModMaker Feb 13, 2017

TheModMaker Feb 13, 2017

TheModMaker Feb 13, 2017

TheModMaker Feb 13, 2017

TheModMaker Feb 13, 2017

beaufortfrancois commented Feb 14, 2017

shaka-bot commented Feb 14, 2017

shaka-bot commented Feb 14, 2017

TheModMaker commented Feb 14, 2017

beaufortfrancois commented Feb 15, 2017

TheModMaker commented Feb 15, 2017

Support Media Session title in demo #689

Support Media Session title in demo #689

Conversation

beaufortfrancois commented Feb 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beaufortfrancois commented Feb 10, 2017

ismena commented Feb 10, 2017

shaka-bot commented Feb 10, 2017

shaka-bot commented Feb 10, 2017

TheModMaker commented Feb 10, 2017

beaufortfrancois commented Feb 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beaufortfrancois commented Feb 14, 2017

shaka-bot commented Feb 14, 2017

shaka-bot commented Feb 14, 2017

TheModMaker commented Feb 14, 2017

beaufortfrancois commented Feb 15, 2017

TheModMaker commented Feb 15, 2017

beaufortfrancois commented Feb 9, 2017 •

edited

Loading