1498246 - Media API for GeckoView

Status: RESOLVED FIXED

(GeckoView :: Media, enhancement, P1)

Description

[Imanol Fernandez]

Attachment: media.txt

I created this bug to discuss the Media API we want to include in GeckoView.

Some context: On Firefox Reality we need a way to control playing media in addition to know some media events and metadata. This is required because a lot of 360/180 video websites (or just a raw video) do not have a WebVR player. Browsers are getting around this by allowing the user to map the video onto a sphere or half sphere and while the video is playing the controls are warped as well. So to allow the user to still control the video, they are given an external controller with play/pause/stop/volume setter and a playback seek bar. Standalone Oculus, Chrome and Samsung browsers have this capability.


Attached you can find a proposal for the GeckoView Media API.

What do you think?

Comment 1

[Eugen Sawin [:esawin]]

What about introducing a MediaElement class instead of referencing media elements by source?
The API could look like this:

    MediaDelegate {
      // This will set the returned delegate for the media element if we're interested in the events.
      GeckoResult<MediaElementDelegate> onNewMedia(MediaElement element)
    }

    MediaElementDelegate {
        void onMetadataChange(Metadata metadata)
        void onVolumeChange(double volume)
        ...
    }
    
    MediaElement {
        void pause()
        void seek(double time)
        void setVolume(double volume)
        ...
    }

Also note that we're in the process of replacing boolean return values with more descriptive values in bug 1495786.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

Yeah, comment #1 is pretty close to the kind of API I was imagining here.

Comment 4

[Imanol Fernandez]

Attachment: media_v2.txt

Comment 5

[Imanol Fernandez]

I like that change :) Updated the attachment with the API changes.

Comment 6

[Imanol Fernandez]

btw is

Comment 7

[Imanol Fernandez]

btw what about adding getters to the MediaElement to get the current state? e.g. getUrl(), getCurrentTime(), getDuration(), etc

They are not implemented for a GeckoSession (e.g. no getURL(), getTitle() and equivalent methods). Do we want use the delegates for everything here too?

Comment 8

[Eugen Sawin [:esawin]]

LGTM. I'm not sure about MediaElement.setDelegate. We could either allow or require the user to set the delegate manually, or we remove that option and set the delegate implicitly through the return value of onNewMedia.

