Closed
Bug 1346120
Opened 8 years ago
Closed 8 years ago
If a video element's video is suspended, keep the last decoded video frame on the screen
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: kaku, Assigned: kaku)
References
(Blocks 3 open bugs)
Details
Attachments
(6 files)
(deleted),
text/x-review-board-request
|
jwwang
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jwwang
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jwwang
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jwwang
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jwwang
:
review+
|
Details |
(deleted),
text/x-review-board-request
|
jwwang
:
review+
|
Details |
Spawn from bug 1309492 comment 15.
Assignee | ||
Updated•8 years ago
|
Assignee | ||
Comment 1•8 years ago
|
||
From bug 1305338 comment 4:
The latest UX spec: https://mozilla.invisionapp.com/share/K48PCVSEM
Assignee | ||
Comment 2•8 years ago
|
||
The upcoming patches are basically from patches-7,8,9,A of Bug 1295921 with the need of the UX spec.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 9•8 years ago
|
||
mozreview-review |
Comment on attachment 8845754 [details]
Bug 1346120 part 1 - Extract BlankMediaDataDecoder so it can be shared;
https://reviewboard.mozilla.org/r/118904/#review120868
::: dom/media/platforms/agnostic/BlankDecoderModule.cpp:7
(Diff revision 1)
> /* vim:set ts=2 sw=2 sts=2 et cindent: */
> /* This Source Code Form is subject to the terms of the Mozilla Public
> * License, v. 2.0. If a copy of the MPL was not distributed with this
> * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>
> +#include "DummyMediaDataDecoder.h"
The sorting should be case sensitive. Please put paths starting with upper-case characters together.
Attachment #8845754 -
Flags: review?(jwwang) → review+
Comment 10•8 years ago
|
||
mozreview-review |
Comment on attachment 8845755 [details]
Bug 1346120 part 2 - Implement NullDecoderModule;
https://reviewboard.mozilla.org/r/118906/#review120870
Attachment #8845755 -
Flags: review?(jwwang) → review+
Comment 11•8 years ago
|
||
mozreview-review |
Comment on attachment 8845756 [details]
Bug 1346120 part 3 - Use NullDecoderModule while suspending a video element's decoder;
https://reviewboard.mozilla.org/r/118908/#review120872
Attachment #8845756 -
Flags: review?(jwwang) → review+
Comment 12•8 years ago
|
||
mozreview-review |
Comment on attachment 8845757 [details]
Bug 1346120 part 4 - Only set ImageContainer if there are valid new images in VideoSink::RenderVideoFrames();
https://reviewboard.mozilla.org/r/118910/#review120874
::: dom/media/mediasink/VideoSink.cpp:404
(Diff revision 1)
>
> VSINK_LOG_V("playing video frame %" PRId64 " (id=%x) (vq-queued=%" PRIuSIZE ")",
> frame->mTime, frame->mFrameID, VideoQueue().GetSize());
> }
> +
> + if (images.Length()) {
Prefer |foo > 0| over |foo| when it comes to integer comparison.
Attachment #8845757 -
Flags: review?(jwwang) → review+
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8845758 [details]
Bug 1346120 part 5 - Revert the blank decoder to create green frames;
https://reviewboard.mozilla.org/r/118912/#review120876
Attachment #8845758 -
Flags: review?(jwwang) → review+
Comment 14•8 years ago
|
||
mozreview-review |
Comment on attachment 8845759 [details]
Bug 1346120 part 6 - Test drawImage gets a non-single-color image from suspended video;
https://reviewboard.mozilla.org/r/118914/#review120878
::: dom/media/test/test_background_video_drawimage_with_suspended_video.html:50
(Diff revision 1)
> + for (let i = 0; i < 4*4; i++) {
> + let r = pixels[4*i+0];
> + let g = pixels[4*i+1];
> + let b = pixels[4*i+2];
> + if (r != rr || g != gg || b != bb) {
> + allSame = false;
Early break when 2 pixels are not the same color.
Attachment #8845759 -
Flags: review?(jwwang) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 21•8 years ago
|
||
Assignee | ||
Comment 22•8 years ago
|
||
Try looks pretty good, let's check in, thanks for the review!
Assignee | ||
Comment 23•8 years ago
|
||
Please help to check-in bug 1345403 comment 29 before this bug, sorry for inconvenience and thanks very much!
Keywords: checkin-needed
Comment 24•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/94a483ef784e
part 1 - Extract BlankMediaDataDecoder so it can be shared; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/c0758b9bf7b5
part 2 - Implement NullDecoderModule; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/ba579adbed21
part 3 - Use NullDecoderModule while suspending a video element's decoder; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/98d212462786
part 4 - Only set ImageContainer if there are valid new images in VideoSink::RenderVideoFrames(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/f16556658fd9
part 5 - Revert the blank decoder to create green frames; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/a30c73fc8d69
part 6 - Test drawImage gets a non-single-color image from suspended video; r=jwwang
Keywords: checkin-needed
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 31•8 years ago
|
||
sorry had to back this out for autophone Mdm tests failure in test_background_video_drawimage_with_suspended_video.html like https://treeherder.mozilla.org/logviewer.html#?job_id=83329627&repo=autoland&lineNumber=806
https://hg.mozilla.org/integration/autoland/rev/a7f05a91241e
Flags: needinfo?(kaku)
Assignee | ||
Comment 32•8 years ago
|
||
(In reply to Iris Hsiao [:ihsiao] from comment #31)
> sorry had to back this out for autophone Mdm tests failure in
> test_background_video_drawimage_with_suspended_video.html like
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=83329627&repo=autoland&lineNumber=806
>
> https://hg.mozilla.org/integration/autoland/rev/a7f05a91241e
comment 30 solves this issue.
Flags: needinfo?(kaku)
Assignee | ||
Comment 33•8 years ago
|
||
Assignee | ||
Comment 34•8 years ago
|
||
Try looks good, thanks for review and check in again!
Keywords: checkin-needed
Comment 35•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f002383e4302
part 1 - Extract BlankMediaDataDecoder so it can be shared; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/5e9e7264a224
part 2 - Implement NullDecoderModule; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/86e79ca1a6bb
part 3 - Use NullDecoderModule while suspending a video element's decoder; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/1f5176831c33
part 4 - Only set ImageContainer if there are valid new images in VideoSink::RenderVideoFrames(); r=jwwang
https://hg.mozilla.org/integration/autoland/rev/b1ce928cbeeb
part 5 - Revert the blank decoder to create green frames; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/e924204de040
part 6 - Test drawImage gets a non-single-color image from suspended video; r=jwwang
Keywords: checkin-needed
Comment 36•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f002383e4302
https://hg.mozilla.org/mozilla-central/rev/5e9e7264a224
https://hg.mozilla.org/mozilla-central/rev/86e79ca1a6bb
https://hg.mozilla.org/mozilla-central/rev/1f5176831c33
https://hg.mozilla.org/mozilla-central/rev/b1ce928cbeeb
https://hg.mozilla.org/mozilla-central/rev/e924204de040
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•