Closed
Bug 1498246
Opened 7 years ago
Closed 7 years ago
Media API for GeckoView
Categories
(GeckoView :: Media, enhancement, P1)
Tracking
(geckoview62 wontfix, geckoview64 wontfix, firefox62 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 fixed)
RESOLVED
FIXED
mozilla65
People
(Reporter: mortimergoro, Assigned: mortimergoro)
References
Details
(Whiteboard: [geckoview:fxr:p1])
Attachments
(3 files)
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•7 years ago
|
Whiteboard: [geckoview:fxr:p1]
Comment 1•7 years ago
|
||
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.
Updated•7 years ago
|
Assignee: nobody → imanol
Priority: -- → P2
Assignee | ||
Comment 4•7 years ago
|
||
Assignee | ||
Comment 5•7 years ago
|
||
I like that change :) Updated the attachment with the API changes.
Assignee: imanol → nobody
Priority: P2 → --
Assignee | ||
Comment 7•7 years 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?
Comment 8•7 years 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.
(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•7 years ago
|
||
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•7 years ago
|
||
P1 for FxR
status-firefox62:
--- → wontfix
status-firefox63:
--- → wontfix
status-geckoview62:
--- → wontfix
Priority: -- → P1
Comment 11•7 years ago
|
||
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•7 years 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•7 years 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
Comment 14•7 years ago
|
||
The tests for this patch are here: https://bugzilla.mozilla.org/show_bug.cgi?id=1503039
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 15•7 years 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•7 years ago
|
||
Backout by shindli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c9b98dcb148
Backed out changeset 60659171e414 for geckoview bustages CLOSED TREE
Comment 17•7 years ago
|
||
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
Flags: needinfo?(imanol)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(imanol)
Comment 18•7 years ago
|
||
Pushed by rbarker@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6b390c7b979
Add GeckoView Media API r=rbarker,snorp,esawin,jchen,cvan
Comment 19•7 years ago
|
||
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)
Comment 20•7 years ago
|
||
(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•7 years ago
|
Flags: needinfo?(imanol)
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Summary: Media API proposal for GeckoView → Media API for GeckoView
Comment 21•7 years 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•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
Comment 23•7 years ago
|
||
64=wontfix because FxR 1.1 is using GV 65 and this issue doesn't block Focus 8.0 from using GV 64.
Updated•7 years ago
|
Product: Firefox for Android → GeckoView
Updated•7 years ago
|
Target Milestone: Firefox 65 → mozilla65
Comment 25•3 years ago
|
||
Moving some media bugs to the new GeckoView::Media component.
Component: General → Media
You need to log in
before you can comment on or make changes to this bug.
Description
•