Bug 1498246

Media API for GeckoView

RESOLVED FIXED in Firefox 65

Get help with this page

Status

Product:

Component:

Type:

enhancement

Importance:

normal

Status:

RESOLVED FIXED

Opened:

9 months ago

Updated:

3 months ago

People

(Reporter: imanol, Assigned: imanol)

Tracking

Version:

Trunk

Target:

mozilla65

Platform:

Unspecified

Android

Blocks:

1503039

Dependency tree / graph

Duplicates:

1487450

Firefox Tracking Flags

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

Tracking Flags:

Tracking

Status

geckoview62

---

wontfix

geckoview64

---

wontfix

firefox62

---

wontfix

firefox63

---

wontfix

firefox64

---

wontfix

firefox65

---

fixed

Details

(Whiteboard: [geckoview:fxr:p1])

Attachments

(3 attachments)

media.txt 9 months ago Imanol Fernandez 3.89 KB, text/plain		Details
media_v2.txt 9 months ago Imanol Fernandez 3.23 KB, text/plain		Details
Bug 1498246 - Add GeckoView Media API 8 months ago Imanol Fernandez 46 bytes, text/x-phabricator-request		Details \| Review

Imanol Fernandez

Assignee

Description

•

9 months ago

Posted file media.txt — Details

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?

Imanol Fernandez

Assignee

Updated

•

9 months ago

Whiteboard: [geckoview:fxr:p1]

Eugen Sawin [:esawin]

Comment 1

•

9 months 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.

James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)

Comment 2

•

9 months ago

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

David Bolter [:davidb] (NeedInfo me for attention)

Updated

•

9 months ago

Assignee: nobody → imanol

Priority: -- → P2

James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)

Updated

•

9 months ago

Duplicate of this bug: 1487450

Imanol Fernandez

Assignee

Comment 4

•

9 months ago

Posted file media_v2.txt — Details

Imanol Fernandez

Assignee

Comment 5

•

9 months ago

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

Assignee: imanol → nobody

Priority: P2 → --

Imanol Fernandez

Assignee

Comment 6

•

9 months ago

btw is

Assignee: nobody → imanol

Imanol Fernandez

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?

Eugen Sawin [:esawin]

Comment 8

•

9 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.

(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.

Eugen Sawin [:esawin]

Comment 9

•

9 months 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.

Chris Peterson [:cpeterson]

Comment 10

•

9 months ago

P1 for FxR

status-firefox62: --- → wontfix

status-firefox63: --- → wontfix

status-geckoview62: --- → wontfix

Priority: -- → P1

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

Comment 11

•

9 months 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?

Imanol Fernandez

Assignee

Comment 12

•

8 months ago

Posted file Bug 1498246 - Add GeckoView Media API — Details

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

Imanol Fernandez

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

Randall Barker [:rbarker]

Updated

•

8 months ago

Blocks: 1503039

Randall Barker [:rbarker]

Comment 14

•

8 months ago

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

Imanol Fernandez

Assignee

Updated

•

8 months ago

Keywords: checkin-needed

Pulsebot

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

Pulsebot

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

Stefan Hindli [:stefan_hindli]

Comment 17

•

8 months 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)

Imanol Fernandez

Assignee

Updated

•

8 months ago

Flags: needinfo?(imanol)

Pulsebot

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

Natalia Csoregi [:nataliaCs]

Comment 19

•

8 months 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)

Natalia Csoregi [:nataliaCs]

Comment 20

•

8 months 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

Imanol Fernandez

Assignee

Updated

•

8 months ago

Flags: needinfo?(imanol)

Imanol Fernandez

Assignee

Updated

•

8 months ago

Keywords: checkin-needed

James Willcox (:snorp) (jwillcox@mozilla.com) (he/him)

Updated

•

8 months ago

Summary: Media API proposal for GeckoView → Media API for GeckoView

Pulsebot

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

Dorel Luca [:dluca]

Comment 22

•

8 months ago

bugherder

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

Status: NEW → RESOLVED

Closed: 8 months ago

status-firefox65: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 65

Chris Peterson [:cpeterson]

Comment 23

•

8 months ago

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

status-firefox64: affected → wontfix

Chris Peterson [:cpeterson]

Comment 24

•

7 months ago

status-geckoview64=wontfix

status-geckoview64: --- → wontfix

BMO Automation

Updated

•

6 months ago

Product: Firefox for Android → GeckoView

Dylan Hardison [:dylan] (he/him)

Updated

•

6 months ago

Target Milestone: Firefox 65 → mozilla65

Tim Guan-tin Chien [:timdream] (please needinfo)

Updated

•

5 months ago

Updated

•

3 months ago