Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement mediasession set positon state #24885

Merged
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -240,11 +240,31 @@ pub enum MediaSessionPlaybackState {
Paused,
}

/// https://w3c.github.io/mediasession/#dictdef-mediapositionstate
#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct MediaPositionState {
pub duration: f64,
pub playback_rate: f64,
pub position: f64,
}

impl MediaPositionState {
pub fn new(duration: f64, playback_rate: f64, position: f64) -> Self {
Self {
duration,
playback_rate,
position,
}
}
}
This conversation was marked as resolved by ferjm

This comment has been minimized.

Copy link
@ferjm

ferjm Nov 28, 2019

Member

I believe these do not need to be Options

This comment has been minimized.

Copy link
@shnmorimoto

shnmorimoto Nov 29, 2019

Author Contributor

Thanks, fixed it.


/// Type of events sent from script to the embedder about the media session.
#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum MediaSessionEvent {
/// Indicates that the media metadata is available.
SetMetadata(MediaMetadata),
/// Indicates that the playback state has changed.
PlaybackStateChange(MediaSessionPlaybackState),
/// Indicates that the position state is set.
SetPositionState(MediaPositionState),
}
@@ -67,7 +67,7 @@ use crate::script_thread::ScriptThread;
use crate::task_source::TaskSource;
use dom_struct::dom_struct;
use embedder_traits::resources::{self, Resource as EmbedderResource};
use embedder_traits::{MediaSessionEvent, MediaSessionPlaybackState};
use embedder_traits::{MediaPositionState, MediaSessionEvent, MediaSessionPlaybackState};
use euclid::default::Size2D;
use headers::{ContentLength, ContentRange, HeaderMapExt};
use html5ever::{LocalName, Prefix};
@@ -1780,6 +1780,15 @@ impl HTMLMediaElement {
.add(self.playback_position.get(), position);
self.playback_position.set(position);
self.time_marches_on();
let media_position_state =
MediaPositionState::new(self.duration.get(), self.playbackRate.get(), position);
debug!(
"Sending media session event set position state {:?}",
media_position_state
);
self.send_media_session_event(MediaSessionEvent::SetPositionState(
media_position_state,
));
},
PlayerEvent::SeekData(p, ref seek_lock) => {
self.fetch_request(Some(p), Some(seek_lock.clone()));
@@ -1925,6 +1934,18 @@ impl HTMLMediaElement {

media_session.send_event(event);
}

pub fn set_duration(&self, duration: f64) {
self.duration.set(duration);
}

pub fn reset(&self) {
if let Some(ref player) = *self.player.borrow() {
if let Err(e) = player.lock().unwrap().stop() {
eprintln!("Could not stop player {:?}", e);
}
}
}
}

// XXX Placeholder for [https://github.com/servo/servo/issues/22293]
@@ -9,10 +9,13 @@ use crate::dom::bindings::codegen::Bindings::HTMLMediaElementBinding::HTMLMediaE
use crate::dom::bindings::codegen::Bindings::MediaMetadataBinding::MediaMetadataInit;
use crate::dom::bindings::codegen::Bindings::MediaMetadataBinding::MediaMetadataMethods;
use crate::dom::bindings::codegen::Bindings::MediaSessionBinding;
use crate::dom::bindings::codegen::Bindings::MediaSessionBinding::MediaPositionState;
use crate::dom::bindings::codegen::Bindings::MediaSessionBinding::MediaSessionAction;
use crate::dom::bindings::codegen::Bindings::MediaSessionBinding::MediaSessionActionHandler;
use crate::dom::bindings::codegen::Bindings::MediaSessionBinding::MediaSessionMethods;
use crate::dom::bindings::codegen::Bindings::MediaSessionBinding::MediaSessionPlaybackState;
use crate::dom::bindings::error::{Error, Fallible};
use crate::dom::bindings::num::Finite;
use crate::dom::bindings::reflector::{reflect_dom_object, DomObject, Reflector};
use crate::dom::bindings::root::{DomRoot, MutNullableDom};
use crate::dom::bindings::str::DOMString;
@@ -194,6 +197,62 @@ impl MediaSessionMethods for MediaSession {
None => self.action_handlers.borrow_mut().remove(&action.into()),
};
}

