1224973 - Reduced decoder usage for background videos

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[u480271]

Video advertising, such as at www.gyg.com.au, keep playing and consuming resources when in a background tab. Automatically pause these videos when then go to the background. This allows the dormant video behaviour to kick in.

Comment 1

[u480271]

Attachment: Pause muted-videos that are in the background. f=cpearce

Comment 2

[u480271]

Try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=3556e5d469c0

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8687750 [details] [diff] [review]
Pause muted-videos that are in the background. f=cpearce

Review of attachment 8687750 [details] [diff] [review]:
-----------------------------------------------------------------

I think we'd be better to just suspend video decoding of muted videos in background tabs, but keep the current playback position advancing, and if the video is made visible again, seek to the current playback position.

This is because I think we should try to make our optimizations unobservable to script.

Comment 4

[u480271]

(In reply to Chris Pearce (:cpearce) from comment #3)
> Comment on attachment 8687750 [details] [diff] [review]
> Pause muted-videos that are in the background. f=cpearce
> 
> Review of attachment 8687750 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think we'd be better to just suspend video decoding of muted videos in
> background tabs, but keep the current playback position advancing, and if
> the video is made visible again, seek to the current playback position.
> 
> This is because I think we should try to make our optimizations unobservable
> to script.

I'll investigate doing this at the MediaFormatReader level.

Comment 5

[u480271]

Attachment: WIP suspend/resume via MediaFormatReader.

Comment 6

[u480271]

Hi Jean-Yves,

Can you take a look at the WIP patch I've attached. When resuming from suspending, the video I'm test with plays fast to catch up. Have I don't something silly?

Comment 7

[u480271]

Attachment: WIP suspend/resume via MediaFormatReader.

Comment 8

[u480271]

I refreshed the WIP patch. It works better now, but doesn't handle videos that "expire" when the tab is in the background. (eg. https://www.paypal.com/nz/webapps/mpp/home)

Comment 10

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

See also https://www.reddit.com/r/firefox/comments/4563ng/high_cpu_usage_with_res_and_gifvs/

Comment 11

[u480271]

Attachment: MozReview Request: Bug 1224973 - Part 1: Remove MediaDecoderOwner->IsHidden(). r?cpearce, jwwang

Used only by dormant mode calculations, remove IsHidden() and replace
with mIsVisible in MediaDecoder.

Review commit: https://reviewboard.mozilla.org/r/47555/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47555/

Comment 12

[u480271]

Attachment: MozReview Request: Bug 1224973 - Part 2: Set MediaDecoder visibility via NotifyOwnerActivityChanged. r?cpearce, jwwang

Make MediaDecoder::SetElementVisibility private.

Review commit: https://reviewboard.mozilla.org/r/47557/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47557/

Comment 13

[u480271]

Attachment: MozReview Request: Bug 1224973 - Part 3: Plumb element visibility into MDSM. r?jya, jwwang

Change MediaDecoder::mIsVisible to be a Canonical<bool> and plumb
through to the MediaDecoderStateMachine.  This will be used to trigger
suspending the decoding of video frames.

Review commit: https://reviewboard.mozilla.org/r/47559/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47559/

Comment 14

[u480271]

Attachment: MozReview Request: Bug 1224973 - Part 4: Pref media.suspend-bkgnd-video.enabled. r?cpearce, jwwang

Pref to control feature.

Review commit: https://reviewboard.mozilla.org/r/47561/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47561/

Comment 15

[u480271]

Attachment: MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

Suspending decoding of video frames in background tabs (ie. invisible)
is implemented by short-circuiting calls to RequestVideoData in MDSM.
This stops the MFR from decoded new video frames.

To resuming playback near to the correct position (audio is still being
decoded when in the background) video is seeked by invoking a wrapper to
MFR InternalSeek, seeking the the time that results from GetMediaTime.

Review commit: https://reviewboard.mozilla.org/r/47563/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/47563/

Comment 16

[u480271]

Testing these patches on OS X + rMBP + Intel Power Gadget, I saw power usage drop to levels near to that of a paused video when listening to YT videos in background tabs.

Comment 17

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8743091 [details]
MozReview Request: Bug 1224973 - Part 1: Remove MediaDecoderOwner->IsHidden(). r?cpearce, jwwang

https://reviewboard.mozilla.org/r/47555/#review44369

::: dom/media/MediaDecoder.cpp:571
(Diff revision 1)
>                       "MediaDecoder::mDecoderPosition (Canonical)")
>    , mMediaSeekable(AbstractThread::MainThread(), true,
>                     "MediaDecoder::mMediaSeekable (Canonical)")
>    , mMediaSeekableOnlyInBufferedRanges(AbstractThread::MainThread(), false,
>                     "MediaDecoder::mMediaSeekableOnlyInBufferedRanges (Canonical)")
> +  , mIsVisible(true)

