From 5112e0435cdbef9bb872bb242750ec75fee4fb53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Mon, 17 Dec 2018 23:50:39 +0100 Subject: [PATCH 1/8] Update servo-media --- Cargo.lock | 10 ++-- components/script/dom/htmlmediaelement.rs | 56 +++++++++++++---------- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7dbe8242f34..19ebb6baca6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3507,7 +3507,7 @@ dependencies = [ [[package]] name = "servo-media" version = "0.1.0" -source = "git+https://github.com/servo/media#35014baeb603b56b1b8b4de75919c58f7390cc30" +source = "git+https://github.com/servo/media#c8cc491c850b18393586500233cc55e29800146c" dependencies = [ "servo-media-audio 0.1.0 (git+https://github.com/servo/media)", "servo-media-gstreamer 0.1.0 (git+https://github.com/servo/media)", @@ -3517,7 +3517,7 @@ dependencies = [ [[package]] name = "servo-media-audio" version = "0.1.0" -source = "git+https://github.com/servo/media#35014baeb603b56b1b8b4de75919c58f7390cc30" +source = "git+https://github.com/servo/media#c8cc491c850b18393586500233cc55e29800146c" dependencies = [ "boxfnonce 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "byte-slice-cast 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3534,7 +3534,7 @@ dependencies = [ [[package]] name = "servo-media-gstreamer" version = "0.1.0" -source = "git+https://github.com/servo/media#35014baeb603b56b1b8b4de75919c58f7390cc30" +source = "git+https://github.com/servo/media#c8cc491c850b18393586500233cc55e29800146c" dependencies = [ "byte-slice-cast 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "glib 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3553,7 +3553,7 @@ dependencies = [ [[package]] name = "servo-media-player" version = "0.1.0" -source = "git+https://github.com/servo/media#35014baeb603b56b1b8b4de75919c58f7390cc30" +source = "git+https://github.com/servo/media#c8cc491c850b18393586500233cc55e29800146c" dependencies = [ "ipc-channel 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.80 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3640,7 +3640,7 @@ dependencies = [ [[package]] name = "servo_media_derive" version = "0.1.0" -source = "git+https://github.com/servo/media#35014baeb603b56b1b8b4de75919c58f7390cc30" +source = "git+https://github.com/servo/media#c8cc491c850b18393586500233cc55e29800146c" dependencies = [ "proc-macro2 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)", "quote 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index b751e176fb4..d397313ee6d 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -1115,6 +1115,24 @@ impl HTMLMediaElement { fn handle_player_event(&self, event: &PlayerEvent) { match *event { + PlayerEvent::EndOfStream => { + // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list + // => "If the media data can be fetched but is found by inspection to be in + // an unsupported format, or can otherwise not be rendered at all" + if self.ready_state.get() < ReadyState::HaveMetadata { + self.queue_dedicated_media_source_failure_steps(); + } + }, + PlayerEvent::Error => { + self.error.set(Some(&*MediaError::new( + &*window_from_node(self), + MEDIA_ERR_DECODE, + ))); + self.upcast::().fire_event(atom!("error")); + }, + PlayerEvent::FrameUpdated => { + self.upcast::().dirty(NodeDamage::OtherNodeDamage); + }, PlayerEvent::MetadataUpdated(ref metadata) => { // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list // => "Once enough of the media data has been fetched to determine the duration..." @@ -1179,6 +1197,12 @@ impl HTMLMediaElement { // XXX Steps 12 and 13 require audio and video tracks support. }, + PlayerEvent::NeedData => { + // XXX(ferjm) implement backoff protocol. + }, + PlayerEvent::EnoughData => { + // XXX(ferjm) implement backoff protocol. + }, PlayerEvent::PositionChanged(position) => { let position = position as f64; let _ = self @@ -1188,25 +1212,6 @@ impl HTMLMediaElement { self.playback_position.set(position); self.time_marches_on(); }, - PlayerEvent::StateChanged(ref state) => match *state { - PlaybackState::Paused => { - if self.ready_state.get() == ReadyState::HaveMetadata { - self.change_ready_state(ReadyState::HaveEnoughData); - } - }, - _ => {}, - }, - PlayerEvent::EndOfStream => { - // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list - // => "If the media data can be fetched but is found by inspection to be in - // an unsupported format, or can otherwise not be rendered at all" - if self.ready_state.get() < ReadyState::HaveMetadata { - self.queue_dedicated_media_source_failure_steps(); - } - }, - PlayerEvent::FrameUpdated => { - self.upcast::().dirty(NodeDamage::OtherNodeDamage); - }, PlayerEvent::SeekData(p) => { self.fetch_request(Some(p)); }, @@ -1221,12 +1226,13 @@ impl HTMLMediaElement { }; ScriptThread::await_stable_state(Microtask::MediaElement(task)); }, - PlayerEvent::Error => { - self.error.set(Some(&*MediaError::new( - &*window_from_node(self), - MEDIA_ERR_DECODE, - ))); - self.upcast::().fire_event(atom!("error")); + PlayerEvent::StateChanged(ref state) => match *state { + PlaybackState::Paused => { + if self.ready_state.get() == ReadyState::HaveMetadata { + self.change_ready_state(ReadyState::HaveEnoughData); + } + }, + _ => {}, }, } } From c8806767a0d4d66669b67b0df71903b05ea5fc70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Mon, 17 Dec 2018 23:57:21 +0100 Subject: [PATCH 2/8] Set media stream as seekable only if the server supports range requests --- components/script/dom/htmlmediaelement.rs | 26 +++++++++++++++++------ 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index d397313ee6d..bbee8f5b33c 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -710,13 +710,6 @@ impl HTMLMediaElement { return; } - // XXX(ferjm) Since we only support Blob for now it is fine to always set - // the stream type to StreamType::Seekable. Once we support MediaStream, - // this should be changed to also consider StreamType::Stream. - if let Err(e) = self.player.set_stream_type(StreamType::Seekable) { - eprintln!("Could not set stream type to Seekable. {:?}", e); - } - // Steps 1-2. // Unapplicable, the `resource` variable already conveys which mode // is in use. @@ -729,6 +722,20 @@ impl HTMLMediaElement { Resource::Url(url) => { // Step 4.remote.1. if self.Preload() == "none" && !self.autoplaying.get() { + // We only set the stream type to Seekable in two cases: + // + // - If the url is a file:// url. Our network stack supports range requests for + // file:// urls. + // - If the url is an http(s):// url and we identify that the server supports + // range requests. + // + // In the remaining cases, we use the default Stream type. + if url.scheme() == "file" { + if let Err(e) = self.player.set_stream_type(StreamType::Seekable) { + warn!("Could not set stream type to Seekable. {:?}", e); + } + } + // Step 4.remote.1.1. self.network_state.set(NetworkState::Idle); @@ -1691,6 +1698,11 @@ impl FetchResponseListener for HTMLMediaElementContext { // header. Otherwise, we get it from the Content-Length header. let content_length = if let Some(content_range) = headers.typed_get::() { + // The server supports range requests, so we can safely set the + // type of stream to Seekable. + if let Err(e) = elem.player.set_stream_type(StreamType::Seekable) { + warn!("Could not set stream type to Seekable. {:?}", e); + } content_range.bytes_len() } else if let Some(content_length) = headers.typed_get::() { Some(content_length.0) From f1d012d782e8c731e6d1aa8db95727d882756fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Tue, 18 Dec 2018 00:02:42 +0100 Subject: [PATCH 3/8] Make sure that we ignore responses from old requests --- components/script/dom/htmlmediaelement.rs | 30 ++++++++++++++--------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index bbee8f5b33c..0c1797c14f3 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -1030,10 +1030,8 @@ impl HTMLMediaElement { } // Step 3. - if self.seeking.get() { - // This will cancel only the sync part of the seek algorithm. - self.generation_id.set(self.generation_id.get() + 1); - } + // The fetch request associated with this seek already takes + // care of cancelling any previous requests. // Step 4. // The flag will be cleared when the media engine tells us the seek was done. @@ -1687,6 +1685,11 @@ impl FetchResponseListener for HTMLMediaElementContext { fn process_response(&mut self, metadata: Result) { let elem = self.elem.root(); + if elem.generation_id.get() != self.generation_id { + // A new fetch request was triggered, so we ignore this response. + return; + } + self.metadata = metadata.ok().map(|m| match m { FetchMetadata::Unfiltered(m) => m, FetchMetadata::Filtered { unsafe_, .. } => unsafe_, @@ -1740,17 +1743,20 @@ impl FetchResponseListener for HTMLMediaElementContext { } fn process_response_chunk(&mut self, payload: Vec) { - if self.ignore_response { - // An error was received previously, skip processing the payload. + let elem = self.elem.root(); + if self.ignore_response || elem.generation_id.get() != self.generation_id { + // An error was received previously or we triggered a new fetch request, + // skip processing the payload. return; } self.bytes_fetched += payload.len(); - let elem = self.elem.root(); // Push input data into the player. if let Err(e) = elem.player.push_data(payload) { - eprintln!("Could not push input data to player {:?}", e); + warn!("Could not push input data to player {:?}", e); + elem.fetch_canceller.borrow_mut().cancel(); + self.ignore_response = true; return; } @@ -1769,9 +1775,10 @@ impl FetchResponseListener for HTMLMediaElementContext { // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list fn process_response_eof(&mut self, status: Result) { let elem = self.elem.root(); - if self.ignore_response { - // An error was received previously, skip processing the payload - // and notify the media backend that we are done pushing data. + if self.ignore_response && elem.generation_id.get() == self.generation_id { + // An error was received previously and no new fetch request was triggered, so + // we skip processing the payload and notify the media backend that we are done + // pushing data. if let Err(e) = elem.player.end_of_stream() { warn!("Could not signal EOS to player {:?}", e); } @@ -1858,6 +1865,7 @@ impl PreInvoke for HTMLMediaElementContext { impl HTMLMediaElementContext { fn new(elem: &HTMLMediaElement, url: ServoUrl) -> HTMLMediaElementContext { + elem.generation_id.set(elem.generation_id.get() + 1); HTMLMediaElementContext { elem: Trusted::new(elem), metadata: None, From e7e390ee8e5bffe974dd2616f8c92ee4b3fb2d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Tue, 18 Dec 2018 10:51:09 +0100 Subject: [PATCH 4/8] Backoff protocol for media fetch requests --- components/script/dom/bindings/trace.rs | 6 +- components/script/dom/htmlmediaelement.rs | 178 ++++++++++++++++------ 2 files changed, 135 insertions(+), 49 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index f94048c47de..75159c875a3 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -38,7 +38,7 @@ use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::bindings::utils::WindowProxyHandler; use crate::dom::document::PendingRestyle; use crate::dom::htmlimageelement::SourceSet; -use crate::dom::htmlmediaelement::MediaFrameRenderer; +use crate::dom::htmlmediaelement::{HTMLMediaElementContext, MediaFrameRenderer}; use crate::task::TaskBox; use app_units::Au; use canvas_traits::canvas::{ @@ -103,7 +103,6 @@ use servo_media::audio::panner_node::{DistanceModel, PanningModel}; use servo_media::audio::param::ParamType; use servo_media::player::Player; use servo_media::Backend; -use servo_media::Error as ServoMediaError; use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl}; use smallvec::SmallVec; use std::cell::{Cell, RefCell, UnsafeCell}; @@ -484,11 +483,12 @@ unsafe_no_jsmanaged_fields!(AudioBuffer); unsafe_no_jsmanaged_fields!(AudioContext); unsafe_no_jsmanaged_fields!(NodeId); unsafe_no_jsmanaged_fields!(AnalysisEngine, DistanceModel, PanningModel, ParamType); -unsafe_no_jsmanaged_fields!(dyn Player); +unsafe_no_jsmanaged_fields!(dyn Player); unsafe_no_jsmanaged_fields!(Mutex); unsafe_no_jsmanaged_fields!(RenderApiSender); unsafe_no_jsmanaged_fields!(ResourceFetchTiming); unsafe_no_jsmanaged_fields!(Timespec); +unsafe_no_jsmanaged_fields!(Mutex); unsafe impl<'a> JSTraceable for &'a str { #[inline] diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 0c1797c14f3..9e9e3dfea0f 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -56,8 +56,7 @@ use net_traits::{CoreResourceMsg, FetchChannels, FetchMetadata, FetchResponseLis use net_traits::{NetworkError, ResourceFetchTiming, ResourceTimingType}; use script_layout_interface::HTMLMediaData; use servo_media::player::frame::{Frame, FrameRenderer}; -use servo_media::player::{PlaybackState, Player, PlayerEvent, StreamType}; -use servo_media::Error as ServoMediaError; +use servo_media::player::{PlaybackState, Player, PlayerError, PlayerEvent, StreamType}; use servo_media::ServoMedia; use servo_url::ServoUrl; use std::cell::Cell; @@ -179,7 +178,7 @@ pub struct HTMLMediaElement { #[ignore_malloc_size_of = "promises are hard"] in_flight_play_promises_queue: DomRefCell]>, ErrorResult)>>, #[ignore_malloc_size_of = "servo_media"] - player: Box>, + player: Box, #[ignore_malloc_size_of = "Arc"] frame_renderer: Arc>, fetch_canceller: DomRefCell, @@ -202,11 +201,12 @@ pub struct HTMLMediaElement { played: Rc>, /// https://html.spec.whatwg.org/multipage/#dom-media-texttracks text_tracks_list: MutNullableDom, - /// Expected content length of the media asset being fetched or played. - content_length: Cell>, /// Time of last timeupdate notification. #[ignore_malloc_size_of = "Defined in time"] next_timeupdate_event: Cell, + /// Latest fetch request context. + #[ignore_malloc_size_of = "Arc"] + current_fetch_request: DomRefCell>>>, } /// @@ -263,8 +263,8 @@ impl HTMLMediaElement { resource_url: DomRefCell::new(None), played: Rc::new(DomRefCell::new(TimeRangesContainer::new())), text_tracks_list: Default::default(), - content_length: Cell::new(None), next_timeupdate_event: Cell::new(time::get_time() + Duration::milliseconds(250)), + current_fetch_request: DomRefCell::new(None), } } @@ -667,10 +667,19 @@ impl HTMLMediaElement { ..RequestInit::default() }; + let mut current_fetch_request = self.current_fetch_request.borrow_mut(); + if let Some(ref current_fetch_request) = *current_fetch_request { + current_fetch_request + .lock() + .unwrap() + .cancel(CancelReason::Overridden); + } let context = Arc::new(Mutex::new(HTMLMediaElementContext::new( self, self.resource_url.borrow().as_ref().unwrap().clone(), + offset.unwrap_or(0), ))); + *current_fetch_request = Some(context.clone()); let (action_sender, action_receiver) = ipc::channel().unwrap(); let window = window_from_node(self); let (task_source, canceller) = window @@ -688,9 +697,10 @@ impl HTMLMediaElement { }), ); // This method may be called the first time we try to fetch the media - // resource or after a seek is requested. In the latter case, we need to - // cancel any previous on-going request. initialize() takes care of - // cancelling previous fetches if any exist. + // resource (from the start or from the last byte fetched before an + // EnoughData event was received) or after a seek is requested. In + // the latter case, we need to cancel any previous on-going request. + // initialize() takes care of cancelling previous fetches if any exist. let cancel_receiver = self.fetch_canceller.borrow_mut().initialize(); let global = self.global(); global @@ -1087,12 +1097,12 @@ impl HTMLMediaElement { task_source.queue_simple_event(self.upcast(), atom!("seeked"), &window); } - fn setup_media_player(&self) -> Result<(), ServoMediaError> { + fn setup_media_player(&self) -> Result<(), PlayerError> { let (action_sender, action_receiver) = ipc::channel().unwrap(); - self.player.register_event_handler(action_sender)?; + self.player.register_event_handler(action_sender); self.player - .register_frame_renderer(self.frame_renderer.clone())?; + .register_frame_renderer(self.frame_renderer.clone()); let trusted_node = Trusted::new(self); let window = window_from_node(self); @@ -1203,10 +1213,32 @@ impl HTMLMediaElement { // XXX Steps 12 and 13 require audio and video tracks support. }, PlayerEvent::NeedData => { - // XXX(ferjm) implement backoff protocol. + // The player needs more data. + // If we already have a valid fetch request, we do nothing. + // Otherwise, if we have no request and the previous request was + // cancelled because we got an EnoughData event, we restart + // fetching where we left. + if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { + let current_fetch_request = current_fetch_request.lock().unwrap(); + match current_fetch_request.cancel_reason() { + Some(ref reason) if *reason == CancelReason::Backoff => { + self.seek(self.playback_position.get(), false) + }, + _ => (), + } + } }, PlayerEvent::EnoughData => { - // XXX(ferjm) implement backoff protocol. + // The player has enough data and it is asking us to stop pushing + // bytes, so we cancel the ongoing fetch request iff we are able + // to restart it from where we left. Otherwise, we continue the + // current fetch request, assuming that some frames will be dropped. + if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { + let mut current_fetch_request = current_fetch_request.lock().unwrap(); + if current_fetch_request.supports_ranges() { + current_fetch_request.cancel(CancelReason::Backoff); + } + } }, PlayerEvent::PositionChanged(position) => { let position = position as f64; @@ -1657,7 +1689,18 @@ enum Resource { Url(ServoUrl), } -struct HTMLMediaElementContext { +/// Indicates the reason why a fetch request was cancelled. +#[derive(Debug, PartialEq)] +enum CancelReason { + /// We were asked to stop pushing data to the player. + Backoff, + /// An error ocurred while fetching the media data. + Error, + /// A new request overrode this one. + Overridden, +} + +pub struct HTMLMediaElementContext { /// The element that initiated the request. elem: Trusted, /// The response metadata received to date. @@ -1666,14 +1709,22 @@ struct HTMLMediaElementContext { generation_id: u32, /// Time of last progress notification. next_progress_event: Timespec, - /// True if this response is invalid and should be ignored. - ignore_response: bool, + /// Some if the request has been cancelled. + cancel_reason: Option, /// timing data for this resource resource_timing: ResourceFetchTiming, /// url for the resource url: ServoUrl, - /// Amount of data fetched. - bytes_fetched: usize, + /// Expected content length of the media asset being fetched or played. + expected_content_length: Option, + /// Number of the last byte fetched from the network for the ongoing + /// request. It is only reset to 0 if we reach EOS. Seek requests + /// set it to the requested position. Requests triggered after an + /// EnoughData event uses this value to restart the download from + /// the last fetched position. + latest_fetched_content: u64, + /// Indicates whether the request support ranges or not. + supports_ranges: bool, } // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list @@ -1701,11 +1752,6 @@ impl FetchResponseListener for HTMLMediaElementContext { // header. Otherwise, we get it from the Content-Length header. let content_length = if let Some(content_range) = headers.typed_get::() { - // The server supports range requests, so we can safely set the - // type of stream to Seekable. - if let Err(e) = elem.player.set_stream_type(StreamType::Seekable) { - warn!("Could not set stream type to Seekable. {:?}", e); - } content_range.bytes_len() } else if let Some(content_length) = headers.typed_get::() { Some(content_length.0) @@ -1714,52 +1760,70 @@ impl FetchResponseListener for HTMLMediaElementContext { }; // We only set the expected input size if it changes. - if content_length != elem.content_length.get() { + if content_length != self.expected_content_length { if let Some(content_length) = content_length { - if let Err(e) = self.elem.root().player.set_input_size(content_length) { + if let Err(e) = elem.player.set_input_size(content_length) { warn!("Could not set player input size {:?}", e); } else { - elem.content_length.set(Some(content_length)); + self.expected_content_length = Some(content_length); } } } } } - let status_is_ok = self + let (status_is_ok, supports_ranges) = self .metadata .as_ref() .and_then(|m| m.status.as_ref()) - .map_or(true, |s| s.0 >= 200 && s.0 < 300); + .map_or((true, false), |s| { + (s.0 >= 200 && s.0 < 300, s.0 == 206 || s.0 == 416) + }); + + if supports_ranges { + // The server supports range requests, + self.supports_ranges = true; + // and we can safely set the type of stream to Seekable. + if let Err(e) = elem.player.set_stream_type(StreamType::Seekable) { + warn!("Could not set stream type to Seekable. {:?}", e); + } + } // => "If the media data cannot be fetched at all..." if !status_is_ok { // Ensure that the element doesn't receive any further notifications // of the aborted fetch. - self.ignore_response = true; - elem.fetch_canceller.borrow_mut().cancel(); + self.cancel(CancelReason::Error); elem.queue_dedicated_media_source_failure_steps(); } } fn process_response_chunk(&mut self, payload: Vec) { let elem = self.elem.root(); - if self.ignore_response || elem.generation_id.get() != self.generation_id { + if self.cancel_reason.is_some() || elem.generation_id.get() != self.generation_id { // An error was received previously or we triggered a new fetch request, // skip processing the payload. return; } - self.bytes_fetched += payload.len(); + let payload_len = payload.len() as u64; // Push input data into the player. if let Err(e) = elem.player.push_data(payload) { + // If we are pushing too much data and we know that we can + // restart the download later from where we left, we cancel + // the current request. Otherwise, we continue the request + // assuming that we may drop some frames. + match e { + PlayerError::EnoughData => self.cancel(CancelReason::Backoff), + _ => (), + } warn!("Could not push input data to player {:?}", e); - elem.fetch_canceller.borrow_mut().cancel(); - self.ignore_response = true; return; } + self.latest_fetched_content += payload_len; + // https://html.spec.whatwg.org/multipage/#concept-media-load-resource step 4, // => "If mode is remote" step 2 if time::get_time() > self.next_progress_event { @@ -1775,17 +1839,19 @@ impl FetchResponseListener for HTMLMediaElementContext { // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list fn process_response_eof(&mut self, status: Result) { let elem = self.elem.root(); - if self.ignore_response && elem.generation_id.get() == self.generation_id { - // An error was received previously and no new fetch request was triggered, so - // we skip processing the payload and notify the media backend that we are done - // pushing data. - if let Err(e) = elem.player.end_of_stream() { - warn!("Could not signal EOS to player {:?}", e); + if elem.generation_id.get() == self.generation_id { + if let Some(CancelReason::Error) = self.cancel_reason { + // An error was received previously and no new fetch request was triggered, so + // we skip processing the payload and notify the media backend that we are done + // pushing data. + if let Err(e) = elem.player.end_of_stream() { + warn!("Could not signal EOS to player {:?}", e); + } + return; } - return; } - if status.is_ok() && self.bytes_fetched != 0 { + if status.is_ok() && self.latest_fetched_content != 0 { if elem.ready_state.get() == ReadyState::HaveNothing { // Make sure that we don't skip the HaveMetadata and HaveCurrentData // states for short streams. @@ -1864,17 +1930,37 @@ impl PreInvoke for HTMLMediaElementContext { } impl HTMLMediaElementContext { - fn new(elem: &HTMLMediaElement, url: ServoUrl) -> HTMLMediaElementContext { + fn new(elem: &HTMLMediaElement, url: ServoUrl, offset: u64) -> HTMLMediaElementContext { elem.generation_id.set(elem.generation_id.get() + 1); HTMLMediaElementContext { elem: Trusted::new(elem), metadata: None, generation_id: elem.generation_id.get(), next_progress_event: time::get_time() + Duration::milliseconds(350), - ignore_response: false, + cancel_reason: None, resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource), url, - bytes_fetched: 0, + expected_content_length: None, + latest_fetched_content: offset, + supports_ranges: false, } } + + fn supports_ranges(&self) -> bool { + self.supports_ranges + } + + fn cancel(&mut self, reason: CancelReason) { + if self.cancel_reason.is_some() { + return; + } + self.cancel_reason = Some(reason); + // XXX(ferjm) move fetch_canceller to context. + let elem = self.elem.root(); + elem.fetch_canceller.borrow_mut().cancel(); + } + + fn cancel_reason(&self) -> &Option { + &self.cancel_reason + } } From 34c1f2587fc4a3e89ed46779e75bf7154b7b0826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Fri, 21 Dec 2018 11:08:37 +0100 Subject: [PATCH 5/8] Add fetch canceller to HTMLMediaElementFetchContext and clarify how we restart after a backoff --- components/script/dom/bindings/trace.rs | 4 +- components/script/dom/htmlmediaelement.rs | 91 ++++++++++++++--------- 2 files changed, 57 insertions(+), 38 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 75159c875a3..c0844394203 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -38,7 +38,7 @@ use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::bindings::utils::WindowProxyHandler; use crate::dom::document::PendingRestyle; use crate::dom::htmlimageelement::SourceSet; -use crate::dom::htmlmediaelement::{HTMLMediaElementContext, MediaFrameRenderer}; +use crate::dom::htmlmediaelement::{HTMLMediaElementFetchContext, MediaFrameRenderer}; use crate::task::TaskBox; use app_units::Au; use canvas_traits::canvas::{ @@ -488,7 +488,7 @@ unsafe_no_jsmanaged_fields!(Mutex); unsafe_no_jsmanaged_fields!(RenderApiSender); unsafe_no_jsmanaged_fields!(ResourceFetchTiming); unsafe_no_jsmanaged_fields!(Timespec); -unsafe_no_jsmanaged_fields!(Mutex); +unsafe_no_jsmanaged_fields!(Mutex); unsafe impl<'a> JSTraceable for &'a str { #[inline] diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 9e9e3dfea0f..4104f8e822f 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -181,7 +181,6 @@ pub struct HTMLMediaElement { player: Box, #[ignore_malloc_size_of = "Arc"] frame_renderer: Arc>, - fetch_canceller: DomRefCell, /// https://html.spec.whatwg.org/multipage/#show-poster-flag show_poster: Cell, /// https://html.spec.whatwg.org/multipage/#dom-media-duration @@ -206,7 +205,7 @@ pub struct HTMLMediaElement { next_timeupdate_event: Cell, /// Latest fetch request context. #[ignore_malloc_size_of = "Arc"] - current_fetch_request: DomRefCell>>>, + current_fetch_request: DomRefCell>>>, } /// @@ -253,7 +252,6 @@ impl HTMLMediaElement { frame_renderer: Arc::new(Mutex::new(MediaFrameRenderer::new( document.window().get_webrender_api_sender(), ))), - fetch_canceller: DomRefCell::new(Default::default()), show_poster: Cell::new(true), duration: Cell::new(f64::NAN), playback_position: Cell::new(0.), @@ -674,11 +672,12 @@ impl HTMLMediaElement { .unwrap() .cancel(CancelReason::Overridden); } - let context = Arc::new(Mutex::new(HTMLMediaElementContext::new( + let (context, cancel_receiver) = HTMLMediaElementFetchContext::new( self, self.resource_url.borrow().as_ref().unwrap().clone(), offset.unwrap_or(0), - ))); + ); + let context = Arc::new(Mutex::new(context)); *current_fetch_request = Some(context.clone()); let (action_sender, action_receiver) = ipc::channel().unwrap(); let window = window_from_node(self); @@ -696,12 +695,6 @@ impl HTMLMediaElement { listener.notify_fetch(message.to().unwrap()); }), ); - // This method may be called the first time we try to fetch the media - // resource (from the start or from the last byte fetched before an - // EnoughData event was received) or after a seek is requested. In - // the latter case, we need to cancel any previous on-going request. - // initialize() takes care of cancelling previous fetches if any exist. - let cancel_receiver = self.fetch_canceller.borrow_mut().initialize(); let global = self.global(); global .core_resource_thread() @@ -908,7 +901,12 @@ impl HTMLMediaElement { task_source.queue_simple_event(self.upcast(), atom!("emptied"), &window); // Step 6.2. - self.fetch_canceller.borrow_mut().cancel(); + if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { + current_fetch_request + .lock() + .unwrap() + .cancel(CancelReason::Error); + } // Step 6.3. // FIXME(nox): Detach MediaSource media provider object. @@ -1222,6 +1220,11 @@ impl HTMLMediaElement { let current_fetch_request = current_fetch_request.lock().unwrap(); match current_fetch_request.cancel_reason() { Some(ref reason) if *reason == CancelReason::Backoff => { + // XXX(ferjm) Ideally we should just create a fetch request from + // where we left. But keeping track of the exact next byte that the + // media backend expects is not the easiest task, so I'm simply + // seeking to the current playback position for now which will create + // a new fetch request for the last rendered frame. self.seek(self.playback_position.get(), false) }, _ => (), @@ -1700,7 +1703,7 @@ enum CancelReason { Overridden, } -pub struct HTMLMediaElementContext { +pub struct HTMLMediaElementFetchContext { /// The element that initiated the request. elem: Trusted, /// The response metadata received to date. @@ -1711,9 +1714,9 @@ pub struct HTMLMediaElementContext { next_progress_event: Timespec, /// Some if the request has been cancelled. cancel_reason: Option, - /// timing data for this resource + /// Timing data for this resource. resource_timing: ResourceFetchTiming, - /// url for the resource + /// Url for the resource. url: ServoUrl, /// Expected content length of the media asset being fetched or played. expected_content_length: Option, @@ -1725,10 +1728,13 @@ pub struct HTMLMediaElementContext { latest_fetched_content: u64, /// Indicates whether the request support ranges or not. supports_ranges: bool, + /// Fetch canceller. Allows cancelling the current fetch request by + /// manually calling its .cancel() method or automatically on Drop. + fetch_canceller: FetchCanceller, } // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list -impl FetchResponseListener for HTMLMediaElementContext { +impl FetchResponseListener for HTMLMediaElementFetchContext { fn process_request_body(&mut self) {} fn process_request_eof(&mut self) {} @@ -1870,7 +1876,12 @@ impl FetchResponseListener for HTMLMediaElementContext { // => "If the connection is interrupted after some media data has been received..." else if elem.ready_state.get() != ReadyState::HaveNothing { // Step 1 - elem.fetch_canceller.borrow_mut().cancel(); + if let Some(ref current_fetch_request) = *elem.current_fetch_request.borrow() { + current_fetch_request + .lock() + .unwrap() + .cancel(CancelReason::Error); + } // Step 2 elem.error.set(Some(&*MediaError::new( @@ -1905,7 +1916,7 @@ impl FetchResponseListener for HTMLMediaElementContext { } } -impl ResourceTimingListener for HTMLMediaElementContext { +impl ResourceTimingListener for HTMLMediaElementFetchContext { fn resource_timing_information(&self) -> (InitiatorType, ServoUrl) { let initiator_type = InitiatorType::LocalName( self.elem @@ -1922,28 +1933,38 @@ impl ResourceTimingListener for HTMLMediaElementContext { } } -impl PreInvoke for HTMLMediaElementContext { +impl PreInvoke for HTMLMediaElementFetchContext { fn should_invoke(&self) -> bool { //TODO: finish_load needs to run at some point if the generation changes. self.elem.root().generation_id.get() == self.generation_id } } -impl HTMLMediaElementContext { - fn new(elem: &HTMLMediaElement, url: ServoUrl, offset: u64) -> HTMLMediaElementContext { +impl HTMLMediaElementFetchContext { + fn new( + elem: &HTMLMediaElement, + url: ServoUrl, + offset: u64, + ) -> (HTMLMediaElementFetchContext, ipc::IpcReceiver<()>) { elem.generation_id.set(elem.generation_id.get() + 1); - HTMLMediaElementContext { - elem: Trusted::new(elem), - metadata: None, - generation_id: elem.generation_id.get(), - next_progress_event: time::get_time() + Duration::milliseconds(350), - cancel_reason: None, - resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource), - url, - expected_content_length: None, - latest_fetched_content: offset, - supports_ranges: false, - } + let mut fetch_canceller = FetchCanceller::new(); + let cancel_receiver = fetch_canceller.initialize(); + ( + HTMLMediaElementFetchContext { + elem: Trusted::new(elem), + metadata: None, + generation_id: elem.generation_id.get(), + next_progress_event: time::get_time() + Duration::milliseconds(350), + cancel_reason: None, + resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource), + url, + expected_content_length: None, + latest_fetched_content: offset, + supports_ranges: false, + fetch_canceller, + }, + cancel_receiver, + ) } fn supports_ranges(&self) -> bool { @@ -1955,9 +1976,7 @@ impl HTMLMediaElementContext { return; } self.cancel_reason = Some(reason); - // XXX(ferjm) move fetch_canceller to context. - let elem = self.elem.root(); - elem.fetch_canceller.borrow_mut().cancel(); + self.fetch_canceller.cancel(); } fn cancel_reason(&self) -> &Option { From 9aced067aa7c5e30e337baf1abd469c24a9a984b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Fri, 21 Dec 2018 11:34:21 +0100 Subject: [PATCH 6/8] Reset seeking flag if stream is not seekable --- components/script/dom/htmlmediaelement.rs | 38 +++++++++-------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 4104f8e822f..166645c7335 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -725,20 +725,6 @@ impl HTMLMediaElement { Resource::Url(url) => { // Step 4.remote.1. if self.Preload() == "none" && !self.autoplaying.get() { - // We only set the stream type to Seekable in two cases: - // - // - If the url is a file:// url. Our network stack supports range requests for - // file:// urls. - // - If the url is an http(s):// url and we identify that the server supports - // range requests. - // - // In the remaining cases, we use the default Stream type. - if url.scheme() == "file" { - if let Err(e) = self.player.set_stream_type(StreamType::Seekable) { - warn!("Could not set stream type to Seekable. {:?}", e); - } - } - // Step 4.remote.1.1. self.network_state.set(NetworkState::Idle); @@ -1059,6 +1045,12 @@ impl HTMLMediaElement { // XXX(ferjm) seekable attribute: we need to get the information about // what's been decoded and buffered so far from servo-media // and add the seekable attribute as a TimeRange. + if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { + if !current_fetch_request.lock().unwrap().is_seekable() { + self.seeking.set(false); + return; + } + } // Step 9. // servo-media with gstreamer does not support inaccurate seeking for now. @@ -1238,7 +1230,7 @@ impl HTMLMediaElement { // current fetch request, assuming that some frames will be dropped. if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { let mut current_fetch_request = current_fetch_request.lock().unwrap(); - if current_fetch_request.supports_ranges() { + if current_fetch_request.is_seekable() { current_fetch_request.cancel(CancelReason::Backoff); } } @@ -1726,8 +1718,8 @@ pub struct HTMLMediaElementFetchContext { /// EnoughData event uses this value to restart the download from /// the last fetched position. latest_fetched_content: u64, - /// Indicates whether the request support ranges or not. - supports_ranges: bool, + /// Indicates whether the stream is seekable. + is_seekable: bool, /// Fetch canceller. Allows cancelling the current fetch request by /// manually calling its .cancel() method or automatically on Drop. fetch_canceller: FetchCanceller, @@ -1778,7 +1770,7 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { } } - let (status_is_ok, supports_ranges) = self + let (status_is_ok, is_seekable) = self .metadata .as_ref() .and_then(|m| m.status.as_ref()) @@ -1786,9 +1778,9 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { (s.0 >= 200 && s.0 < 300, s.0 == 206 || s.0 == 416) }); - if supports_ranges { + if is_seekable { // The server supports range requests, - self.supports_ranges = true; + self.is_seekable = true; // and we can safely set the type of stream to Seekable. if let Err(e) = elem.player.set_stream_type(StreamType::Seekable) { warn!("Could not set stream type to Seekable. {:?}", e); @@ -1960,15 +1952,15 @@ impl HTMLMediaElementFetchContext { url, expected_content_length: None, latest_fetched_content: offset, - supports_ranges: false, + is_seekable: false, fetch_canceller, }, cancel_receiver, ) } - fn supports_ranges(&self) -> bool { - self.supports_ranges + fn is_seekable(&self) -> bool { + self.is_seekable } fn cancel(&mut self, reason: CancelReason) { From da32c8488cc963434465fe73256df29340782003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Fri, 11 Jan 2019 10:44:43 +0100 Subject: [PATCH 7/8] Do not let fetch request drive generation id increments --- components/script/dom/htmlmediaelement.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 166645c7335..d3146f949a5 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -1938,7 +1938,6 @@ impl HTMLMediaElementFetchContext { url: ServoUrl, offset: u64, ) -> (HTMLMediaElementFetchContext, ipc::IpcReceiver<()>) { - elem.generation_id.set(elem.generation_id.get() + 1); let mut fetch_canceller = FetchCanceller::new(); let cancel_receiver = fetch_canceller.initialize(); ( From 9a18074b889b0d1b01e857bbdd2f583b1bbf6db6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fernando=20Jim=C3=A9nez=20Moreno?= Date: Fri, 11 Jan 2019 13:58:48 +0100 Subject: [PATCH 8/8] Split media fetch context and fetch listener to prevent deadlocks --- components/script/dom/bindings/trace.rs | 2 +- components/script/dom/htmlmediaelement.rs | 214 ++++++++++++---------- 2 files changed, 117 insertions(+), 99 deletions(-) diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index c0844394203..e645fb181d5 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -488,7 +488,7 @@ unsafe_no_jsmanaged_fields!(Mutex); unsafe_no_jsmanaged_fields!(RenderApiSender); unsafe_no_jsmanaged_fields!(ResourceFetchTiming); unsafe_no_jsmanaged_fields!(Timespec); -unsafe_no_jsmanaged_fields!(Mutex); +unsafe_no_jsmanaged_fields!(HTMLMediaElementFetchContext); unsafe impl<'a> JSTraceable for &'a str { #[inline] diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index d3146f949a5..64c36f1d017 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -204,8 +204,7 @@ pub struct HTMLMediaElement { #[ignore_malloc_size_of = "Defined in time"] next_timeupdate_event: Cell, /// Latest fetch request context. - #[ignore_malloc_size_of = "Arc"] - current_fetch_request: DomRefCell>>>, + current_fetch_context: DomRefCell>, } /// @@ -262,7 +261,7 @@ impl HTMLMediaElement { played: Rc::new(DomRefCell::new(TimeRangesContainer::new())), text_tracks_list: Default::default(), next_timeupdate_event: Cell::new(time::get_time() + Duration::milliseconds(250)), - current_fetch_request: DomRefCell::new(None), + current_fetch_context: DomRefCell::new(None), } } @@ -665,34 +664,31 @@ impl HTMLMediaElement { ..RequestInit::default() }; - let mut current_fetch_request = self.current_fetch_request.borrow_mut(); - if let Some(ref current_fetch_request) = *current_fetch_request { - current_fetch_request - .lock() - .unwrap() - .cancel(CancelReason::Overridden); + let mut current_fetch_context = self.current_fetch_context.borrow_mut(); + if let Some(ref mut current_fetch_context) = *current_fetch_context { + current_fetch_context.cancel(CancelReason::Overridden); } - let (context, cancel_receiver) = HTMLMediaElementFetchContext::new( + let (fetch_context, cancel_receiver) = HTMLMediaElementFetchContext::new(); + *current_fetch_context = Some(fetch_context); + let fetch_listener = Arc::new(Mutex::new(HTMLMediaElementFetchListener::new( self, self.resource_url.borrow().as_ref().unwrap().clone(), offset.unwrap_or(0), - ); - let context = Arc::new(Mutex::new(context)); - *current_fetch_request = Some(context.clone()); + ))); let (action_sender, action_receiver) = ipc::channel().unwrap(); let window = window_from_node(self); let (task_source, canceller) = window .task_manager() .networking_task_source_with_canceller(); - let listener = NetworkListener { - context, + let network_listener = NetworkListener { + context: fetch_listener, task_source, canceller: Some(canceller), }; ROUTER.add_route( action_receiver.to_opaque(), Box::new(move |message| { - listener.notify_fetch(message.to().unwrap()); + network_listener.notify_fetch(message.to().unwrap()); }), ); let global = self.global(); @@ -887,11 +883,8 @@ impl HTMLMediaElement { task_source.queue_simple_event(self.upcast(), atom!("emptied"), &window); // Step 6.2. - if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { - current_fetch_request - .lock() - .unwrap() - .cancel(CancelReason::Error); + if let Some(ref mut current_fetch_context) = *self.current_fetch_context.borrow_mut() { + current_fetch_context.cancel(CancelReason::Error); } // Step 6.3. @@ -1045,8 +1038,8 @@ impl HTMLMediaElement { // XXX(ferjm) seekable attribute: we need to get the information about // what's been decoded and buffered so far from servo-media // and add the seekable attribute as a TimeRange. - if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { - if !current_fetch_request.lock().unwrap().is_seekable() { + if let Some(ref current_fetch_context) = *self.current_fetch_context.borrow() { + if !current_fetch_context.is_seekable() { self.seeking.set(false); return; } @@ -1208,9 +1201,8 @@ impl HTMLMediaElement { // Otherwise, if we have no request and the previous request was // cancelled because we got an EnoughData event, we restart // fetching where we left. - if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { - let current_fetch_request = current_fetch_request.lock().unwrap(); - match current_fetch_request.cancel_reason() { + if let Some(ref current_fetch_context) = *self.current_fetch_context.borrow() { + match current_fetch_context.cancel_reason() { Some(ref reason) if *reason == CancelReason::Backoff => { // XXX(ferjm) Ideally we should just create a fetch request from // where we left. But keeping track of the exact next byte that the @@ -1228,10 +1220,11 @@ impl HTMLMediaElement { // bytes, so we cancel the ongoing fetch request iff we are able // to restart it from where we left. Otherwise, we continue the // current fetch request, assuming that some frames will be dropped. - if let Some(ref current_fetch_request) = *self.current_fetch_request.borrow() { - let mut current_fetch_request = current_fetch_request.lock().unwrap(); - if current_fetch_request.is_seekable() { - current_fetch_request.cancel(CancelReason::Backoff); + if let Some(ref mut current_fetch_context) = + *self.current_fetch_context.borrow_mut() + { + if current_fetch_context.is_seekable() { + current_fetch_context.cancel(CancelReason::Backoff); } } }, @@ -1685,7 +1678,7 @@ enum Resource { } /// Indicates the reason why a fetch request was cancelled. -#[derive(Debug, PartialEq)] +#[derive(Debug, MallocSizeOf, PartialEq)] enum CancelReason { /// We were asked to stop pushing data to the player. Backoff, @@ -1695,7 +1688,53 @@ enum CancelReason { Overridden, } +#[derive(MallocSizeOf)] pub struct HTMLMediaElementFetchContext { + /// Some if the request has been cancelled. + cancel_reason: Option, + /// Indicates whether the fetched stream is seekable. + is_seekable: bool, + /// Fetch canceller. Allows cancelling the current fetch request by + /// manually calling its .cancel() method or automatically on Drop. + fetch_canceller: FetchCanceller, +} + +impl HTMLMediaElementFetchContext { + fn new() -> (HTMLMediaElementFetchContext, ipc::IpcReceiver<()>) { + let mut fetch_canceller = FetchCanceller::new(); + let cancel_receiver = fetch_canceller.initialize(); + ( + HTMLMediaElementFetchContext { + cancel_reason: None, + is_seekable: false, + fetch_canceller, + }, + cancel_receiver, + ) + } + + fn is_seekable(&self) -> bool { + self.is_seekable + } + + fn set_seekable(&mut self, seekable: bool) { + self.is_seekable = seekable; + } + + fn cancel(&mut self, reason: CancelReason) { + if self.cancel_reason.is_some() { + return; + } + self.cancel_reason = Some(reason); + self.fetch_canceller.cancel(); + } + + fn cancel_reason(&self) -> &Option { + &self.cancel_reason + } +} + +struct HTMLMediaElementFetchListener { /// The element that initiated the request. elem: Trusted, /// The response metadata received to date. @@ -1704,8 +1743,6 @@ pub struct HTMLMediaElementFetchContext { generation_id: u32, /// Time of last progress notification. next_progress_event: Timespec, - /// Some if the request has been cancelled. - cancel_reason: Option, /// Timing data for this resource. resource_timing: ResourceFetchTiming, /// Url for the resource. @@ -1718,15 +1755,10 @@ pub struct HTMLMediaElementFetchContext { /// EnoughData event uses this value to restart the download from /// the last fetched position. latest_fetched_content: u64, - /// Indicates whether the stream is seekable. - is_seekable: bool, - /// Fetch canceller. Allows cancelling the current fetch request by - /// manually calling its .cancel() method or automatically on Drop. - fetch_canceller: FetchCanceller, } // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list -impl FetchResponseListener for HTMLMediaElementFetchContext { +impl FetchResponseListener for HTMLMediaElementFetchListener { fn process_request_body(&mut self) {} fn process_request_eof(&mut self) {} @@ -1780,7 +1812,9 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { if is_seekable { // The server supports range requests, - self.is_seekable = true; + if let Some(ref mut current_fetch_context) = *elem.current_fetch_context.borrow_mut() { + current_fetch_context.set_seekable(true); + } // and we can safely set the type of stream to Seekable. if let Err(e) = elem.player.set_stream_type(StreamType::Seekable) { warn!("Could not set stream type to Seekable. {:?}", e); @@ -1791,18 +1825,25 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { if !status_is_ok { // Ensure that the element doesn't receive any further notifications // of the aborted fetch. - self.cancel(CancelReason::Error); + if let Some(ref mut current_fetch_context) = *elem.current_fetch_context.borrow_mut() { + current_fetch_context.cancel(CancelReason::Error); + } elem.queue_dedicated_media_source_failure_steps(); } } fn process_response_chunk(&mut self, payload: Vec) { let elem = self.elem.root(); - if self.cancel_reason.is_some() || elem.generation_id.get() != self.generation_id { - // An error was received previously or we triggered a new fetch request, - // skip processing the payload. + // If an error was received previously or if we triggered a new fetch request, + // we skip processing the payload. + if elem.generation_id.get() != self.generation_id { return; } + if let Some(ref current_fetch_context) = *elem.current_fetch_context.borrow() { + if current_fetch_context.cancel_reason().is_some() { + return; + } + } let payload_len = payload.len() as u64; @@ -1813,7 +1854,13 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { // the current request. Otherwise, we continue the request // assuming that we may drop some frames. match e { - PlayerError::EnoughData => self.cancel(CancelReason::Backoff), + PlayerError::EnoughData => { + if let Some(ref mut current_fetch_context) = + *elem.current_fetch_context.borrow_mut() + { + current_fetch_context.cancel(CancelReason::Backoff); + } + }, _ => (), } warn!("Could not push input data to player {:?}", e); @@ -1837,15 +1884,17 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { // https://html.spec.whatwg.org/multipage/#media-data-processing-steps-list fn process_response_eof(&mut self, status: Result) { let elem = self.elem.root(); + // If an error was previously received and no new fetch request was triggered, + // we skip processing the payload and notify the media backend that we are done + // pushing data. if elem.generation_id.get() == self.generation_id { - if let Some(CancelReason::Error) = self.cancel_reason { - // An error was received previously and no new fetch request was triggered, so - // we skip processing the payload and notify the media backend that we are done - // pushing data. - if let Err(e) = elem.player.end_of_stream() { - warn!("Could not signal EOS to player {:?}", e); + if let Some(ref current_fetch_context) = *elem.current_fetch_context.borrow() { + if let Some(CancelReason::Error) = current_fetch_context.cancel_reason() { + if let Err(e) = elem.player.end_of_stream() { + warn!("Could not signal EOS to player {:?}", e); + } + return; } - return; } } @@ -1868,11 +1917,8 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { // => "If the connection is interrupted after some media data has been received..." else if elem.ready_state.get() != ReadyState::HaveNothing { // Step 1 - if let Some(ref current_fetch_request) = *elem.current_fetch_request.borrow() { - current_fetch_request - .lock() - .unwrap() - .cancel(CancelReason::Error); + if let Some(ref mut current_fetch_context) = *elem.current_fetch_context.borrow_mut() { + current_fetch_context.cancel(CancelReason::Error); } // Step 2 @@ -1908,7 +1954,7 @@ impl FetchResponseListener for HTMLMediaElementFetchContext { } } -impl ResourceTimingListener for HTMLMediaElementFetchContext { +impl ResourceTimingListener for HTMLMediaElementFetchListener { fn resource_timing_information(&self) -> (InitiatorType, ServoUrl) { let initiator_type = InitiatorType::LocalName( self.elem @@ -1925,52 +1971,24 @@ impl ResourceTimingListener for HTMLMediaElementFetchContext { } } -impl PreInvoke for HTMLMediaElementFetchContext { +impl PreInvoke for HTMLMediaElementFetchListener { fn should_invoke(&self) -> bool { //TODO: finish_load needs to run at some point if the generation changes. self.elem.root().generation_id.get() == self.generation_id } } -impl HTMLMediaElementFetchContext { - fn new( - elem: &HTMLMediaElement, - url: ServoUrl, - offset: u64, - ) -> (HTMLMediaElementFetchContext, ipc::IpcReceiver<()>) { - let mut fetch_canceller = FetchCanceller::new(); - let cancel_receiver = fetch_canceller.initialize(); - ( - HTMLMediaElementFetchContext { - elem: Trusted::new(elem), - metadata: None, - generation_id: elem.generation_id.get(), - next_progress_event: time::get_time() + Duration::milliseconds(350), - cancel_reason: None, - resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource), - url, - expected_content_length: None, - latest_fetched_content: offset, - is_seekable: false, - fetch_canceller, - }, - cancel_receiver, - ) - } - - fn is_seekable(&self) -> bool { - self.is_seekable - } - - fn cancel(&mut self, reason: CancelReason) { - if self.cancel_reason.is_some() { - return; +impl HTMLMediaElementFetchListener { + fn new(elem: &HTMLMediaElement, url: ServoUrl, offset: u64) -> Self { + Self { + elem: Trusted::new(elem), + metadata: None, + generation_id: elem.generation_id.get(), + next_progress_event: time::get_time() + Duration::milliseconds(350), + resource_timing: ResourceFetchTiming::new(ResourceTimingType::Resource), + url, + expected_content_length: None, + latest_fetched_content: offset, } - self.cancel_reason = Some(reason); - self.fetch_canceller.cancel(); - } - - fn cancel_reason(&self) -> &Option { - &self.cancel_reason } }