/// https://w3c.github.io/mediasession/#dom-mediasession-setpositionstate
fn SetPositionState(&self, state: &MediaPositionState) -> Fallible<()> {
// If the state is an empty dictionary then clear the position state.
if state.duration.is_none() && state.position.is_none() && state.playbackRate.is_none() {
This conversation was marked as resolved by jdm

This comment has been minimized.

Copy link
@shnmorimoto

shnmorimoto Nov 27, 2019

Author Contributor

https://w3c.github.io/mediasession/#dom-mediasession-setpositionstate

If the state is an empty dictionary then clear the position state.

Is the position state the combination of duration and playback_position and playbackRate in media_instance(type is HTMLMediaElement)?
And, Does clear mean to set at 0?

This comment has been minimized.

Copy link
@ferjm

ferjm Nov 28, 2019

Member

That's my understanding reading the spec.

Rather than setting these values to 0, I would expose a reset method in HTMLMediaElement which should call stop in the inner player. Like

if let Err(e) = player.lock().unwrap().stop() {

This comment has been minimized.

Copy link
@shnmorimoto

shnmorimoto Nov 29, 2019

Author Contributor

Thanks! I added the reset method, and fixed to use it.

if let Some(media_instance) = self.media_instance.get() {
media_instance.reset();
}
return Ok(());
}

// If the duration is not present or its value is null, throw a TypeError.
if state.duration.is_none() {
return Err(Error::Type(
"duration is not present or its value is null".to_owned(),
));
}

// If the duration is negative, throw a TypeError.
if let Some(state_duration) = state.duration {
if *state_duration < 0.0 {
return Err(Error::Type("duration is negative".to_owned()));
}
}

// If the position is negative or greater than duration, throw a TypeError.
if let Some(state_position) = state.position {
if *state_position < 0.0 {
return Err(Error::Type("position is negative".to_owned()));
}
if let Some(state_duration) = state.duration {
if *state_position > *state_duration {
return Err(Error::Type("position is greater than duration".to_owned()));
}
}
}

// If the playbackRate is zero throw a TypeError.
if let Some(state_playback_rate) = state.playbackRate {
if *state_playback_rate <= 0.0 {
return Err(Error::Type("playbackRate is zero".to_owned()));
}
}

// Update the position state and last position updated time.
if let Some(media_instance) = self.media_instance.get() {
media_instance.set_duration(state.duration.map(|v| *v).unwrap());
// If the playbackRate is not present or its value is null, set it to 1.0.
let _ =
media_instance.SetPlaybackRate(state.playbackRate.unwrap_or(Finite::wrap(1.0)))?;
// If the position is not present or its value is null, set it to zero.
media_instance.SetCurrentTime(state.position.unwrap_or(Finite::wrap(0.0)));
}

Ok(())
}
}

impl From<MediaSessionAction> for MediaSessionActionType {
@@ -42,6 +42,12 @@ dictionary MediaSessionSeekToActionDetails : MediaSessionActionDetails {
boolean? fastSeek;
};

dictionary MediaPositionState {
double duration;
double playbackRate;
double position;
};

callback MediaSessionActionHandler = void(/*MediaSessionActionDetails details*/);

[Exposed=Window]
@@ -52,6 +58,5 @@ interface MediaSession {

void setActionHandler(MediaSessionAction action, MediaSessionActionHandler? handler);

//void setPositionState(optional MediaPositionState? state);
[Throws] void setPositionState(optional MediaPositionState state = {});
};

@@ -132,8 +132,10 @@ pub trait HostTrait {
fn set_clipboard_contents(&self, contents: String);
/// Called when we get the media session metadata/
fn on_media_session_metadata(&self, title: String, artist: String, album: String);
/// Called when the media sessoin playback state changes.
/// Called when the media session playback state changes.
fn on_media_session_playback_state_change(&self, state: i32);
/// Called when the media session position state is set.
fn on_media_session_set_position_state(&self, duration: f64, position: f64, playback_rate: f64);
}

pub struct ServoGlue {
@@ -594,6 +596,14 @@ impl ServoGlue {
.callbacks
.host_callbacks
.on_media_session_playback_state_change(state as i32),
MediaSessionEvent::SetPositionState(position_state) => self
.callbacks
.host_callbacks
.on_media_session_set_position_state(
position_state.duration,
position_state.position,
position_state.playback_rate,
),
};
},
EmbedderMsg::Status(..) |
@@ -219,6 +219,8 @@ pub struct CHostCallbacks {
pub on_media_session_metadata:
extern "C" fn(title: *const c_char, album: *const c_char, artist: *const c_char),
pub on_media_session_playback_state_change: extern "C" fn(state: i32),
pub on_media_session_set_position_state:
extern "C" fn(duration: f64, position: f64, playback_rate: f64),
}

/// Servo options
@@ -648,7 +650,7 @@ impl HostTrait for HostCallbacks {
}

fn on_load_started(&self) {
debug!("on_load_ended");
debug!("on_load_started");
This conversation was marked as resolved by ferjm

This comment has been minimized.

Copy link
@ferjm

ferjm Nov 28, 2019

Member

Thanks!

(self.0.on_load_started)();
}

@@ -727,4 +729,17 @@ impl HostTrait for HostCallbacks {
debug!("on_media_session_playback_state_change {:?}", state);
(self.0.on_media_session_playback_state_change)(state);
}

fn on_media_session_set_position_state(
&self,
duration: f64,
position: f64,
playback_rate: f64,
) {
debug!(
"on_media_session_set_position_state ({:?} {:?} {:?})",
duration, position, playback_rate
);
(self.0.on_media_session_set_position_state)(duration, position, playback_rate);
}
}
@@ -560,6 +560,30 @@ impl HostTrait for HostCallbacks {
)
.unwrap();
}