(In reply to Imanol Fernandez from comment #7)
> btw what about adding getters to the MediaElement to get the current state?
> e.g. getUrl(), getCurrentTime(), getDuration(), etc
> 
> They are not implemented for a GeckoSession (e.g. no getURL(), getTitle()
> and equivalent methods). Do we want use the delegates for everything here
> too?

So far we've preferred to avoid keeping state in GeckoSession and to delegate that to the app, e.g., see onCanGoBack.

Comment 9

[Eugen Sawin [:esawin]]

The last comment was referring to the (missing) GeckoSession getters, it makes more sense to have them for MediaElement since most of this values are static and not something that the use will get through a delegate callback.

Comment 10

[Chris Peterson [:cpeterson]]

P1 for FxR

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Is there a callback for play state change? (e.g. if script stops a video we should get a callback). Also what happens when media elements are unloaded (through navigation, etc.), do we get callbacks for that?

Comment 12

[Imanol Fernandez]

Attachment: Bug 1498246 - Add GeckoView Media API

Add GeckoView Media API which provides a way to listen to HTMLMediaElement events in a GeckoSession and control the playback externally

Comment 13

[Imanol Fernandez]

> LGTM. I'm not sure about MediaElement.setDelegate. We could either allow or require the user to set the delegate manually, or we remove that option and set the delegate implicitly through the return value of onNewMedia.

I decided to change the API. It no longer returns the delegate in onNewMedia GeckoResult<> to register the observer. Now the delegate can be automatically set in the HTMLMediaElement and it automatically notifies to JS to observe or unobserve (via null) the associated HTMLMediaElement. The good thing is that it provides more control to the user, and you can start/stop observing a specific video when you want.

> Is there a callback for play state change? 

Yes, I thought it was already added in the spec :D

> Also what happens when media elements are unloaded (through navigation, etc.), do we get callbacks for that?

I added callbacks for video unload when there is a navigation. Also added a callback when a video whets unloaded in the same page because it's removed from the DOM

Comment 14

[Randall Barker [:rbarker]]

The tests for this patch are here: https://bugzilla.mozilla.org/show_bug.cgi?id=1503039

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60659171e414
Add GeckoView Media API r=rbarker,snorp,esawin,jchen,cvan

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5c9b98dcb148
Backed out changeset 60659171e414 for geckoview bustages CLOSED TREE

Comment 17

[Stefan Hindli [:stefan_hindli]]

Backed out changeset 60659171e414 (Bug 1498246) for geckoview bustages CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=210564167
https://treeherder.mozilla.org/logviewer.html#?job_id=210564167&repo=autoland&lineNumber=39494

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6b390c7b979
Add GeckoView Media API r=rbarker,snorp,esawin,jchen,cvan

Comment 19

[Natalia Csoregi [:nataliaCs]]

Backed out for org.mozilla.geckoview.test.GeckoSessionTestRuleTest.includesAllCallbacks failures

backout:

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=gv-junit&revision=a6b390c7b9791dfa760f9b09248ff84bd2a4c3e6&group_state=expanded&selectedJob=210690077

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=210690077&repo=autoland&lineNumber=2498

task 2018-11-08T23:57:52.472Z] 23:57:52     INFO -  TEST-START | org.mozilla.geckoview.test.GeckoSessionTestRuleTest.includesAllCallbacks
[task 2018-11-08T23:57:53.469Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2018-11-08T23:57:53.469Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=71
[task 2018-11-08T23:57:53.471Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.GeckoSessionTestRuleTest
[task 2018-11-08T23:57:53.472Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2018-11-08T23:57:53.474Z] 23:57:53     INFO -  org.mozilla.geckoview.test | Error in includesAllCallbacks(org.mozilla.geckoview.test.GeckoSessionTestRuleTest):
[task 2018-11-08T23:57:53.475Z] 23:57:53     INFO -  org.mozilla.geckoview.test | java.lang.AssertionError: Callbacks.All should include interface MediaDelegate
[task 2018-11-08T23:57:53.475Z] 23:57:53     INFO -  org.mozilla.geckoview.test | Expected: <true>
[task 2018-11-08T23:57:53.476Z] 23:57:53     INFO -  org.mozilla.geckoview.test |      but: was <false>
[task 2018-11-08T23:57:53.476Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
[task 2018-11-08T23:57:53.477Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.Assert.assertThat(Assert.java:956)
[task 2018-11-08T23:57:53.478Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.checkThat(GeckoSessionTestRule.java:881)
[task 2018-11-08T23:57:53.478Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.BaseSessionTest.assertThat(BaseSessionTest.kt:53)
[task 2018-11-08T23:57:53.479Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.GeckoSessionTestRuleTest.includesAllCallbacks(GeckoSessionTestRuleTest.kt:79)
[task 2018-11-08T23:57:53.479Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invokeNative(Native Method)
[task 2018-11-08T23:57:53.480Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Method.java:525)
[task 2018-11-08T23:57:53.480Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
[task 2018-11-08T23:57:53.481Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
[task 2018-11-08T23:57:53.482Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
[task 2018-11-08T23:57:53.482Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
[task 2018-11-08T23:57:53.483Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$3$1.run(GeckoSessionTestRule.java:1383)
[task 2018-11-08T23:57:53.483Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1719)
[task 2018-11-08T23:57:53.484Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.handleCallback(Handler.java:730)
[task 2018-11-08T23:57:53.484Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.dispatchMessage(Handler.java:92)
[task 2018-11-08T23:57:53.485Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.os.Looper.loop(Looper.java:137)
[task 2018-11-08T23:57:53.485Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.app.ActivityThread.main(ActivityThread.java:5103)
[task 2018-11-08T23:57:53.486Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invokeNative(Native Method)
[task 2018-11-08T23:57:53.486Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Method.java:525)
[task 2018-11-08T23:57:53.487Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
[task 2018-11-08T23:57:53.487Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
[task 2018-11-08T23:57:53.487Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at dalvik.system.NativeStart.main(Native Method)
[task 2018-11-08T23:57:53.488Z] 23:57:53     INFO -  org.mozilla.geckoview.test |
[task 2018-11-08T23:57:53.488Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=208
[task 2018-11-08T23:57:53.489Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stack=java.lang.AssertionError: Callbacks.All should include interface MediaDelegate
[task 2018-11-08T23:57:53.489Z] 23:57:53     INFO -  org.mozilla.geckoview.test | Expected: <true>
[task 2018-11-08T23:57:53.490Z] 23:57:53     INFO -  org.mozilla.geckoview.test |      but: was <false>
[task 2018-11-08T23:57:53.490Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
[task 2018-11-08T23:57:53.491Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.Assert.assertThat(Assert.java:956)
[task 2018-11-08T23:57:53.491Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.checkThat(GeckoSessionTestRule.java:881)
[task 2018-11-08T23:57:53.492Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.BaseSessionTest.assertThat(BaseSessionTest.kt:53)
[task 2018-11-08T23:57:53.492Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.GeckoSessionTestRuleTest.includesAllCallbacks(GeckoSessionTestRuleTest.kt:79)
[task 2018-11-08T23:57:53.492Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invokeNative(Native Method)
[task 2018-11-08T23:57:53.493Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Method.java:525)
[task 2018-11-08T23:57:53.493Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
[task 2018-11-08T23:57:53.494Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
[task 2018-11-08T23:57:53.494Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
[task 2018-11-08T23:57:53.495Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
[task 2018-11-08T23:57:53.495Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$3$1.run(GeckoSessionTestRule.java:1383)
[task 2018-11-08T23:57:53.496Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1719)
[task 2018-11-08T23:57:53.496Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.handleCallback(Handler.java:730)
[task 2018-11-08T23:57:53.497Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.os.Handler.dispatchMessage(Handler.java:92)
[task 2018-11-08T23:57:53.497Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.os.Looper.loop(Looper.java:137)
[task 2018-11-08T23:57:53.498Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at android.app.ActivityThread.main(ActivityThread.java:5103)
[task 2018-11-08T23:57:53.498Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invokeNative(Native Method)
[task 2018-11-08T23:57:53.499Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at java.lang.reflect.Method.invoke(Method.java:525)
[task 2018-11-08T23:57:53.499Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737)
[task 2018-11-08T23:57:53.499Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
[task 2018-11-08T23:57:53.500Z] 23:57:53     INFO -  org.mozilla.geckoview.test | 	at dalvik.system.NativeStart.main(Native Method)
[task 2018-11-08T23:57:53.500Z] 23:57:53     INFO -  org.mozilla.geckoview.test |
[task 2018-11-08T23:57:53.501Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=includesAllCallbacks
[task 2018-11-08T23:57:53.501Z] 23:57:53     INFO -  org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
[task 2018-11-08T23:57:53.502Z] 23:57:53  WARNING -  TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.GeckoSessionTestRuleTest.includesAllCallbacks | status -2
[task 2018-11-08T23:57:53.502Z] 23:57:53     INFO -  TEST-INFO took 1007ms

Comment 20

[Natalia Csoregi [:nataliaCs]]

(In reply to Natalia Csoregi [:nataliaCs] from comment #19)
> Backed out for
> org.mozilla.geckoview.test.GeckoSessionTestRuleTest.includesAllCallbacks
> failures
> 
> backout:
> 
https://hg.mozilla.org/integration/autoland/rev/758408e4e612af2a86f1cf8a50a77604a923db0e

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4c759f171cef
Add GeckoView Media API r=rbarker,snorp,esawin,jchen,cvan

Comment 22

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/4c759f171cef

Comment 23

[Chris Peterson [:cpeterson]]

64=wontfix because FxR 1.1 is using GV 65 and this issue doesn't block Focus 8.0 from using GV 64.

Comment 24

[Chris Peterson [:cpeterson]]

status-geckoview64=wontfix

Comment 25

[Chris Peterson [:cpeterson]]

Moving some media bugs to the new GeckoView::Media component.