It might be better to init the value with mOwner->IsHidden().

Comment 18

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8743092 [details]
MozReview Request: Bug 1224973 - Part 2: Set MediaDecoder visibility via NotifyOwnerActivityChanged. r?cpearce, jwwang

https://reviewboard.mozilla.org/r/47557/#review44371

::: dom/media/MediaDecoder.cpp:1366
(Diff revision 1)
>  }
>  
>  void
> +MediaDecoder::SetElementVisibility(bool aIsVisible)
> +{
> +  mIsVisible = aIsVisible;

assert main thread.

Comment 19

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8743093 [details]
MozReview Request: Bug 1224973 - Part 3: Plumb element visibility into MDSM. r?jya, jwwang

https://reviewboard.mozilla.org/r/47559/#review44373

Comment 20

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8743094 [details]
MozReview Request: Bug 1224973 - Part 4: Pref media.suspend-bkgnd-video.enabled. r?cpearce, jwwang

https://reviewboard.mozilla.org/r/47561/#review44375

Comment 21

[u480271]

https://reviewboard.mozilla.org/r/47555/#review44369

> It might be better to init the value with mOwner->IsHidden().

Good idea.

Comment 22

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

https://reviewboard.mozilla.org/r/47563/#review44377

r+ for MediaDecoderStateMachine.cpp.

::: dom/media/MediaDecoderReader.h:294
(Diff revision 1)
>    // Notified by the OggReader during playback when chained ogg is detected.
>    MediaEventSource<void>& OnMediaNotSeekable() { return mOnMediaNotSeekable; }
>  
> +  // Overrides of this function should fast seek to the time threshold and drop
> +  // all frames up to aTimeThreshold.
> +  virtual void FastVideoSeek(int64_t aTimeThreshold)

Since FastVideoSeek() is not supported by legacy reader, it might be prudent for MDSM to check that before activating stop-video-decoding.

Comment 23

[u480271]

https://reviewboard.mozilla.org/r/47563/#review44377

> Since FastVideoSeek() is not supported by legacy reader, it might be prudent for MDSM to check that before activating stop-video-decoding.

OK. I think you're suggesting adding a predicate that the MDSM can query to find out if fast video seek is available?

I'm also expecting :jya to jump in here and tell me that there's a better way to invoke MFR InternalSeek. :-)

Comment 24

[u480271]

https://reviewboard.mozilla.org/r/47555/#review44369

> Good idea.

Althought this patch removes IsHidden ...

Comment 25

[Jean-Yves Avenard [:jya]]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

https://reviewboard.mozilla.org/r/47563/#review44391

::: dom/media/MediaDecoderReader.h:294
(Diff revision 1)
>    // Notified by the OggReader during playback when chained ogg is detected.
>    MediaEventSource<void>& OnMediaNotSeekable() { return mOnMediaNotSeekable; }
>  
> +  // Overrides of this function should fast seek to the time threshold and drop
> +  // all frames up to aTimeThreshold.
> +  virtual void FastVideoSeek(int64_t aTimeThreshold)

No need to export another symbol. Amend the SeekTarget type to add a new seek type

and hook up MediaFormatReader::Seek to directly call InternalSeek.

