Status

enhancement
P1
normal
RESOLVED FIXED
9 months ago
3 months ago

People

(Reporter: imanol, Assigned: imanol)

Tracking

Trunk
mozilla65
Unspecified
Android
Dependency tree / graph

Firefox Tracking Flags

(geckoview62 wontfix, geckoview64 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 fixed)

Details

(Whiteboard: [geckoview:fxr:p1])

Attachments

(3 attachments)

Assignee

Description

9 months ago
Posted file 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?
Assignee

Updated

9 months ago
Whiteboard: [geckoview:fxr:p1]
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.
Yeah, comment #1 is pretty close to the kind of API I was imagining here.
Assignee: nobody → imanol
Priority: -- → P2
Assignee

Comment 4

9 months ago
Posted file media_v2.txt
Assignee

Comment 5

9 months ago
I like that change :) Updated the attachment with the API changes.
Assignee: imanol → nobody
Priority: P2 → --
Assignee

Comment 6

9 months ago
btw is
Assignee: nobody → imanol
Assignee

Comment 7

9 months ago
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?
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.
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.
P1 for FxR
Priority: -- → P1
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?
Assignee

Comment 12

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

Comment 13

8 months ago
> 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
Blocks: 1503039
Assignee

Updated

8 months ago
Keywords: checkin-needed

Comment 15

8 months ago
Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/60659171e414
Add GeckoView Media API r=rbarker,snorp,esawin,jchen,cvan
Keywords: checkin-needed

Comment 16

8 months ago
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c9b98dcb148
Backed out changeset 60659171e414 for geckoview bustages CLOSED TREE
Assignee

Updated

8 months ago
Flags: needinfo?(imanol)

Comment 18

8 months ago
Pushed by rbarker@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6b390c7b979
Add GeckoView Media API r=rbarker,snorp,esawin,jchen,cvan
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
Flags: needinfo?(imanol)
(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
Assignee

Updated

8 months ago
Flags: needinfo?(imanol)
Assignee

Updated

8 months ago
Keywords: checkin-needed
Summary: Media API proposal for GeckoView → Media API for GeckoView

Comment 21

8 months ago
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c759f171cef
Add GeckoView Media API r=rbarker,snorp,esawin,jchen,cvan
Keywords: checkin-needed

Comment 22

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4c759f171cef
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
64=wontfix because FxR 1.1 is using GV 65 and this issue doesn't block Focus 8.0 from using GV 64.
status-geckoview64=wontfix

Updated

6 months ago
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 65 → mozilla65
You need to log in before you can comment on or make changes to this bug.