fn on_media_session_set_position_state(
&self,
duration: f64,
position: f64,
playback_rate: f64,
) {
info!(
"on_media_session_playback_state_change ({:?}, {:?}, {:?})",
duration, position, playback_rate
);
let env = self.jvm.get_env().unwrap();
let duration = JValue::Float(duration as jfloat);
let position = JValue::Float(position as jfloat);
let playback_rate = JValue::Float(playback_rate as jfloat);

env.call_method(
self.callbacks.as_obj(),
"onMediaSessionSetPositionState",
"(FFF)V",
&[duration, position, playback_rate],
)
.unwrap();
}
}

fn initialize_android_glue(env: &JNIEnv, activity: JObject) {
@@ -255,4 +255,15 @@ public void onMediaSessionPlaybackStateChange(int state) {
return;
}
}

@Override
public void onMediaSessionSetPositionState(float duration, float position, float playbackRate) {
Log.d("onMediaSessionSetPositionState", duration + " " + position + " " + playbackRate);
if (mMediaSession == null) {
mMediaSession = new MediaSession(mServoView, this, getApplicationContext());
}

mMediaSession.setPositionState(duration, position, playbackRate);
This conversation was marked as resolved by ferjm

This comment has been minimized.

Copy link
@shnmorimoto

shnmorimoto Dec 1, 2019

Author Contributor

I have a wrong implementation.(Because setPositionState is not implemented in MediaSession.java.)
Should I call onMediaSessionSetPositionState of Servo.java directly here?
Or is it better to implement setPositionState in MediaSession.java and call it there?

This comment has been minimized.

Copy link
@ferjm

ferjm Dec 2, 2019

Member

Servo.onMediaSessionSetPositionState is what calls this method (note that MainActivity implements the Servo.Client interface).

We need to implement MediaSession.setPositionState. That's where we should modify the existing notification UI to add the current position, the total duration and a progress bar. If you don't feel like hacking on the Android UI, leave the method unimplemented.

This comment has been minimized.

Copy link
@shnmorimoto

shnmorimoto Dec 2, 2019

Author Contributor

Thanks!
I don't have a good understanding of the Android UI.
So this time I will not implement it. Sorry.

return;
}
}
@@ -192,4 +192,8 @@ public void updateMetadata(String title, String artist, String album) {
showMediaSessionControls();
}
}
}

// Not implemented
// see https://github.com/servo/servo/pull/24885#discussion_r352496117
public void setPositionState(float duration, float position, float playbackRate) {}
This conversation was marked as resolved by ferjm

This comment has been minimized.

Copy link
@shnmorimoto

shnmorimoto Dec 2, 2019

Author Contributor

For compile, I added only the signature.

}
@@ -115,6 +115,8 @@
void onMediaSessionMetadata(String title, String artist, String album);

void onMediaSessionPlaybackStateChange(int state);

void onMediaSessionSetPositionState(float duration, float position, float playbackRate);
}
}

@@ -192,6 +192,8 @@ public void mediaSessionAction(int action) {
void onMediaSessionMetadata(String title, String artist, String album);

void onMediaSessionPlaybackStateChange(int state);

void onMediaSessionSetPositionState(float duration, float position, float playbackRate);
}

public interface RunCallback {
@@ -285,5 +287,9 @@ public void onMediaSessionMetadata(String title, String artist, String album) {
public void onMediaSessionPlaybackStateChange(int state) {
mRunCallback.inUIThread(() -> mClient.onMediaSessionPlaybackStateChange(state));
}

public void onMediaSessionSetPositionState(float duration, float position, float playbackRate) {
mRunCallback.inUIThread(() -> mClient.onMediaSessionSetPositionState(duration, position, playbackRate));
}
}
}
@@ -1,13 +1,4 @@
[idlharness.window.html]
[MediaSession interface: calling setPositionState(MediaPositionState) on navigator.mediaSession with too few arguments must throw TypeError]
expected: FAIL

[MediaSession interface: navigator.mediaSession must inherit property "setPositionState(MediaPositionState)" with the proper type]
expected: FAIL

[MediaSession interface: operation setPositionState(MediaPositionState)]
expected: FAIL

[MediaMetadata interface: attribute artwork]
expected: FAIL

This file was deleted.

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.