You can have MFR::Seek return nullptr there ; with the amended documentation that the new seek is to be used (directly seek and call RequestVideoData)

And please don't use int64_t for time in any new code, let's only use TimeUnit from now on

::: dom/media/MediaDecoderStateMachine.cpp:1346
(Diff revision 1)
>    if (!sSuspendBackgroundVideos) {
>      // Not suspending background videos so there's nothing to do.
>      return;
>    }
> +
> +  if (mIsVisible && mPlayState == MediaDecoder::PLAY_STATE_PLAYING) {

Don't forget to adjust the targetTime (which is ajusted to start at 0) with time used by the reader (need to add StartTime).

And please use the new MediaDecoderReaderWrapper so that fits with JW's work.
The MediaDecoderReaderWrapper will do that adjustment of the time for you, you just need to amend it to add a new wrapper.

That way the MDSM only deals with time in its own referential, and ultimately doesn't need to know about the reader.

Comment 26

[Jean-Yves Avenard [:jya]]

https://reviewboard.mozilla.org/r/47559/#review44393

::: dom/media/MediaDecoder.h:807
(Diff revision 1)
>    Canonical<bool> mMediaSeekable;
>  
>    // True if the media is only seekable within its buffered ranges.
>    Canonical<bool> mMediaSeekableOnlyInBufferedRanges;
>  
> -  bool mIsVisible;
> +  // True if the decoder is visible.

I don't think the comment is very explicit.

IT probably would be clearer is the comment refered to the actual media element being visible.

Comment 27

[u480271]

Comment on attachment 8743091 [details]
MozReview Request: Bug 1224973 - Part 1: Remove MediaDecoderOwner->IsHidden(). r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47555/diff/1-2/

Comment 28

[u480271]

Comment on attachment 8743092 [details]
MozReview Request: Bug 1224973 - Part 2: Set MediaDecoder visibility via NotifyOwnerActivityChanged. r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47557/diff/1-2/

Comment 29

[u480271]

Comment on attachment 8743093 [details]
MozReview Request: Bug 1224973 - Part 3: Plumb element visibility into MDSM. r?jya, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47559/diff/1-2/

Comment 30

[u480271]

Comment on attachment 8743094 [details]
MozReview Request: Bug 1224973 - Part 4: Pref media.suspend-bkgnd-video.enabled. r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47561/diff/1-2/

Comment 31

[u480271]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47563/diff/1-2/

Comment 32

[u480271]

Comment on attachment 8743093 [details]
MozReview Request: Bug 1224973 - Part 3: Plumb element visibility into MDSM. r?jya, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47559/diff/2-3/

Comment 33

[u480271]

Comment on attachment 8743094 [details]
MozReview Request: Bug 1224973 - Part 4: Pref media.suspend-bkgnd-video.enabled. r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47561/diff/2-3/

Comment 34

[u480271]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47563/diff/2-3/

Comment 35

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://reviewboard.mozilla.org/r/47563/#review44697

Will leave the details to review to jya, but some drive by comments...

::: dom/media/MediaDecoderStateMachine.cpp:1347
(Diff revision 3)
>      // Not suspending background videos so there's nothing to do.
>      return;
>    }
> +
> +  if (mIsVisible && mPlayState == MediaDecoder::PLAY_STATE_PLAYING) {
> +    // Transition from invisible to visible - fast seek to the current time by

Don't say "fast seek to the current time", as fast seek means a keyframe seek.

::: dom/media/MediaDecoderStateMachine.cpp:1350
(Diff revision 3)
> +
> +  if (mIsVisible && mPlayState == MediaDecoder::PLAY_STATE_PLAYING) {
> +    // Transition from invisible to visible - fast seek to the current time by
> +    // requesting the video data for the current media time.
> +    int64_t targetTime = GetMediaTime();
> +    DECODER_LOG("Dispatching FastVideoSeek() current time=%lld", targetTime);

Logging mentions fast seek, but this isn't a fast seek.

::: dom/media/SeekTarget.h:26
(Diff revision 3)
>  struct SeekTarget {
>    enum Type {
>      Invalid,
>      PrevSyncPoint,
> -    Accurate
> +    Accurate,
> +    VideoOnly

I think you should call that AccurateVideoOnly, just so that it's obvious...

Comment 36

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743091 [details]
MozReview Request: Bug 1224973 - Part 1: Remove MediaDecoderOwner->IsHidden(). r?cpearce, jwwang

https://reviewboard.mozilla.org/r/47555/#review45065

::: dom/media/MediaDecoder.cpp:329
(Diff revision 2)
>    {
>      return;
>    }
>  
>    DECODER_LOG("UpdateDormantState aTimeout=%d aActivity=%d mIsDormant=%d "
> -              "ownerActive=%d ownerHidden=%d mIsHeuristicDormant=%d "
> +              "ownerActive=%d ownerVisible=%d mIsHeuristicDormant=%d "

"ownerActive=%d mIsVisible=%d mIsHeuristicDormant=%d "

i.e. replace "ownerVisible=%d" with "mIsVisible=%d", to reflect new name.

Comment 37

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743092 [details]
MozReview Request: Bug 1224973 - Part 2: Set MediaDecoder visibility via NotifyOwnerActivityChanged. r?cpearce, jwwang

https://reviewboard.mozilla.org/r/47557/#review45067

::: dom/html/HTMLMediaElement.cpp:2778
(Diff revision 2)
>    mElementInTreeState = ELEMENT_INTREE;
>  
>    if (mDecoder) {
>      // When the MediaElement is binding to tree, the dormant status is
>      // aligned to document's hidden status.
> -    mDecoder->NotifyOwnerActivityChanged();
> +    mDecoder->NotifyOwnerActivityChanged(!IsHidden());

Does this mean that we won't start decoding video for videos created in non-visible tabs? That would mean we won't reach readyState >= HAVE_METADATA for those videos. This means we're making a script-observable change here, right? As script may be expecting readyState > HAVE_METADATA in order to start playback, but never reach there.

We may need to have the MediaDecoderStateMachine::HaveEnoughDecodedVideo() always report that it "has enough" video if the media element isn't visible, in order to unblock this.

Comment 38

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743094 [details]
MozReview Request: Bug 1224973 - Part 4: Pref media.suspend-bkgnd-video.enabled. r?cpearce, jwwang

https://reviewboard.mozilla.org/r/47561/#review45071

Comment 39

[JW Wang [:jwwang] [:jw_wang]]

https://reviewboard.mozilla.org/r/47557/#review45067

> Does this mean that we won't start decoding video for videos created in non-visible tabs? That would mean we won't reach readyState >= HAVE_METADATA for those videos. This means we're making a script-observable change here, right? As script may be expecting readyState > HAVE_METADATA in order to start playback, but never reach there.
> 
> We may need to have the MediaDecoderStateMachine::HaveEnoughDecodedVideo() always report that it "has enough" video if the media element isn't visible, in order to unblock this.

I think the policy is left to MDSM to decide when to stop video decoding (without breaking existing behavior). All HTMLMediaElement and MediaDecoder do is plumb necessary information to MDSM which will determine how to act upon the received info.

Comment 40

[Jean-Yves Avenard [:jya]]

Comment on attachment 8743093 [details]
MozReview Request: Bug 1224973 - Part 3: Plumb element visibility into MDSM. r?jya, jwwang

https://reviewboard.mozilla.org/r/47559/#review45089

Comment 41

[Jean-Yves Avenard [:jya]]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

https://reviewboard.mozilla.org/r/47563/#review45093

::: dom/media/MediaDecoderStateMachine.cpp:1355
(Diff revision 3)
> +    DECODER_LOG("Dispatching FastVideoSeek() current time=%lld", targetTime);
> +    SeekTarget target(targetTime,
> +                      SeekTarget::Type::VideoOnly,
> +                      MediaDecoderEventVisibility::Suppressed);
> +
> +    mReaderWrapper->Seek(target, Duration());

Please open a follow up bug so that seeking follows the same logic as the rest that is you act on the completion of the promise. that's too hacky

::: dom/media/MediaFormatReader.cpp:97
(Diff revision 3)
>  
>    mDemuxerInitRequest.DisconnectIfExists();
>    mMetadataPromise.RejectIfExists(ReadMetadataFailureReason::METADATA_ERROR, __func__);
>    mSeekPromise.RejectIfExists(NS_ERROR_FAILURE, __func__);
>    mSkipRequest.DisconnectIfExists();
>  

1- You need to reject any pending promise there, otherwise a racy shutdown will cause assert.
2- the InternalSeekPromise needs to be happening per track (either audio and video) and must reject that promise in the DecoderData::ResetState()

And also reject those promises in MediaFormatReader::ResetDecode()

::: dom/media/MediaFormatReader.cpp:1049
(Diff revision 3)
>      LOGV("Nothing more to do");
>      return;
>    }
>  
> +  if (aTrack == TrackType::kVideoTrack && mVideo.mError) {
> +    mInternalSeekPromise.RejectIfExists(NS_ERROR_FAILURE, __func__);

can internal seek only be called for video ?

::: dom/media/MediaFormatReader.cpp:1432
(Diff revision 3)
>      return SeekPromise::CreateAndReject(NS_ERROR_FAILURE, __func__);
>    }
>  
> +  if (aTarget.IsVideoOnly()) {
> +    auto internalSeekTarget = InternalSeekTarget(aTarget.GetTime(), false);
> +    InternalSeek(TrackType::kVideoTrack, internalSeekTarget);

The InternalSeek is per track, it would be preferable for the mInternalSeek promise to be set per track

And have methods in the track to facilitate setting and resolving the promise

::: dom/media/SeekTarget.h:25
(Diff revision 3)
>  // or "Fast" (nearest keyframe) seek was requested.
>  struct SeekTarget {
>    enum Type {
>      Invalid,
>      PrevSyncPoint,
> -    Accurate
> +    Accurate,

I'm not a fan of this name, it doesn't really reflect what internalseek does.

it certainly needs some documentation above anyway

Comment 42

[Jean-Yves Avenard [:jya]]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

https://reviewboard.mozilla.org/r/47563/#review45095

Comment 43

[u480271]

Comment on attachment 8743091 [details]
MozReview Request: Bug 1224973 - Part 1: Remove MediaDecoderOwner->IsHidden(). r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47555/diff/2-3/

Comment 44

[u480271]

Comment on attachment 8743092 [details]
MozReview Request: Bug 1224973 - Part 2: Set MediaDecoder visibility via NotifyOwnerActivityChanged. r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47557/diff/2-3/

Comment 45

[u480271]

Comment on attachment 8743093 [details]
MozReview Request: Bug 1224973 - Part 3: Plumb element visibility into MDSM. r?jya, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47559/diff/3-4/

Comment 46

[u480271]

Comment on attachment 8743094 [details]
MozReview Request: Bug 1224973 - Part 4: Pref media.suspend-bkgnd-video.enabled. r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47561/diff/3-4/

Comment 47

[u480271]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47563/diff/3-4/

Comment 48

[u480271]

https://reviewboard.mozilla.org/r/47557/#review45067

> I think the policy is left to MDSM to decide when to stop video decoding (without breaking existing behavior). All HTMLMediaElement and MediaDecoder do is plumb necessary information to MDSM which will determine how to act upon the received info.

This is taken care of by the MDSM.

Comment 49

[u480271]

https://reviewboard.mozilla.org/r/47563/#review44697

> Logging mentions fast seek, but this isn't a fast seek.

Replaced the use of MFR InternalSeek() with MDSM SeekTask/SeekJob, so this isn't needed anymore.

Comment 50

[u480271]

https://reviewboard.mozilla.org/r/47563/#review45093

> Please open a follow up bug so that seeking follows the same logic as the rest that is you act on the completion of the promise. that's too hacky

Now use the MDSM SeekTask/SeekJob to achieve the video only seek.

> 1- You need to reject any pending promise there, otherwise a racy shutdown will cause assert.
> 2- the InternalSeekPromise needs to be happening per track (either audio and video) and must reject that promise in the DecoderData::ResetState()
> 
> And also reject those promises in MediaFormatReader::ResetDecode()

Now use the MDSM SeekTask/SeekJob so don't need to add extra promises.

> The InternalSeek is per track, it would be preferable for the mInternalSeek promise to be set per track
> 
> And have methods in the track to facilitate setting and resolving the promise

Removed the use of InternalSeek() so this is no longer needed.

> I'm not a fan of this name, it doesn't really reflect what internalseek does.
> 
> it certainly needs some documentation above anyway

Changed name to AccurateVideoOnly because that's what it means now when passed to the SeekTask/SeekJob machinery in MDSM.

Comment 51

[u480271]

https://reviewboard.mozilla.org/r/47563/#review44697

> Don't say "fast seek to the current time", as fast seek means a keyframe seek.

Removed all references to "fast seek".

Comment 52

[u480271]

Comment on attachment 8743091 [details]
MozReview Request: Bug 1224973 - Part 1: Remove MediaDecoderOwner->IsHidden(). r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47555/diff/3-4/

Comment 53

[u480271]

Comment on attachment 8743092 [details]
MozReview Request: Bug 1224973 - Part 2: Set MediaDecoder visibility via NotifyOwnerActivityChanged. r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47557/diff/3-4/

Comment 54

[u480271]

Comment on attachment 8743093 [details]
MozReview Request: Bug 1224973 - Part 3: Plumb element visibility into MDSM. r?jya, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47559/diff/4-5/

Comment 55

[u480271]

Comment on attachment 8743094 [details]
MozReview Request: Bug 1224973 - Part 4: Pref media.suspend-bkgnd-video.enabled. r?cpearce, jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47561/diff/4-5/

Comment 56

[u480271]

Comment on attachment 8743095 [details]
MozReview Request: Bug 1224973 - Part 5: Implement suspend decoding for background video. r?jwwang

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47563/diff/4-5/

Comment 57

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8743092 [details]
MozReview Request: Bug 1224973 - Part 2: Set MediaDecoder visibility via NotifyOwnerActivityChanged. r?cpearce, jwwang

https://reviewboard.mozilla.org/r/47557/#review47671

Comment 58

[u480271]

Try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=5bbf42d26071&selectedJob=20647912

Comment 59

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/57b1ec020ca0
https://hg.mozilla.org/integration/mozilla-inbound/rev/7429233bc5e9
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f141a694952
https://hg.mozilla.org/integration/mozilla-inbound/rev/97344d529727
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fe467985f04

Comment 60

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/57b1ec020ca0
https://hg.mozilla.org/mozilla-central/rev/7429233bc5e9
https://hg.mozilla.org/mozilla-central/rev/0f141a694952
https://hg.mozilla.org/mozilla-central/rev/97344d529727
https://hg.mozilla.org/mozilla-central/rev/2fe467985f04

Comment 65

[Lily Kim]

Not sure where to ask, so I will ask here.

Is it possible to have Twitch running in the background without it going into suspended mode freezing the stream? 

I know I can bypass this by toggling the pref off, but it would still be nice to have it on, on my weaker machines.

Comment 66

[Jean-Yves Avenard [:jya]]

What would be the use case for it?

Comment 67

[Lily Kim]

(In reply to Jean-Yves Avenard (On PTO until July 10) [:jya] from comment #66)
Sometimes you just want to listen to streams instead of having them in the foreground all the time, or what if you're watching a stream but also want to browse in the meantime every now and then.

But sorry if I am wrong about this!

Comment 68

[Jean-Yves Avenard [:jya]]

I Believe you misunderstood what suspending background videos means then. Audio still play, video decoding is suspended while the images aren't visible (like the tab is in the background or not selected). As far as the user is concerned it makes no visible difference. You will still here the stream