Closed
Bug 1143575
Opened 9 years ago
Closed 9 years ago
Send timestamped images from video decoding to the compositor
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: roc, Assigned: roc)
References
()
Details
Attachments
(64 files, 1 obsolete file)
29.66 KB,
text/plain
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
nical
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
bent.mozilla
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
Details | |
MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
mattwoodrow
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
cpearce
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
nical
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
nical
:
review+
sotaro
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
nical
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
mattwoodrow
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
mattwoodrow
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
mattwoodrow
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
MozReview Request: Bug 1143575. Let ImageContainer::SetCurrentImages accept multiple images. r=nical
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
Details | |
40 bytes,
text/x-review-board-request
|
cpearce
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
Details |
No description provided.
Assignee | ||
Comment 1•9 years ago
|
||
WIP here: https://bitbucket.org/rocallahan/mozilla-inbound/
Assignee | ||
Comment 2•9 years ago
|
||
I have this basically working. Still working on getting it to build and pass tests on all platforms. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae65256a240a On my desktop Linux setup (OMTC, Basic compositor, no hardware vsync) it doesn't provide much benefit on a regular ~25fps video. While my machine is idle, we basically never skip frames with or without these patches. With these patches, once in a while we display the second frame in the queue, so the frame timing is a little more accurate, but that never results in a frame being rendered that would otherwise be completely skipped. When I load the machine by doing a FF build, we sometimes skip a lot of frames, but that's always because the compositor thread didn't get scheduled for a while. So to get maximum benefit from this we need to have the compositor thread scheduled more reliably, by tweaking thread priorities and/or using HW vsync. I'm also testing http://media.junglecode.net/test/1077519/test.html. It doesn't get good results yet, apparently because of a bug where the MDSM thread too eagerly removes frames from the head of the queue that we need to display...
Flags: needinfo?(mchang)
Assignee | ||
Comment 4•9 years ago
|
||
Here's a log showing what happens during a run of Jet's gray-box test. There are several different issues, none of which seem to be related to these patches: There are some issues in the startup phase: 1) We skip the first frame because by the time the first data reaches the compositor the very first frame is out of date. 2) Right after the first composite, there's a massive compositor delay (2.3s in which the compositor doesn't run). This obviously causes a lot of frames to be skipped. Not sure what causes that. 3) After the massive compositor delay, the media stack throws out a lot of frames and the next frames it produces are significantly ahead of the current time. Maybe this is keyframe skipping? In the steady state there are two main issues: 4) Sometimes the compositor just doesn't get scheduled. At more or less random intervals, the delay between composites is 33/34ms (i.e. 2 frames at 60fps) instead of 16/17ms. I don't know what's causing this. I suspected kernel scheduling but the fact that it's exactly two frames (exactly 3 frames in just one case) suggests that it might be our bug. This is the most frequently observed problem. 5) The video rate isn't exactly matched to the compositor rate even though they're both trying to hit 60fps. You can see how the delay values (current time - frame time) slowly drift, usually increasing but sometimes decreasing for a while. From frameID 20 to 70 and from frameID 130 to 170 the delay drifts up and then drifts down again. A problem occurs when the delay drifts across 0, i.e. when the frame times almost exactly match the composition time. At that point, very small amounts of jitter can cause a video frame time to land either side of the composition time, giving us two or zero video frames in a composition interval. At the end: 6) For the last couple of frames, the media stack repeatedly sends them to the compositor with their timestamps increasing every time it sends them. There's some kind of shutdown issue I guess.
Flags: needinfo?(mchang)
Assignee | ||
Comment 5•9 years ago
|
||
In this log, there are three times where we select a frame that's not first in the queue to display the next frame in sequence, and we're not adjacent to one of the above problems. Those three times --- frameID 324, 356-358, and 479 --- are the places where fixing this bug enables us to avoid a glitch. So this work helps, but fixing the above problems, especially #4 and #5, will help more. Jet's testcase has an audio track, so the drift in issue #5 is likely just the audio clock drifting vs real-time clock (or screen vsync, if we had that enabled). So even though the video and screen rates are both nominally 60fps, those rates can drift relative to one another and it's not clear to me what we should do to avoid problems with jitter when the timestamps are nearly aligned. The best I can think of is a heuristic so that if consecutive frame timestamps are drifting from being slightly before the composition time to slightly after, we apply a negative bias to hold them before the composition time, until they exceed the composition time by some threshold, at which point we remove the bias. (And something similar for the other direction.) That would ensure we only miss or dup one frame instead of several in a row as we see in this log (e.g. in this log, frameIDs 177 and 179 are both skipped and frameIDs 176, 178 and 180 are duped; all we really need to do is dup one frame). Jesup, got any suggestions? :-)
Flags: needinfo?(rjesup)
Comment 6•9 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #5) > Jet's testcase has an audio track, so the drift in issue #5 is likely just > the audio clock drifting vs real-time clock (or screen vsync, if we had that > enabled). So even though the video and screen rates are both nominally > 60fps, those rates can drift relative to one another and it's not clear to > me what we should do to avoid problems with jitter when the timestamps are > nearly aligned. The best I can think of is a heuristic so that if > consecutive frame timestamps are drifting from being slightly before the > composition time to slightly after, we apply a negative bias to hold them > before the composition time, until they exceed the composition time by some > threshold, at which point we remove the bias. (And something similar for the > other direction.) That would ensure we only miss or dup one frame instead of > several in a row as we see in this log (e.g. in this log, frameIDs 177 and > 179 are both skipped and frameIDs 176, 178 and 180 are duped; all we really > need to do is dup one frame). Jesup, got any suggestions? :-) Right, some form of bias/PLL/averaging to avoid aligned-clock jitter effects is a common/reasonable solution. It can't guarantee consistent behavior, but in normal situations so long as the smoothing applied can absorb the amount of timing jitter, it will work well. Even if jitter exceeds the smoothing levels, it will greatly reduce residual jitter across the timing boundary. You should be running this more-or-less all the time probably. Effectively it just needs to provide an appropriate hysteresis across the aligned-clocks state.
Flags: needinfo?(rjesup)
Assignee | ||
Comment 7•9 years ago
|
||
#4 actually seems to be caused by a bug where we're not computing an invalid region for an image layer when there's not been a layer transaction, but the image layer wants to display a new image just because of the composition time advancing. I'll fix that, and that plus the bias fix might solve our problems (apart from the start/end issues).
Comment 8•9 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #5) > Jet's testcase has an audio track, so the drift in issue #5 is likely just > the audio clock drifting vs real-time clock (or screen vsync, if we had that > enabled). So even though the video and screen rates are both nominally > 60fps, those rates can drift relative to one another and it's not clear to > me what we should do to avoid problems with jitter when the timestamps are > nearly aligned. The best I can think of is a heuristic so that if > consecutive frame timestamps are drifting from being slightly before the > composition time to slightly after, we apply a negative bias to hold them > before the composition time, until they exceed the composition time by some > threshold, at which point we remove the bias. (And something similar for the > other direction.) That would ensure we only miss or dup one frame instead of > several in a row as we see in this log (e.g. in this log, frameIDs 177 and > 179 are both skipped and frameIDs 176, 178 and 180 are duped; all we really > need to do is dup one frame). Jesup, got any suggestions? :-) From what I gather from the bug, these problems are on Linux. Some composition timing drift is due to software vsync, but I wonder if these results reproduce on Mac or Windows, which have hardware vsync enabled. Linux has "software" vsync enabled, so we could fix some drift if we implement hardware vsync on linux. For #5, is there any reason we should try to fix this drift versus making the audio and video clock key off the same timer? Then they would be aligned, but I'm not familiar with how video is implemented. It seems like the audio and video timers should be synced. Also, I checked out the repo at comment 1 but couldn't find where the log was being produced. That would help diagnose this issue a bit better. Thanks!
Flags: needinfo?(mchang)
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to Mason Chang [:mchang] from comment #8) > From what I gather from the bug, these problems are on Linux. Right. > Some composition timing drift is due to software vsync, but I wonder if these > results reproduce on Mac or Windows, which have hardware vsync enabled. I'm sure there will still be drift with HW vsync, since the problem is that the audio clock is an independent clock from the video sync clock, regardless of what's driving the video clock. > For #5, is there any reason we should try to fix this drift versus making > the audio and video clock key off the same timer? Then they would be > aligned, but I'm not familiar with how video is implemented. It seems like > the audio and video timers should be synced. We can't do that, we're at the mercy of the audio and video hardware here. E.g. even if a 50fps movie has audio and video data spanning exactly 1 second (in terms of timestamps in the media file), it may take slightly more or less than 1 second to play that audio out due to the audio hardware playing slightly slower or faster than "real time". Likewise we might not be able to play exactly 60 frames to the screen, since the screen vsync will not be exactly 60fps. But we can't really fix or work around either of those issues. I think the bias fix will be pretty easy anyway. > Also, I checked out the repo at comment 1 but couldn't find where the log > was being produced. That would help diagnose this issue a bit better. Thanks! Sorry, that's an mq patch not in the queue. I just wanted you to be aware of what's going on here, I don't need anything changed at the moment. I do think we need to think about how to get the compositor thread to be scheduled reliably on a loaded system.
Comment 10•9 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9) > We can't do that, we're at the mercy of the audio and video hardware here. > E.g. even if a 50fps movie has audio and video data spanning exactly 1 > second (in terms of timestamps in the media file), it may take slightly more > or less than 1 second to play that audio out due to the audio hardware > playing slightly slower or faster than "real time". Likewise we might not be > able to play exactly 60 frames to the screen, since the screen vsync will > not be exactly 60fps. But we can't really fix or work around either of those > issues. I think the bias fix will be pretty easy anyway. > That's a bummer :/ > > Also, I checked out the repo at comment 1 but couldn't find where the log > > was being produced. That would help diagnose this issue a bit better. Thanks! > > Sorry, that's an mq patch not in the queue. I just wanted you to be aware of > what's going on here, I don't need anything changed at the moment. I do > think we need to think about how to get the compositor thread to be > scheduled reliably on a loaded system. Do you know how long the composites are taking? If the composites are short and we can decode the video on hardware, I'm less worried about scheduling composition reliably versus just ensuring the composite finishes before the next vblank. As long as we composite in time, it doesn't matter when we start composting versus we finish in time.
Assignee | ||
Comment 11•9 years ago
|
||
I think I've fixed the invalidation and bias issues. Results have improved on Jet's test but I still see some problems: -- The starting and ending issues are still there. -- Sometimes moving the mouse seems to trigger missed compositor deadlines. rr suggests that's because the compositor is blocking on X11 display locks held by the main thread during synchronous X server round trips to convert mouse event coordinates. -- Sometimes even when my results suggest we didn't miss any frames during composition, I still see missed frames visually. This could be our software vsync timer jittering against screen vsync, or the gnome-shell compositor messing things up. On the bright side, my patch queue is definitely improving things. It regularly picks a queue entry > 0 to render at times when we would otherwise have displayed a glitch. And we're certainly doing better than Chrome :-).
Assignee | ||
Comment 12•9 years ago
|
||
> Do you know how long the composites are taking? If the composites are short
> and we can decode the video on hardware, I'm less worried about scheduling
> composition reliably versus just ensuring the composite finishes before the
> next vblank. As long as we composite in time, it doesn't matter when we
> start composting versus we finish in time.
Sometimes we're not able to finish the composite before the next vsync signal in which case we're just skipping the composite because CompositorVsyncScheduler::NotifyVsync sees that mCurrentCompositeTask is not null. Maybe we should have some telemetry there?
Looking at the compositor thread when that happens, I've seen it in the compositor waiting on an X11 lock held by the main thread (see above), and I've seen it in a LayerBridge update.
Updated•9 years ago
|
Assignee | ||
Comment 13•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3f2413c7593
Assignee | ||
Comment 14•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=163b7a27ee9d
Assignee | ||
Comment 15•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c88505f7fc0a
Assignee | ||
Comment 16•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6089884ad4b
Assignee | ||
Comment 17•9 years ago
|
||
Windows builds fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=14cbe5450224 We appear to be green on desktop and B2G. On Android we failed in dom/media/mediasource/test/test_HaveMetadataUnbufferedSeek.html, which I've confirmed is a bug in the test. The final issue is that we're crashing in robocop tests such as testAboutPage. I'm looking into that now.
Assignee | ||
Comment 18•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=11da2cf71a96
Assignee | ||
Comment 19•9 years ago
|
||
Alright, I'm green on Android now, after wasting time on bug 1174072 :-(. Got a couple of failures on Windows to go: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/rocallahan@mozilla.com-14cbe5450224/try-win32-debug/try_win7-ix-debug_test-mochitest-3-bm119-tests1-windows-build1327.txt.gz http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/rocallahan@mozilla.com-14cbe5450224/try-win32-debug/try_win7-ix-debug_test-mochitest-gl-bm111-tests1-windows-build834.txt.gz
Assignee | ||
Comment 20•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=632a2a5906f3
Assignee | ||
Comment 21•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4d8fc44b8a7
Assignee | ||
Comment 22•9 years ago
|
||
OK, with this push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4711f511ab8 and this fix on top of it: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b84b37a7973 we are green.
Assignee | ||
Comment 23•9 years ago
|
||
Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical
Attachment #8622203 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 24•9 years ago
|
||
Bug 1143575. Resizing a WebGL canvas should trigger PresentScreenBuffer. r=jgilbert At the end of drawingbuffer-static-canvas-test.html, the test resizes the canvas and does no drawing. The canvas layer changes size but the front buffer size is unchanged, and this mismatch triggers assertions that later patches will add. I guess it could also trigger rendering issues.
Attachment #8622204 -
Flags: review?(jgilbert)
Assignee | ||
Comment 25•9 years ago
|
||
Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical On some Android versions, GraphicBuffer.h ends up including libui's hardware.h, which #defines the symbols version_minor and version_major, which are used as field names in Ogg Theora's th_info struct. Later patches will require some files to include both Theora headers and LayerTypes.h.
Attachment #8622205 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 26•9 years ago
|
||
Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent
Attachment #8622206 -
Flags: review?(bent.mozilla)
Assignee | ||
Comment 27•9 years ago
|
||
Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce
Attachment #8622207 -
Flags: review?(cpearce)
Assignee | ||
Comment 28•9 years ago
|
||
Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical
Attachment #8622208 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 29•9 years ago
|
||
Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical
Attachment #8622209 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 30•9 years ago
|
||
Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce
Attachment #8622210 -
Flags: review?(cpearce)
Assignee | ||
Comment 31•9 years ago
|
||
Bug 1143575. Make GL context current before cleaning up programs. r=nical Otherwise we can get a crash with the following stack: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 14711] 0x5d99974e in mozilla::gl::GLContext::BeforeGLCall (this=0x6dbf0800, funcName=0x60f251a4 <mozilla::gl::GLContext::raw_fDeleteProgram(unsigned int)::__PRETTY_FUNCTION__> "void mozilla::gl::GLContext::raw_fDeleteProgram(GLuint)") at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:683 683 MOZ_ASSERT(IsCurrent()); (gdb) where #0 0x5d99974e in mozilla::gl::GLContext::BeforeGLCall (this=0x6dbf0800, funcName=0x60f251a4 <mozilla::gl::GLContext::raw_fDeleteProgram(unsigned int)::__PRETTY_FUNCTION__> "void mozilla::gl::GLContext::raw_fDeleteProgram(GLuint)") at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:683 #1 0x5d99bed6 in mozilla::gl::GLContext::raw_fDeleteProgram (this=0x6dbf0800, program=210003) at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:2232 #2 0x5d99c10a in mozilla::gl::GLContext::fDeleteProgram (this=0x6dbf0800, program=210003) at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:2270 #3 0x5daa0ae6 in mozilla::layers::ShaderProgramOGL::~ShaderProgramOGL (this=0x6d7df000, __in_chrg=<optimized out>) at /home/roc/mozilla-inbound/gfx/layers/opengl/OGLShaderProgram.cpp:491 #4 0x5da86bdc in mozilla::layers::CompositorOGL::CleanupResources (this=0x67ae4d70) at /home/roc/mozilla-inbound/gfx/layers/opengl/CompositorOGL.cpp:177
Attachment #8622211 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 32•9 years ago
|
||
Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical There is some ambiguity about whether ScheduleComposite will necessarily trigger a composite all the way to nsWindow::DrawWindowUnderlay. Android robocop tests assume it will, because they rely on DrawWindowOverlay being called so they can take a screenshot and make progress, but this is a very fragile assumption. They also rely on the entire window being painted, which is also a fragile assumption. This patch improves the situation by explicitly invalidating the current window area when Android Java code needs to trigger a composite. This avoids regressions from future patches in this series which make composition bail out when there is nothing invalid. The resulting setup is still a bit fragile for my taste but I'm not sure what the ideal solution would be.
Attachment #8622212 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 33•9 years ago
|
||
Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical
Attachment #8622213 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 34•9 years ago
|
||
Bug 1143575. Remove unused CompositionNotifySink. r=nical
Attachment #8622214 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 35•9 years ago
|
||
Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical
Attachment #8622215 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 36•9 years ago
|
||
Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical
Attachment #8622216 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 37•9 years ago
|
||
Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical
Attachment #8622217 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 38•9 years ago
|
||
Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical
Attachment #8622218 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 39•9 years ago
|
||
Bug 1143575. Remove unused ReturnReleaseFence. r=nical
Attachment #8622219 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 40•9 years ago
|
||
Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow
Attachment #8622220 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 41•9 years ago
|
||
Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical
Attachment #8622221 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 42•9 years ago
|
||
Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical
Attachment #8622222 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 43•9 years ago
|
||
Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical This simplifies code slightly.
Attachment #8622223 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 44•9 years ago
|
||
Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical
Attachment #8622224 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 45•9 years ago
|
||
Bug 1143575. Fix indent. r=cpearce
Attachment #8622225 -
Flags: review?(cpearce)
Assignee | ||
Comment 46•9 years ago
|
||
Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce
Attachment #8622226 -
Flags: review?(cpearce)
Assignee | ||
Comment 47•9 years ago
|
||
Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce
Attachment #8622227 -
Flags: review?(cpearce)
Assignee | ||
Comment 48•9 years ago
|
||
Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce This makes MediaDecoderStateMachine::GetVideoStreamPosition compute a time that's more consistent with the audio clock.
Attachment #8622228 -
Flags: review?(cpearce)
Assignee | ||
Comment 49•9 years ago
|
||
Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce
Attachment #8622229 -
Flags: review?(cpearce)
Assignee | ||
Comment 50•9 years ago
|
||
Bug 1143575. Rename clock_time to clockTime. r=cpearce
Attachment #8622230 -
Flags: review?(cpearce)
Assignee | ||
Comment 51•9 years ago
|
||
Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce This makes normal playback consistent with the buffering state, which already does this. We'll also need this when we handle multiple images, because then we need to hande the entire queue of images to the ImageContainer without pulling any of them off the queue.
Attachment #8622231 -
Flags: review?(cpearce)
Assignee | ||
Comment 52•9 years ago
|
||
Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce
Attachment #8622232 -
Flags: review?(cpearce)
Assignee | ||
Comment 53•9 years ago
|
||
Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical The picture rect logically belongs with the texture, and later patches will make OpUseTexture take multiple textures, each of which needs its own picture rect.
Attachment #8622233 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 54•9 years ago
|
||
Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical
Attachment #8622234 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 55•9 years ago
|
||
Bug 1143575. Fix typo in ImageContainer comment. r=nical
Attachment #8622235 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 56•9 years ago
|
||
Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical
Attachment #8622236 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 57•9 years ago
|
||
Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical
Attachment #8622237 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 58•9 years ago
|
||
Bug 1143575. Store composition time in Compositor. r=nical We'll need this later so ImageHost can select the correct image to use.
Attachment #8622238 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 59•9 years ago
|
||
Bug 1143575. Implement ImageHost support for multiple timed images. r=nical
Attachment #8622239 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 60•9 years ago
|
||
Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical
Attachment #8622240 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 61•9 years ago
|
||
Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical When ImageContainer and ImageClient are managing a list of images, the individual Image serial numbers are no longer enough to detect whether the state has changed.
Attachment #8622241 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 62•9 years ago
|
||
Bug 1143575. Remove ImageClientBridge::Updated. r=nical
Attachment #8622242 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 63•9 years ago
|
||
Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical
Attachment #8622243 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 64•9 years ago
|
||
Comment on attachment 8622203 [details] MozReview Request: Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical
Assignee | ||
Comment 65•9 years ago
|
||
Bug 1143575. Fix some code formatting. r=nical
Attachment #8622244 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 66•9 years ago
|
||
Comment on attachment 8622204 [details] MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical Bug 1143575. Resizing a WebGL canvas should trigger PresentScreenBuffer. r=jgilbert At the end of drawingbuffer-static-canvas-test.html, the test resizes the canvas and does no drawing. The canvas layer changes size but the front buffer size is unchanged, and this mismatch triggers assertions that later patches will add. I guess it could also trigger rendering issues.
Assignee | ||
Comment 67•9 years ago
|
||
Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical
Attachment #8622245 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 68•9 years ago
|
||
Comment on attachment 8622205 [details] MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical On some Android versions, GraphicBuffer.h ends up including libui's hardware.h, which #defines the symbols version_minor and version_major, which are used as field names in Ogg Theora's th_info struct. Later patches will require some files to include both Theora headers and LayerTypes.h.
Assignee | ||
Comment 69•9 years ago
|
||
Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical
Attachment #8622246 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 70•9 years ago
|
||
Comment on attachment 8622206 [details] MozReview Request: Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent
Assignee | ||
Comment 71•9 years ago
|
||
Comment on attachment 8622207 [details] MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce
Attachment #8622247 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 72•9 years ago
|
||
Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical For frame statistics to work properly, we have to notify an ImageContainer when it has been composited. This requires a few changes, which have been lumped together in this patch: -- Create PImageContainer and ImageContainerParent/ImageContainerChild. -- Add mFrameID and mProducerID everywhere we're passing around images. -- Route composition notifications from the compositor back to ImageContainerChild.
Assignee | ||
Comment 73•9 years ago
|
||
Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow
Attachment #8622248 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 74•9 years ago
|
||
Comment on attachment 8622208 [details] MozReview Request: Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical
Assignee | ||
Comment 75•9 years ago
|
||
Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow We need this change so that when ImageHost has a next image to display more than one composition-interval in the future, we skip the actual compositing work in those intermediate composition(s) if nothing else has changed. This change is a little bit scary since it breaks any code that was previously assuming ScheduleComposition would actually update the screen. However, that code was already broken for BasicCompositor.
Attachment #8622249 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 76•9 years ago
|
||
Comment on attachment 8622209 [details] MozReview Request: Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical
Assignee | ||
Comment 77•9 years ago
|
||
Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow We need to remove this so that adding images to the end of the list of images for an ImageLayer doesn't force composition to happen even if nothing else has changed.
Attachment #8622250 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 78•9 years ago
|
||
Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical
Attachment #8622251 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 79•9 years ago
|
||
Bug 1143575. Don't report negative frame delays. r=cpearce
Attachment #8622252 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 80•9 years ago
|
||
Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical
Attachment #8622253 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 81•9 years ago
|
||
Comment on attachment 8622203 [details] MozReview Request: Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical
Assignee | ||
Comment 82•9 years ago
|
||
Comment on attachment 8622204 [details] MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical Bug 1143575. Resizing a WebGL canvas should trigger PresentScreenBuffer. r=jgilbert At the end of drawingbuffer-static-canvas-test.html, the test resizes the canvas and does no drawing. The canvas layer changes size but the front buffer size is unchanged, and this mismatch triggers assertions that later patches will add. I guess it could also trigger rendering issues.
Assignee | ||
Comment 83•9 years ago
|
||
Comment on attachment 8622205 [details] MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical On some Android versions, GraphicBuffer.h ends up including libui's hardware.h, which #defines the symbols version_minor and version_major, which are used as field names in Ogg Theora's th_info struct. Later patches will require some files to include both Theora headers and LayerTypes.h.
Assignee | ||
Comment 84•9 years ago
|
||
Comment on attachment 8622206 [details] MozReview Request: Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent
Assignee | ||
Comment 85•9 years ago
|
||
Comment on attachment 8622207 [details] MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce
Assignee | ||
Comment 86•9 years ago
|
||
Comment on attachment 8622208 [details] MozReview Request: Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical
Assignee | ||
Comment 87•9 years ago
|
||
Comment on attachment 8622209 [details] MozReview Request: Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical
Assignee | ||
Comment 88•9 years ago
|
||
Comment on attachment 8622210 [details] MozReview Request: Bug 1143575. Make GL context current before cleaning up programs. r=nical Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce
Assignee | ||
Comment 89•9 years ago
|
||
Comment on attachment 8622211 [details] MozReview Request: Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical Bug 1143575. Make GL context current before cleaning up programs. r=nical Otherwise we can get a crash with the following stack: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 14711] 0x5d99974e in mozilla::gl::GLContext::BeforeGLCall (this=0x6dbf0800, funcName=0x60f251a4 <mozilla::gl::GLContext::raw_fDeleteProgram(unsigned int)::__PRETTY_FUNCTION__> "void mozilla::gl::GLContext::raw_fDeleteProgram(GLuint)") at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:683 683 MOZ_ASSERT(IsCurrent()); (gdb) where #0 0x5d99974e in mozilla::gl::GLContext::BeforeGLCall (this=0x6dbf0800, funcName=0x60f251a4 <mozilla::gl::GLContext::raw_fDeleteProgram(unsigned int)::__PRETTY_FUNCTION__> "void mozilla::gl::GLContext::raw_fDeleteProgram(GLuint)") at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:683 #1 0x5d99bed6 in mozilla::gl::GLContext::raw_fDeleteProgram (this=0x6dbf0800, program=210003) at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:2232 #2 0x5d99c10a in mozilla::gl::GLContext::fDeleteProgram (this=0x6dbf0800, program=210003) at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:2270 #3 0x5daa0ae6 in mozilla::layers::ShaderProgramOGL::~ShaderProgramOGL (this=0x6d7df000, __in_chrg=<optimized out>) at /home/roc/mozilla-inbound/gfx/layers/opengl/OGLShaderProgram.cpp:491 #4 0x5da86bdc in mozilla::layers::CompositorOGL::CleanupResources (this=0x67ae4d70) at /home/roc/mozilla-inbound/gfx/layers/opengl/CompositorOGL.cpp:177
Assignee | ||
Comment 90•9 years ago
|
||
Comment on attachment 8622212 [details] MozReview Request: Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical There is some ambiguity about whether ScheduleComposite will necessarily trigger a composite all the way to nsWindow::DrawWindowUnderlay. Android robocop tests assume it will, because they rely on DrawWindowOverlay being called so they can take a screenshot and make progress, but this is a very fragile assumption. They also rely on the entire window being painted, which is also a fragile assumption. This patch improves the situation by explicitly invalidating the current window area when Android Java code needs to trigger a composite. This avoids regressions from future patches in this series which make composition bail out when there is nothing invalid. The resulting setup is still a bit fragile for my taste but I'm not sure what the ideal solution would be.
Assignee | ||
Comment 91•9 years ago
|
||
Comment on attachment 8622213 [details] MozReview Request: Bug 1143575. Remove unused CompositionNotifySink. r=nical Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical
Assignee | ||
Comment 92•9 years ago
|
||
Comment on attachment 8622214 [details] MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical Bug 1143575. Remove unused CompositionNotifySink. r=nical
Assignee | ||
Comment 93•9 years ago
|
||
Comment on attachment 8622215 [details] MozReview Request: Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical
Assignee | ||
Comment 94•9 years ago
|
||
Comment on attachment 8622216 [details] MozReview Request: Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical
Assignee | ||
Comment 95•9 years ago
|
||
Comment on attachment 8622217 [details] MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical
Assignee | ||
Comment 96•9 years ago
|
||
Comment on attachment 8622218 [details] MozReview Request: Bug 1143575. Remove unused ReturnReleaseFence. r=nical Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical
Assignee | ||
Comment 97•9 years ago
|
||
Comment on attachment 8622219 [details] MozReview Request: Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow Bug 1143575. Remove unused ReturnReleaseFence. r=nical
Assignee | ||
Comment 98•9 years ago
|
||
Comment on attachment 8622220 [details] MozReview Request: Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow
Assignee | ||
Comment 99•9 years ago
|
||
Comment on attachment 8622221 [details] MozReview Request: Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical
Assignee | ||
Comment 100•9 years ago
|
||
Comment on attachment 8622222 [details] MozReview Request: Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical
Assignee | ||
Comment 101•9 years ago
|
||
Comment on attachment 8622223 [details] MozReview Request: Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical This simplifies code slightly.
Assignee | ||
Comment 102•9 years ago
|
||
Comment on attachment 8622224 [details] MozReview Request: Bug 1143575. Fix indent. r=cpearce Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical
Assignee | ||
Comment 103•9 years ago
|
||
Comment on attachment 8622225 [details] MozReview Request: Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce Bug 1143575. Fix indent. r=cpearce
Assignee | ||
Comment 104•9 years ago
|
||
Comment on attachment 8622226 [details] MozReview Request: Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce
Assignee | ||
Comment 105•9 years ago
|
||
Comment on attachment 8622227 [details] MozReview Request: Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce
Assignee | ||
Comment 106•9 years ago
|
||
Comment on attachment 8622228 [details] MozReview Request: Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce This makes MediaDecoderStateMachine::GetVideoStreamPosition compute a time that's more consistent with the audio clock.
Assignee | ||
Comment 107•9 years ago
|
||
Comment on attachment 8622229 [details] MozReview Request: Bug 1143575. Rename clock_time to clockTime. r=cpearce Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce
Assignee | ||
Comment 108•9 years ago
|
||
Comment on attachment 8622230 [details] MozReview Request: Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce Bug 1143575. Rename clock_time to clockTime. r=cpearce
Assignee | ||
Comment 109•9 years ago
|
||
Comment on attachment 8622231 [details] MozReview Request: Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce This makes normal playback consistent with the buffering state, which already does this. We'll also need this when we handle multiple images, because then we need to hande the entire queue of images to the ImageContainer without pulling any of them off the queue.
Assignee | ||
Comment 110•9 years ago
|
||
Comment on attachment 8622232 [details] MozReview Request: Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce
Assignee | ||
Comment 111•9 years ago
|
||
Comment on attachment 8622233 [details] MozReview Request: Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical The picture rect logically belongs with the texture, and later patches will make OpUseTexture take multiple textures, each of which needs its own picture rect.
Assignee | ||
Comment 112•9 years ago
|
||
Comment on attachment 8622234 [details] MozReview Request: Bug 1143575. Fix typo in ImageContainer comment. r=nical Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical
Assignee | ||
Comment 113•9 years ago
|
||
Comment on attachment 8622235 [details] MozReview Request: Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical Bug 1143575. Fix typo in ImageContainer comment. r=nical
Assignee | ||
Comment 114•9 years ago
|
||
Comment on attachment 8622236 [details] MozReview Request: Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical
Assignee | ||
Comment 115•9 years ago
|
||
Comment on attachment 8622237 [details] MozReview Request: Bug 1143575. Store composition time in Compositor. r=nical Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical
Assignee | ||
Comment 116•9 years ago
|
||
Comment on attachment 8622238 [details] MozReview Request: Bug 1143575. Implement ImageHost support for multiple timed images. r=nical Bug 1143575. Store composition time in Compositor. r=nical We'll need this later so ImageHost can select the correct image to use.
Assignee | ||
Comment 117•9 years ago
|
||
Comment on attachment 8622203 [details] MozReview Request: Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical
Assignee | ||
Comment 118•9 years ago
|
||
Comment on attachment 8622239 [details] MozReview Request: Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical Bug 1143575. Implement ImageHost support for multiple timed images. r=nical
Assignee | ||
Comment 119•9 years ago
|
||
Comment on attachment 8622204 [details] MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical Bug 1143575. Resizing a WebGL canvas should trigger PresentScreenBuffer. r=jgilbert At the end of drawingbuffer-static-canvas-test.html, the test resizes the canvas and does no drawing. The canvas layer changes size but the front buffer size is unchanged, and this mismatch triggers assertions that later patches will add. I guess it could also trigger rendering issues.
Assignee | ||
Comment 120•9 years ago
|
||
Comment on attachment 8622240 [details] MozReview Request: Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical
Assignee | ||
Comment 121•9 years ago
|
||
Comment on attachment 8622205 [details] MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical On some Android versions, GraphicBuffer.h ends up including libui's hardware.h, which #defines the symbols version_minor and version_major, which are used as field names in Ogg Theora's th_info struct. Later patches will require some files to include both Theora headers and LayerTypes.h.
Assignee | ||
Comment 122•9 years ago
|
||
Comment on attachment 8622241 [details] MozReview Request: Bug 1143575. Remove ImageClientBridge::Updated. r=nical Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical When ImageContainer and ImageClient are managing a list of images, the individual Image serial numbers are no longer enough to detect whether the state has changed.
Assignee | ||
Comment 123•9 years ago
|
||
Comment on attachment 8622206 [details] MozReview Request: Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent
Assignee | ||
Comment 124•9 years ago
|
||
Comment on attachment 8622242 [details] MozReview Request: Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical Bug 1143575. Remove ImageClientBridge::Updated. r=nical
Assignee | ||
Comment 125•9 years ago
|
||
Comment on attachment 8622207 [details] MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce
Assignee | ||
Comment 126•9 years ago
|
||
Comment on attachment 8622243 [details] MozReview Request: Bug 1143575. Fix some code formatting. r=nical Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical
Assignee | ||
Comment 127•9 years ago
|
||
Comment on attachment 8622244 [details] MozReview Request: Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical,sotaro Bug 1143575. Fix some code formatting. r=nical
Assignee | ||
Comment 128•9 years ago
|
||
Comment on attachment 8622245 [details] MozReview Request: Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical
Assignee | ||
Comment 129•9 years ago
|
||
Comment on attachment 8622246 [details] MozReview Request: Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical
Assignee | ||
Comment 130•9 years ago
|
||
Comment on attachment 8622247 [details] MozReview Request: Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical For frame statistics to work properly, we have to notify an ImageContainer when it has been composited. This requires a few changes, which have been lumped together in this patch: -- Create PImageContainer and ImageContainerParent/ImageContainerChild. -- Add mFrameID and mProducerID everywhere we're passing around images. -- Route composition notifications from the compositor back to ImageContainerChild.
Assignee | ||
Comment 131•9 years ago
|
||
Comment on attachment 8622248 [details] MozReview Request: Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow
Assignee | ||
Comment 132•9 years ago
|
||
Comment on attachment 8622249 [details] MozReview Request: Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow We need this change so that when ImageHost has a next image to display more than one composition-interval in the future, we skip the actual compositing work in those intermediate composition(s) if nothing else has changed. This change is a little bit scary since it breaks any code that was previously assuming ScheduleComposition would actually update the screen. However, that code was already broken for BasicCompositor.
Assignee | ||
Comment 133•9 years ago
|
||
Comment on attachment 8622250 [details] MozReview Request: Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow We need to remove this so that adding images to the end of the list of images for an ImageLayer doesn't force composition to happen even if nothing else has changed.
Assignee | ||
Comment 134•9 years ago
|
||
Comment on attachment 8622251 [details] MozReview Request: Bug 1143575. Don't report negative frame delays. r=cpearce Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical
Assignee | ||
Comment 135•9 years ago
|
||
Comment on attachment 8622252 [details] MozReview Request: Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical Bug 1143575. Don't report negative frame delays. r=cpearce
Assignee | ||
Comment 136•9 years ago
|
||
Comment on attachment 8622253 [details] MozReview Request: Bug 1143575. Remove ClearAllImagesExceptFront because it doesn't do anything. r=nical Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical
Assignee | ||
Comment 137•9 years ago
|
||
Bug 1143575. Remove ClearAllImagesExceptFront because it doesn't do anything. r=nical ImageBridgeChild::FlushAllImages with aExceptFront==true does absolutely nothing, so remove the parameter and remove all callers which pass true.
Attachment #8622255 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 138•9 years ago
|
||
Bug 1143575. Clarify code by renaming method to ClearCurrentImageFromImageBridge. r=nical We need to make it clear that ClearCurrentImage is really an internal method of the ImageContainer implementation, not a method that ImageContainer users should call.
Attachment #8622256 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 139•9 years ago
|
||
Bug 1143575. Implement ImageContainer::GetDroppedCount. r=nical
Attachment #8622257 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 140•9 years ago
|
||
Bug 1143575. Reimplement ImageContainer::GetPaintCount to be composition-aware. r=nical
Attachment #8622258 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 141•9 years ago
|
||
Bug 1143575. Let callers of ImageContainer::SetCurrentImages specify frame IDs. r=nical
Attachment #8622259 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 142•9 years ago
|
||
Bug 1143575. Let ImageContainer::SetCurrentImages accept multiple images. r=nical
Attachment #8622260 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 143•9 years ago
|
||
Bug 1143575. Introduce VideoFrameContainer::ClearCurrentFrame(size), and don't increment mFrameID when clearing frames. r=cpearce
Attachment #8622261 -
Flags: review?(cpearce)
Assignee | ||
Comment 144•9 years ago
|
||
Bug 1143575. Introduce VideoFrameContainer::SetCurrentFrames. r=cpearce
Attachment #8622262 -
Flags: review?(cpearce)
Assignee | ||
Comment 145•9 years ago
|
||
Bug 1143575. Add MediaQueue::GetFirstElements. r=cpearce
Attachment #8622263 -
Flags: review?(cpearce)
Assignee | ||
Comment 146•9 years ago
|
||
Bug 1143575. Add frame IDs to VideoData. r=cpearce
Attachment #8622264 -
Flags: review?(cpearce)
Assignee | ||
Comment 147•9 years ago
|
||
Bug 1143575. Refactor UpdateRenderedVideoFrames to support pushing multiple frames from the VideoQueue to the ImageContainer. r=cpearce
Attachment #8622265 -
Flags: review?(cpearce)
Assignee | ||
Comment 148•9 years ago
|
||
Bug 1143575. Push all available frames to the compositor. r=cpearce
Attachment #8622266 -
Flags: review?(cpearce)
Assignee | ||
Comment 149•9 years ago
|
||
Bug 1143575. Add a bias value to ImageHost to avoid unpredictable results when image times and compositor times are closely aligned. r=nical
Attachment #8622267 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 150•9 years ago
|
||
The patches in the "Mozreview request" list in this bug appear out of order. The patches in the attachments list are in the correct order. Note that the first 30 patches (up to and including "Remove unused MediaQueue::Empty") are refactorings and bug fixes that should make sense independent of the actual work in this bug. The remaining 34 patches do the work of this bug. They're all reasonably small and self-contained except for "Route ImageCompositeNotifications to ImageContainers".
Comment 151•9 years ago
|
||
https://reviewboard.mozilla.org/r/11269/#review9713 ::: dom/media/MediaDecoderStateMachine.cpp:2991 (Diff revision 1) > - int64_t delay = remainingTime / mPlaybackRate; > + int64_t delay = remainingTime / mPlaybackRate; tab ::: dom/media/MediaDecoderStateMachine.cpp:2993 (Diff revision 1) > ScheduleStateMachineIn(delay); MOZ_ASSERT(aMicroseconds > 0) at 3252. So delay can't be 0.
Comment 152•9 years ago
|
||
https://reviewboard.mozilla.org/r/11333/#review9717 ::: dom/media/MediaData.h:274 (Diff revision 1) > + int32_t mFrameID; Could it be a const?
Comment 153•9 years ago
|
||
https://reviewboard.mozilla.org/r/11333/#review9721 ::: dom/media/MediaData.h:274 (Diff revision 1) > + int32_t mFrameID; No. It is updated in MediaDecoderStateMachine::Push in next patch.
Comment 154•9 years ago
|
||
https://reviewboard.mozilla.org/r/11269/#review9723 > tab Oops. this is not a tab but a mark from review board to indicate indent changes.
Comment 155•9 years ago
|
||
https://reviewboard.mozilla.org/r/11335/#review9719 ::: dom/media/MediaDecoderStateMachine.h:526 (Diff revision 1) > + // timestamps for the frames; when omitted, aMaxFrames must be 1 and the s/and the/and/ ::: dom/media/MediaDecoderStateMachine.h:753 (Diff revision 1) > + ProducerID mProducerID; const ::: dom/media/MediaDecoderStateMachine.cpp:901 (Diff revision 1) > + aSample->mFrameID = ++mCurrentFrameID; Is it ok to update the frame ID again when the popped video sample is pushed back to the queue in UpdateRenderedVideoFrames(). ::: dom/media/MediaDecoderStateMachine.cpp:1267 (Diff revision 1) > + RenderVideoFrames(1); Why needing to render video frames here? ::: dom/media/MediaDecoderStateMachine.cpp:2972 (Diff revision 1) > + frameStats.NotifyPresentedFrame(); Shouldn't we call NotifyPresentedFrame() as many times as framesRemoved? ::: dom/media/MediaDecoderStateMachine.h:523 (Diff revision 1) > + // state machine thread. Decode monitor must be held with exactly one lock It would be nice to add some checks to ensure no re-entry into this function so lock count == 1 can hold.
Assignee | ||
Comment 156•9 years ago
|
||
https://reviewboard.mozilla.org/r/11269/#review9727 > MOZ_ASSERT(aMicroseconds > 0) at 3252. > So delay can't be 0. True. I'll make 'delay' a minimum of 1, then. The only way it could reach 0 is if remainingTime was very small and then got rounded down to zero when dividing by mPlaybackRate.
Assignee | ||
Comment 157•9 years ago
|
||
https://reviewboard.mozilla.org/r/11335/#review9729 > s/and the/and/ Will fix. > It would be nice to add some checks to ensure no re-entry into this function so lock count == 1 can hold. OK, I'll do that. > const OK > Is it ok to update the frame ID again when the popped video sample is pushed back to the queue in UpdateRenderedVideoFrames(). The code in UpdateRenderedVideoFrames() calls VideoQueue().PushFront() which doesn't reach this line. > Why needing to render video frames here? We have to call RenderVideoFrames(1) in StopPlayback() because we may have queued multiple frames in the compositor. If we don't call RenderVideoFrames(1), those frames will stay queued in the compositor so video playback will continue until the compositor reaches the end of the queue. I'll put this in a comment. > Shouldn't we call NotifyPresentedFrame() as many times as framesRemoved? Maybe, but the code I'm changing here doesn't call NotifyPresentedFrame() for frames that were skipped and never sent to the compositor, so I preserved that behavior. Let's see what cpearce says.
Comment 158•9 years ago
|
||
Comment on attachment 8622220 [details] MozReview Request: Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical https://reviewboard.mozilla.org/r/11247/#review9737 Ship It!
Attachment #8622220 -
Flags: review?(matt.woodrow) → review+
Updated•9 years ago
|
Attachment #8622248 -
Flags: review?(matt.woodrow)
Comment 159•9 years ago
|
||
Comment on attachment 8622248 [details] MozReview Request: Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow https://reviewboard.mozilla.org/r/11303/#review9739 ::: gfx/layers/LayerTreeInvalidation.cpp:411 (Diff revision 1) > + if (host) { else if?
Comment 160•9 years ago
|
||
Comment on attachment 8622248 [details] MozReview Request: Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow https://reviewboard.mozilla.org/r/11303/#review9741 Ship It!
Attachment #8622248 -
Flags: review+
Updated•9 years ago
|
Attachment #8622249 -
Flags: review?(matt.woodrow) → review+
Comment 161•9 years ago
|
||
Comment on attachment 8622249 [details] MozReview Request: Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow https://reviewboard.mozilla.org/r/11305/#review9743 Ship It!
Updated•9 years ago
|
Attachment #8622250 -
Flags: review?(matt.woodrow) → review+
Comment 162•9 years ago
|
||
Comment on attachment 8622250 [details] MozReview Request: Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical https://reviewboard.mozilla.org/r/11307/#review9745 Ship It!
Comment 163•9 years ago
|
||
Wow that's a lot of reviews and I am on PTO until Whistler, I'll start doing a few reviews this week but I expect to get most of them done in Whistler, is that OK?
Assignee | ||
Comment 164•9 years ago
|
||
https://reviewboard.mozilla.org/r/11303/#review9811 > else if? Could be, but I think it's fine as is.
Assignee | ||
Comment 165•9 years ago
|
||
Sure. I understand it's a lot of reviews :-)
Comment 166•9 years ago
|
||
https://reviewboard.mozilla.org/r/11335/#review9821 ::: dom/media/MediaDecoderStateMachine.cpp:901 (Diff revision 1) > + aSample->mFrameID = ++mCurrentFrameID; Got it. ::: dom/media/MediaDecoderStateMachine.cpp:1267 (Diff revision 1) > + RenderVideoFrames(1); I still can't understand how RenderVideoFrames(1) will flush frames in the compositor. Is there any way to tell the compositor to discard frames that are queued but not rendered yet? ::: dom/media/MediaDecoderStateMachine.cpp:2972 (Diff revision 1) > + frameStats.NotifyPresentedFrame(); I see. This makes sense.
Comment 167•9 years ago
|
||
Comment on attachment 8622225 [details] MozReview Request: Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce https://reviewboard.mozilla.org/r/11257/#review9913 Ship It!
Attachment #8622225 -
Flags: review?(cpearce) → review+
Comment 168•9 years ago
|
||
https://reviewboard.mozilla.org/r/11257/#review9915 r+
Comment 169•9 years ago
|
||
Comment on attachment 8622207 [details] MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical https://reviewboard.mozilla.org/r/11221/#review9917 Ship It!
Attachment #8622207 -
Flags: review?(cpearce) → review+
Updated•9 years ago
|
Attachment #8622210 -
Flags: review?(cpearce) → review+
Comment 170•9 years ago
|
||
Comment on attachment 8622210 [details] MozReview Request: Bug 1143575. Make GL context current before cleaning up programs. r=nical https://reviewboard.mozilla.org/r/11227/#review9919 Ship It!
Comment 171•9 years ago
|
||
Comment on attachment 8622204 [details] MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical https://reviewboard.mozilla.org/r/11215/#review9925 ::: dom/canvas/WebGLContextGL.cpp:144 (Diff revision 1) > + ClearBackbufferIfNeeded(); This should not be here. ::: dom/canvas/WebGLContext.cpp:862 (Diff revision 1) > + mShouldPresent = true; I do not think we want to mark this for presentation in the absence of drawing. This will frequently cause black flashing after resize. ::: dom/canvas/WebGLContext.cpp (Diff revision 1) > - PresentScreenBuffer(); This will cause us to discard anything drawn between two resize events if there is no normal Present called between them. This could cause odd behavior when the user is drag-resizing. Lots of rendered-but-not-displayed frames.
Attachment #8622204 -
Flags: review?(jgilbert)
Assignee | ||
Comment 172•9 years ago
|
||
Alright, tell me how to fix the bug properly.
Flags: needinfo?(jgilbert)
Comment 173•9 years ago
|
||
Comment on attachment 8622231 [details] MozReview Request: Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce https://reviewboard.mozilla.org/r/11269/#review9923 ::: dom/media/MediaData.h:76 (Diff revision 1) > + bool mSentToCompositor; Why not put this on VideoData? It doesn't make sense for AudioData and MediaRawData, right? ::: dom/media/MediaDecoderStateMachine.cpp:2990 (Diff revision 1) > - // The remainingTime is negative (include zero): > + if (remainingTime < INT64_MAX) { remainingTime is an int64_t, so how could this condition ever be false?
Attachment #8622231 -
Flags: review?(cpearce)
Assignee | ||
Comment 174•9 years ago
|
||
https://reviewboard.mozilla.org/r/11335/#review9929 > I still can't understand how RenderVideoFrames(1) will flush frames in the compositor. Is there any way to tell the compositor to discard frames that are queued but not rendered yet? Yes. Every time RenderVideoFrames is called, it completely replaces the set of frames queued at the compositor. So when we call it here, we replace whatever frames are queued with a single frame.
Assignee | ||
Comment 175•9 years ago
|
||
https://reviewboard.mozilla.org/r/11269/#review9931 > Why not put this on VideoData? It doesn't make sense for AudioData and MediaRawData, right? Sure, I'll move it to VideoData. > remainingTime is an int64_t, so how could this condition ever be false? You're right. I'll remove this check.
Updated•9 years ago
|
Attachment #8622264 -
Flags: review?(cpearce)
Comment 176•9 years ago
|
||
Comment on attachment 8622264 [details] MozReview Request: Bug 1143575. Refactor UpdateRenderedVideoFrames to support pushing multiple frames from the VideoQueue to the ImageContainer. r=cpearce https://reviewboard.mozilla.org/r/11333/#review9933 ::: dom/media/MediaData.cpp:258 (Diff revision 1) > + 0)); Why it OK to pass 0 as a frame ID here and elsewhere? In what cases is it needed and not needed?
Assignee | ||
Comment 177•9 years ago
|
||
https://reviewboard.mozilla.org/r/11333/#review9935 > Why it OK to pass 0 as a frame ID here and elsewhere? In what cases is it needed and not needed? We fill the frame ID in later, in MediaDecoderStateMachine::Push.
Updated•9 years ago
|
Attachment #8622265 -
Flags: review?(cpearce)
Comment 178•9 years ago
|
||
Comment on attachment 8622265 [details] MozReview Request: Bug 1143575. Push all available frames to the compositor. r=cpearce https://reviewboard.mozilla.org/r/11335/#review10031 ::: dom/media/MediaDecoderStateMachine.cpp:2972 (Diff revision 1) > + frameStats.NotifyPresentedFrame(); "Presented" means "sent to the compositor", so not calling it for skipped frames is expected. It seems to me however, that the call to NotifyPresentedFrame() makes most sense to be made inside RenderVideoFrame() (as that's the thing that sends frames to the compositor), whenever we encounter a frame with a !mSentToCompositor field. Otherwise, we'll be calling NotifyPresentedFrame() whenever we call RenderVideoFrame(), right?
Updated•9 years ago
|
Attachment #8622226 -
Flags: review?(cpearce) → review+
Comment 179•9 years ago
|
||
Comment on attachment 8622226 [details] MozReview Request: Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce https://reviewboard.mozilla.org/r/11259/#review10033
Updated•9 years ago
|
Attachment #8622227 -
Flags: review?(cpearce) → review+
Comment 180•9 years ago
|
||
Comment on attachment 8622227 [details] MozReview Request: Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce https://reviewboard.mozilla.org/r/11261/#review10035 Ship It!
Comment 181•9 years ago
|
||
Comment on attachment 8622228 [details] MozReview Request: Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce https://reviewboard.mozilla.org/r/11263/#review10037 Ship It!
Attachment #8622228 -
Flags: review?(cpearce) → review+
Comment 182•9 years ago
|
||
Comment on attachment 8622229 [details] MozReview Request: Bug 1143575. Rename clock_time to clockTime. r=cpearce https://reviewboard.mozilla.org/r/11265/#review10039 ::: dom/media/MediaDecoderStateMachine.cpp:3325 (Diff revision 1) > mPlaybackRate = mLogicalPlaybackRate; It's a shame we're not using state mirroring for the playback rate.
Attachment #8622229 -
Flags: review?(cpearce)
Comment 183•9 years ago
|
||
Comment on attachment 8622229 [details] MozReview Request: Bug 1143575. Rename clock_time to clockTime. r=cpearce https://reviewboard.mozilla.org/r/11265/#review10041 Ship It!
Attachment #8622229 -
Flags: review+
Comment 184•9 years ago
|
||
Comment on attachment 8622230 [details] MozReview Request: Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce https://reviewboard.mozilla.org/r/11267/#review10043 Ship It!
Attachment #8622230 -
Flags: review?(cpearce) → review+
Comment 185•9 years ago
|
||
Comment on attachment 8622232 [details] MozReview Request: Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical https://reviewboard.mozilla.org/r/11271/#review10045 Ship It!
Attachment #8622232 -
Flags: review?(cpearce) → review+
Comment 186•9 years ago
|
||
Comment on attachment 8622261 [details] MozReview Request: Bug 1143575. Introduce VideoFrameContainer::SetCurrentFrames. r=cpearce https://reviewboard.mozilla.org/r/11327/#review10047 Ship It!
Attachment #8622261 -
Flags: review?(cpearce) → review+
Updated•9 years ago
|
Attachment #8622262 -
Flags: review?(cpearce) → review+
Comment 187•9 years ago
|
||
Comment on attachment 8622262 [details] MozReview Request: Bug 1143575. Add MediaQueue::GetFirstElements. r=cpearce https://reviewboard.mozilla.org/r/11329/#review10049 Ship It!
Comment 188•9 years ago
|
||
Comment on attachment 8622263 [details] MozReview Request: Bug 1143575. Add frame IDs to VideoData. r=cpearce https://reviewboard.mozilla.org/r/11331/#review10051 Ship It!
Attachment #8622263 -
Flags: review?(cpearce) → review+
Comment 189•9 years ago
|
||
Comment on attachment 8622264 [details] MozReview Request: Bug 1143575. Refactor UpdateRenderedVideoFrames to support pushing multiple frames from the VideoQueue to the ImageContainer. r=cpearce https://reviewboard.mozilla.org/r/11333/#review10055 Ship It!
Attachment #8622264 -
Flags: review+
Updated•9 years ago
|
Attachment #8622266 -
Flags: review?(cpearce) → review+
Comment 190•9 years ago
|
||
Comment on attachment 8622266 [details] MozReview Request: Bug 1143575. Add a bias value to ImageHost to avoid unpredictable results when image times and compositor times are closely aligned. r=nical https://reviewboard.mozilla.org/r/11337/#review10057 Ship It!
Comment on attachment 8622206 [details] MozReview Request: Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce https://reviewboard.mozilla.org/r/11219/#review10277 Ship It!
Attachment #8622206 -
Flags: review?(bent.mozilla) → review+
Comment 192•9 years ago
|
||
Comment on attachment 8622203 [details] MozReview Request: Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical https://reviewboard.mozilla.org/r/11213/#review10289 Ship It!
Attachment #8622203 -
Flags: review?(nical.bugzilla) → review+
Comment 193•9 years ago
|
||
Comment on attachment 8622205 [details] MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent https://reviewboard.mozilla.org/r/11217/#review10291 Ship It!
Attachment #8622205 -
Flags: review?(nical.bugzilla) → review+
Comment 194•9 years ago
|
||
Comment on attachment 8622208 [details] MozReview Request: Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical https://reviewboard.mozilla.org/r/11223/#review10299 Ship It!
Attachment #8622208 -
Flags: review?(nical.bugzilla) → review+
Comment 195•9 years ago
|
||
Comment on attachment 8622209 [details] MozReview Request: Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce https://reviewboard.mozilla.org/r/11225/#review10301 Ship It!
Attachment #8622209 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622211 -
Flags: review?(nical.bugzilla) → review+
Comment 196•9 years ago
|
||
Comment on attachment 8622211 [details] MozReview Request: Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical https://reviewboard.mozilla.org/r/11229/#review10303 Ship It!
Comment 197•9 years ago
|
||
Comment on attachment 8622212 [details] MozReview Request: Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical https://reviewboard.mozilla.org/r/11231/#review10305 Ship It!
Attachment #8622212 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622213 -
Flags: review?(nical.bugzilla) → review+
Comment 198•9 years ago
|
||
Comment on attachment 8622213 [details] MozReview Request: Bug 1143575. Remove unused CompositionNotifySink. r=nical https://reviewboard.mozilla.org/r/11233/#review10307 Ship It!
Comment 199•9 years ago
|
||
Comment on attachment 8622214 [details] MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical https://reviewboard.mozilla.org/r/11235/#review10309 Ship It!
Attachment #8622214 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622215 -
Flags: review?(nical.bugzilla) → review+
Comment 200•9 years ago
|
||
Comment on attachment 8622215 [details] MozReview Request: Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical https://reviewboard.mozilla.org/r/11237/#review10311 Ship It!
Comment 201•9 years ago
|
||
Comment on attachment 8622216 [details] MozReview Request: Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical https://reviewboard.mozilla.org/r/11239/#review10329 Ship It!
Attachment #8622216 -
Flags: review?(nical.bugzilla) → review+
Comment 202•9 years ago
|
||
Comment on attachment 8622217 [details] MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical https://reviewboard.mozilla.org/r/11241/#review10331 Ship It!
Attachment #8622217 -
Flags: review?(nical.bugzilla) → review+
Comment 203•9 years ago
|
||
Comment on attachment 8622218 [details] MozReview Request: Bug 1143575. Remove unused ReturnReleaseFence. r=nical https://reviewboard.mozilla.org/r/11243/#review10333 Ship It!
Attachment #8622218 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622219 -
Flags: review?(nical.bugzilla) → review+
Comment 204•9 years ago
|
||
Comment on attachment 8622219 [details] MozReview Request: Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow https://reviewboard.mozilla.org/r/11245/#review10335 Ship It!
Comment 205•9 years ago
|
||
Comment on attachment 8622221 [details] MozReview Request: Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical https://reviewboard.mozilla.org/r/11249/#review10337 Ship It!
Attachment #8622221 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622222 -
Flags: review?(nical.bugzilla) → review+
Comment 206•9 years ago
|
||
Comment on attachment 8622222 [details] MozReview Request: Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical https://reviewboard.mozilla.org/r/11251/#review10395 Ship It!
Comment 207•9 years ago
|
||
Comment on attachment 8622223 [details] MozReview Request: Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical https://reviewboard.mozilla.org/r/11253/#review10397 Ship It!
Attachment #8622223 -
Flags: review?(nical.bugzilla) → review+
Comment 208•9 years ago
|
||
Comment on attachment 8622224 [details] MozReview Request: Bug 1143575. Fix indent. r=cpearce https://reviewboard.mozilla.org/r/11255/#review10399 Ship It!
Attachment #8622224 -
Flags: review?(nical.bugzilla) → review+
Comment 209•9 years ago
|
||
Comment on attachment 8622233 [details] MozReview Request: Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical https://reviewboard.mozilla.org/r/11273/#review10401 Ship It!
Attachment #8622233 -
Flags: review?(nical.bugzilla) → review+
Comment 210•9 years ago
|
||
Comment on attachment 8622234 [details] MozReview Request: Bug 1143575. Fix typo in ImageContainer comment. r=nical https://reviewboard.mozilla.org/r/11275/#review10403 Ship It!
Attachment #8622234 -
Flags: review?(nical.bugzilla) → review+
Comment 211•9 years ago
|
||
Comment on attachment 8622235 [details] MozReview Request: Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical https://reviewboard.mozilla.org/r/11277/#review10405 Ship It!
Attachment #8622235 -
Flags: review?(nical.bugzilla) → review+
Comment 212•9 years ago
|
||
Comment on attachment 8622236 [details] MozReview Request: Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical https://reviewboard.mozilla.org/r/11279/#review10407 Ship It!
Attachment #8622236 -
Flags: review?(nical.bugzilla) → review+
Comment 213•9 years ago
|
||
Roc, Why are we setting the composition time on the compositor just before scheduling the composition rather than just before doing the actual composition? It's not extremely important but I'd rather have an api like Compositor::BeginFrame(..., TimeStamp aCompositionTime) than Compositor::SetCompositionTime(TimeStamp aCompositionTime).
Comment 214•9 years ago
|
||
Comment on attachment 8622244 [details] MozReview Request: Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical,sotaro https://reviewboard.mozilla.org/r/11295/#review10493 Ship It!
Attachment #8622244 -
Flags: review?(nical.bugzilla) → review+
Comment 215•9 years ago
|
||
Comment on attachment 8622243 [details] MozReview Request: Bug 1143575. Fix some code formatting. r=nical https://reviewboard.mozilla.org/r/11293/#review10495 Ship It!
Attachment #8622243 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622241 -
Flags: review?(nical.bugzilla) → review+
Comment 216•9 years ago
|
||
Comment on attachment 8622241 [details] MozReview Request: Bug 1143575. Remove ImageClientBridge::Updated. r=nical https://reviewboard.mozilla.org/r/11289/#review10501 Ship It!
Comment 217•9 years ago
|
||
Comment on attachment 8622242 [details] MozReview Request: Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical https://reviewboard.mozilla.org/r/11291/#review10503 Ship It!
Attachment #8622242 -
Flags: review?(nical.bugzilla) → review+
Comment 218•9 years ago
|
||
Comment on attachment 8622246 [details] MozReview Request: Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical https://reviewboard.mozilla.org/r/11299/#review10515 Ship It!
Attachment #8622246 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622251 -
Flags: review?(nical.bugzilla) → review+
Comment 219•9 years ago
|
||
Comment on attachment 8622251 [details] MozReview Request: Bug 1143575. Don't report negative frame delays. r=cpearce https://reviewboard.mozilla.org/r/11309/#review10523 Ship It!
Comment 220•9 years ago
|
||
Comment on attachment 8622252 [details] MozReview Request: Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical https://reviewboard.mozilla.org/r/11311/#review10525 Ship It!
Attachment #8622252 -
Flags: review?(nical.bugzilla) → review+
Comment 221•9 years ago
|
||
Comment on attachment 8622253 [details] MozReview Request: Bug 1143575. Remove ClearAllImagesExceptFront because it doesn't do anything. r=nical https://reviewboard.mozilla.org/r/11313/#review10533 Ship It! ::: gfx/layers/ImageContainer.cpp:19 (Diff revision 1) > +#include "mozilla/layers/LayersMessages.h" // for ImageClient nit: remove the "// for ImageClient", looks like a copy-pasting mistake
Attachment #8622253 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622255 -
Flags: review?(nical.bugzilla) → review+
Comment 222•9 years ago
|
||
Comment on attachment 8622255 [details] MozReview Request: Bug 1143575. Clarify code by renaming method to ClearCurrentImageFromImageBridge. r=nical https://reviewboard.mozilla.org/r/11315/#review10543 Ship It!
Assignee | ||
Comment 223•9 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #213) > Roc, Why are we setting the composition time on the compositor just before > scheduling the composition rather than just before doing the actual > composition? We set it in CompositorParent::CompositeToTarget. That's not "just before scheduling the composition" ... right?
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 224•9 years ago
|
||
https://reviewboard.mozilla.org/r/11335/#review10031 > "Presented" means "sent to the compositor", so not calling it for skipped frames is expected. > > It seems to me however, that the call to NotifyPresentedFrame() makes most sense to be made inside RenderVideoFrame() (as that's the thing that sends frames to the compositor), whenever we encounter a frame with a !mSentToCompositor field. Otherwise, we'll be calling NotifyPresentedFrame() whenever we call RenderVideoFrame(), right? We can do it that way, but that would mean for example that if we decode 10 frames, send them all to the compositor, but stop playback after the first frame has been composited but none of the others, then we'll record having presented 10 frames. Is that OK?
Assignee | ||
Comment 225•9 years ago
|
||
https://reviewboard.mozilla.org/r/11215/#review9925 > I do not think we want to mark this for presentation in the absence of drawing. > > This will frequently cause black flashing after resize. After further discussion I agree we should drop this patch and I'll adjust the picture-rect asserts to ignore canvas.
Updated•9 years ago
|
Flags: needinfo?(jgilbert)
Comment 226•9 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #223) > (In reply to Nicolas Silva [:nical] from comment #213) > > Roc, Why are we setting the composition time on the compositor just before > > scheduling the composition rather than just before doing the actual > > composition? > > We set it in CompositorParent::CompositeToTarget. That's not "just before > scheduling the composition" ... right? Indeed, my bad. Although, I still think that we should incorporate the timestamp in BeginFrame rather than in a separate method (which is not used near the code that actually calls BeginFrame and all). I am not too found of the idea that as the code evolves and moves around, there is a risk that we might start compositing without having set the proper timestamp (and end up with the previous timestamp which is not quite right but sufficiently subtle that it may take more time for us to figure out why frames seem to drop in rare cases, etc). Would that work for you?
Flags: needinfo?(nical.bugzilla) → needinfo?(roc)
Comment 228•9 years ago
|
||
Comment on attachment 8622240 [details] MozReview Request: Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical https://reviewboard.mozilla.org/r/11287/#review10841 Ship It!
Attachment #8622240 -
Flags: review?(nical.bugzilla) → review+
Comment 229•9 years ago
|
||
Comment on attachment 8622256 [details] MozReview Request: Bug 1143575. Implement ImageContainer::GetDroppedCount. r=nical https://reviewboard.mozilla.org/r/11317/#review10843 Ship It!
Attachment #8622256 -
Flags: review?(nical.bugzilla) → review+
Comment 230•9 years ago
|
||
Comment on attachment 8622257 [details] MozReview Request: Bug 1143575. Reimplement ImageContainer::GetPaintCount to be composition-aware. r=nical https://reviewboard.mozilla.org/r/11319/#review10845 Ship It!
Attachment #8622257 -
Flags: review?(nical.bugzilla) → review+
Comment 231•9 years ago
|
||
Sotaro, could you have a look at this patch https://reviewboard.mozilla.org/r/11297/diff/1#index_header ? I can't add reviewers from reviewboard (I think that only the patch subimtter can at the moment) but this part should have your approval.
Flags: needinfo?(sotaro.ikeda.g)
Updated•9 years ago
|
Attachment #8622247 -
Flags: review?(nical.bugzilla)
Comment 232•9 years ago
|
||
Comment on attachment 8622247 [details] MozReview Request: Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow https://reviewboard.mozilla.org/r/11301/#review10847 ::: gfx/layers/composite/LayerManagerComposite.h:21 (Diff revision 1) > +#include "mozilla/layers/LayersMessages.h" // for LayersBackend, etc nit: remove the comment on this line, looks like it's a copy-paste leftover from the line below. ::: gfx/layers/ipc/ShadowLayerParent.cpp:43 (Diff revision 1) > + Disconnect(); In what situation can Bind be called and the ShadowLayerParent is already be connected to something? Currently we always call Bind on a layer that we just created but this change suggests otherwise. If this situation is actually possible, wouldn't it make sense to only disconnect if layer != mLayer ?
Updated•9 years ago
|
Attachment #8622258 -
Flags: review?(nical.bugzilla) → review+
Comment 233•9 years ago
|
||
Comment on attachment 8622258 [details] MozReview Request: Bug 1143575. Let callers of ImageContainer::SetCurrentImages specify frame IDs. r=nical https://reviewboard.mozilla.org/r/11321/#review10851 Ship It!
Updated•9 years ago
|
Attachment #8622259 -
Flags: review?(nical.bugzilla) → review+
Comment 234•9 years ago
|
||
Comment on attachment 8622259 [details] MozReview Request: Bug 1143575. Let ImageContainer::SetCurrentImages accept multiple images. r=nical https://reviewboard.mozilla.org/r/11323/#review10853 ::: dom/media/VideoFrameContainer.h:81 (Diff revision 1) > + ImageContainer::FrameID mFrameID; nit: it'd be nice to have a comment here explaining why we keep track of a frame id.
Updated•9 years ago
|
Attachment #8622260 -
Flags: review?(nical.bugzilla) → review+
Comment 235•9 years ago
|
||
Comment on attachment 8622260 [details] MozReview Request: Bug 1143575. Introduce VideoFrameContainer::ClearCurrentFrame(size), and don't increment mFrameID when clearing frames. r=cpearce https://reviewboard.mozilla.org/r/11325/#review10855 Ship It!
Updated•9 years ago
|
Attachment #8622237 -
Flags: review?(nical.bugzilla) → review+
Comment 236•9 years ago
|
||
Comment on attachment 8622237 [details] MozReview Request: Bug 1143575. Store composition time in Compositor. r=nical https://reviewboard.mozilla.org/r/11281/#review10857 Ship It!
Updated•9 years ago
|
Attachment #8622239 -
Flags: review?(nical.bugzilla) → review+
Comment 237•9 years ago
|
||
Comment on attachment 8622239 [details] MozReview Request: Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical https://reviewboard.mozilla.org/r/11285/#review10859 Ship It!
Assignee | ||
Comment 238•9 years ago
|
||
https://reviewboard.mozilla.org/r/11301/#review10847 > In what situation can Bind be called and the ShadowLayerParent is already be connected to something? > Currently we always call Bind on a layer that we just created but this change suggests otherwise. If this situation is actually possible, wouldn't it make sense to only disconnect if layer != mLayer ? I don't know that Bind can be called with non-null mLayer. I was being paranoid because with the weak pointers I'm introducing here, failure to call mLayer->Disconnect would leave mLayer with a dangling pointer. I'll wrap this in a layer != mLayer check if that helps.
Assignee | ||
Comment 239•9 years ago
|
||
Comment on attachment 8622245 [details] MozReview Request: Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical
Attachment #8622245 -
Flags: review?(sotaro.ikeda.g)
Comment 240•9 years ago
|
||
Comment on attachment 8622267 [details] MozReview Request: Bug 1143575. Add a bias value to ImageHost to avoid unpredictable results when image times and compositor times are closely aligned. r=nical https://reviewboard.mozilla.org/r/11339/#review10919 Ship It!
Attachment #8622267 -
Flags: review?(nical.bugzilla) → review+
Assignee | ||
Comment 241•9 years ago
|
||
https://reviewboard.mozilla.org/r/11335/#review9719 > OK, I'll do that. Actually, this method doesn't need the lock count to be 1, so I'm just fixing the comment.
Assignee | ||
Comment 242•9 years ago
|
||
Comment on attachment 8622203 [details] MozReview Request: Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical
Attachment #8622203 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 243•9 years ago
|
||
Comment on attachment 8622204 [details] MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical On some Android versions, GraphicBuffer.h ends up including libui's hardware.h, which #defines the symbols version_minor and version_major, which are used as field names in Ogg Theora's th_info struct. Later patches will require some files to include both Theora headers and LayerTypes.h.
Attachment #8622204 -
Attachment description: MozReview Request: Bug 1143575. Resizing a WebGL canvas should trigger PresentScreenBuffer. r=jgilbert → MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical
Attachment #8622204 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 244•9 years ago
|
||
Comment on attachment 8622205 [details] MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent
Attachment #8622205 -
Attachment description: MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical → MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent
Attachment #8622205 -
Flags: review+ → review?(bent.mozilla)
Assignee | ||
Comment 245•9 years ago
|
||
Comment on attachment 8622206 [details] MozReview Request: Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce
Attachment #8622206 -
Attachment description: MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent → MozReview Request: Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce
Attachment #8622206 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 246•9 years ago
|
||
Comment on attachment 8622207 [details] MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical
Attachment #8622207 -
Attachment description: MozReview Request: Bug 1143575. Add RefBase #include to stagefright stubs. r=cpearce → MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical
Attachment #8622207 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 247•9 years ago
|
||
Comment on attachment 8622208 [details] MozReview Request: Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical
Attachment #8622208 -
Attachment description: MozReview Request: Bug 1143575. Add some #includes to avoid unified-build issues on Windows. r=nical → MozReview Request: Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical
Attachment #8622208 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 248•9 years ago
|
||
Comment on attachment 8622209 [details] MozReview Request: Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce
Attachment #8622209 -
Attachment description: MozReview Request: Bug 1143575. Add some #includes to avoid more unified-build issues on Windows. r=nical → MozReview Request: Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce
Attachment #8622209 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 249•9 years ago
|
||
Comment on attachment 8622210 [details] MozReview Request: Bug 1143575. Make GL context current before cleaning up programs. r=nical Bug 1143575. Make GL context current before cleaning up programs. r=nical Otherwise we can get a crash with the following stack: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 14711] 0x5d99974e in mozilla::gl::GLContext::BeforeGLCall (this=0x6dbf0800, funcName=0x60f251a4 <mozilla::gl::GLContext::raw_fDeleteProgram(unsigned int)::__PRETTY_FUNCTION__> "void mozilla::gl::GLContext::raw_fDeleteProgram(GLuint)") at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:683 683 MOZ_ASSERT(IsCurrent()); (gdb) where #0 0x5d99974e in mozilla::gl::GLContext::BeforeGLCall (this=0x6dbf0800, funcName=0x60f251a4 <mozilla::gl::GLContext::raw_fDeleteProgram(unsigned int)::__PRETTY_FUNCTION__> "void mozilla::gl::GLContext::raw_fDeleteProgram(GLuint)") at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:683 #1 0x5d99bed6 in mozilla::gl::GLContext::raw_fDeleteProgram (this=0x6dbf0800, program=210003) at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:2232 #2 0x5d99c10a in mozilla::gl::GLContext::fDeleteProgram (this=0x6dbf0800, program=210003) at /home/roc/mozilla-inbound/gfx/gl/GLContext.h:2270 #3 0x5daa0ae6 in mozilla::layers::ShaderProgramOGL::~ShaderProgramOGL (this=0x6d7df000, __in_chrg=<optimized out>) at /home/roc/mozilla-inbound/gfx/layers/opengl/OGLShaderProgram.cpp:491 #4 0x5da86bdc in mozilla::layers::CompositorOGL::CleanupResources (this=0x67ae4d70) at /home/roc/mozilla-inbound/gfx/layers/opengl/CompositorOGL.cpp:177
Attachment #8622210 -
Attachment description: MozReview Request: Bug 1143575. test_HaveMetadataUnbufferedSeek should not wait for canplay since preload='metadata' elements may not fire canplay. r=cpearce → MozReview Request: Bug 1143575. Make GL context current before cleaning up programs. r=nical
Attachment #8622210 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622211 -
Attachment description: MozReview Request: Bug 1143575. Make GL context current before cleaning up programs. r=nical → MozReview Request: Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical
Attachment #8622211 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 250•9 years ago
|
||
Comment on attachment 8622211 [details] MozReview Request: Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical There is some ambiguity about whether ScheduleComposite will necessarily trigger a composite all the way to nsWindow::DrawWindowUnderlay. Android robocop tests assume it will, because they rely on DrawWindowOverlay being called so they can take a screenshot and make progress, but this is a very fragile assumption. They also rely on the entire window being painted, which is also a fragile assumption. This patch improves the situation by explicitly invalidating the current window area when Android Java code needs to trigger a composite. This avoids regressions from future patches in this series which make composition bail out when there is nothing invalid. The resulting setup is still a bit fragile for my taste but I'm not sure what the ideal solution would be.
Assignee | ||
Updated•9 years ago
|
Attachment #8622212 -
Attachment description: MozReview Request: Bug 1143575. Android's screenshotting code should invalidate the LayerManagerComposite to ensure composition will actually happen. r=nical → MozReview Request: Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical
Attachment #8622212 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 251•9 years ago
|
||
Comment on attachment 8622212 [details] MozReview Request: Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622213 -
Attachment description: MozReview Request: Bug 1143575. Remove unused Image::IsSentToCompositor tracking. r=nical → MozReview Request: Bug 1143575. Remove unused CompositionNotifySink. r=nical
Attachment #8622213 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 252•9 years ago
|
||
Comment on attachment 8622213 [details] MozReview Request: Bug 1143575. Remove unused CompositionNotifySink. r=nical Bug 1143575. Remove unused CompositionNotifySink. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622214 -
Attachment description: MozReview Request: Bug 1143575. Remove unused CompositionNotifySink. r=nical → MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical
Attachment #8622214 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 253•9 years ago
|
||
Comment on attachment 8622214 [details] MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622215 -
Attachment description: MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::Reset. r=nical → MozReview Request: Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical
Attachment #8622215 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 254•9 years ago
|
||
Comment on attachment 8622215 [details] MozReview Request: Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622216 -
Attachment description: MozReview Request: Bug 1143575. Rename mAsyncTransactionTrackeres to mAsyncTransactionTrackers. r=nical → MozReview Request: Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical
Attachment #8622216 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 255•9 years ago
|
||
Comment on attachment 8622216 [details] MozReview Request: Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622217 -
Attachment description: MozReview Request: Bug 1143575. Remove unused ImageContainer::ResetPaintCount. r=nical → MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical
Attachment #8622217 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 256•9 years ago
|
||
Comment on attachment 8622217 [details] MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622218 -
Attachment description: MozReview Request: Bug 1143575. Remove unused VideoFrameContainer::ClearCurrentFrame aResetSize parameter. r=nical → MozReview Request: Bug 1143575. Remove unused ReturnReleaseFence. r=nical
Attachment #8622218 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 257•9 years ago
|
||
Comment on attachment 8622218 [details] MozReview Request: Bug 1143575. Remove unused ReturnReleaseFence. r=nical Bug 1143575. Remove unused ReturnReleaseFence. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622219 -
Attachment description: MozReview Request: Bug 1143575. Remove unused ReturnReleaseFence. r=nical → MozReview Request: Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow
Attachment #8622219 -
Flags: review+ → review?(matt.woodrow)
Assignee | ||
Comment 258•9 years ago
|
||
Comment on attachment 8622219 [details] MozReview Request: Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow
Assignee | ||
Updated•9 years ago
|
Attachment #8622220 -
Attachment description: MozReview Request: Bug 1143575. LayerManagerComposite can't get END_NO_COMPOSITE. r=mattwoodrow → MozReview Request: Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical
Attachment #8622220 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 259•9 years ago
|
||
Comment on attachment 8622220 [details] MozReview Request: Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical
Assignee | ||
Comment 260•9 years ago
|
||
Comment on attachment 8622221 [details] MozReview Request: Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical
Attachment #8622221 -
Attachment description: MozReview Request: Bug 1143575. Remove unused AttachAsyncCompositable overload. r=nical → MozReview Request: Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical
Attachment #8622221 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 261•9 years ago
|
||
Comment on attachment 8622222 [details] MozReview Request: Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical This simplifies code slightly.
Attachment #8622222 -
Attachment description: MozReview Request: Bug 1143575. Remove unused CompositableClient::OnTransaction. r=nical → MozReview Request: Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical
Attachment #8622222 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 262•9 years ago
|
||
Comment on attachment 8622223 [details] MozReview Request: Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical
Attachment #8622223 -
Attachment description: MozReview Request: Bug 1143575. Move mLayer from ImageClientBridge up into its superclass ImageClient. r=nical → MozReview Request: Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical
Attachment #8622223 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 263•9 years ago
|
||
Comment on attachment 8622224 [details] MozReview Request: Bug 1143575. Fix indent. r=cpearce Bug 1143575. Fix indent. r=cpearce
Attachment #8622224 -
Attachment description: MozReview Request: Bug 1143575. Convert SetCurrentImage(nullptr) callers to call ClearAllImages instead. r=nical → MozReview Request: Bug 1143575. Fix indent. r=cpearce
Attachment #8622224 -
Flags: review+ → review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622225 -
Attachment description: MozReview Request: Bug 1143575. Fix indent. r=cpearce → MozReview Request: Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce
Attachment #8622225 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 264•9 years ago
|
||
Comment on attachment 8622225 [details] MozReview Request: Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce
Assignee | ||
Updated•9 years ago
|
Attachment #8622226 -
Attachment description: MozReview Request: Bug 1143575. Remove Theora-only duplicate frame optimization. r=cpearce → MozReview Request: Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce
Attachment #8622226 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 265•9 years ago
|
||
Comment on attachment 8622226 [details] MozReview Request: Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce
Assignee | ||
Updated•9 years ago
|
Attachment #8622227 -
Attachment description: MozReview Request: Bug 1143575. Rename AdvanceFrame to UpdateRenderedVideoFrames. r=cpearce → MozReview Request: Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce
Attachment #8622227 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 266•9 years ago
|
||
Comment on attachment 8622227 [details] MozReview Request: Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce This makes MediaDecoderStateMachine::GetVideoStreamPosition compute a time that's more consistent with the audio clock.
Assignee | ||
Updated•9 years ago
|
Attachment #8622228 -
Attachment description: MozReview Request: Bug 1143575. Make GetClock return a TimeStamp as well as the stream time. r=cpearce → MozReview Request: Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce
Attachment #8622228 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 267•9 years ago
|
||
Comment on attachment 8622228 [details] MozReview Request: Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce
Assignee | ||
Comment 268•9 years ago
|
||
Comment on attachment 8622229 [details] MozReview Request: Bug 1143575. Rename clock_time to clockTime. r=cpearce Bug 1143575. Rename clock_time to clockTime. r=cpearce
Attachment #8622229 -
Attachment description: MozReview Request: Bug 1143575. ScheduleStateMachine when the playback rate changes, so we can update the rendered frame queue. r=cpearce → MozReview Request: Bug 1143575. Rename clock_time to clockTime. r=cpearce
Attachment #8622229 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 269•9 years ago
|
||
Comment on attachment 8622230 [details] MozReview Request: Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce This makes normal playback consistent with the buffering state, which already does this. We'll also need this when we handle multiple images, because then we need to hande the entire queue of images to the ImageContainer without pulling any of them off the queue.
Attachment #8622230 -
Attachment description: MozReview Request: Bug 1143575. Rename clock_time to clockTime. r=cpearce → MozReview Request: Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce
Attachment #8622230 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 270•9 years ago
|
||
Comment on attachment 8622231 [details] MozReview Request: Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce
Attachment #8622231 -
Attachment description: MozReview Request: Bug 1143575. Keep currently-rendered frame at the front of the video queue. r=cpearce → MozReview Request: Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce
Attachment #8622231 -
Flags: review?(cpearce)
Assignee | ||
Comment 271•9 years ago
|
||
Comment on attachment 8622232 [details] MozReview Request: Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical The picture rect logically belongs with the texture, and later patches will make OpUseTexture take multiple textures, each of which needs its own picture rect.
Attachment #8622232 -
Attachment description: MozReview Request: Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce → MozReview Request: Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical
Attachment #8622232 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 272•9 years ago
|
||
Comment on attachment 8622203 [details] MozReview Request: Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical Bug 1143575. #include nsDebug.h in YCbCrImageDataSerializer.cpp for NS_WARN_IF. r=nical
Assignee | ||
Comment 273•9 years ago
|
||
Comment on attachment 8622233 [details] MozReview Request: Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical
Attachment #8622233 -
Attachment description: MozReview Request: Bug 1143575. Pass a picture rect with OpUseOverlaySource and OpUseTexture, and eliminate OpUpdatePictureRect. r=nical → MozReview Request: Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical
Attachment #8622233 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 274•9 years ago
|
||
Comment on attachment 8622204 [details] MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical On some Android versions, GraphicBuffer.h ends up including libui's hardware.h, which #defines the symbols version_minor and version_major, which are used as field names in Ogg Theora's th_info struct. Later patches will require some files to include both Theora headers and LayerTypes.h.
Assignee | ||
Comment 275•9 years ago
|
||
Comment on attachment 8622234 [details] MozReview Request: Bug 1143575. Fix typo in ImageContainer comment. r=nical Bug 1143575. Fix typo in ImageContainer comment. r=nical
Attachment #8622234 -
Attachment description: MozReview Request: Bug 1143575. Rename ImageBridgeChild's AutoRemoteTextures to AutoRemoveTexturesFromImageBridge to avoid clashes with later work. r=nical → MozReview Request: Bug 1143575. Fix typo in ImageContainer comment. r=nical
Attachment #8622234 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 276•9 years ago
|
||
Comment on attachment 8622205 [details] MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent
Assignee | ||
Comment 277•9 years ago
|
||
Comment on attachment 8622235 [details] MozReview Request: Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical
Attachment #8622235 -
Attachment description: MozReview Request: Bug 1143575. Fix typo in ImageContainer comment. r=nical → MozReview Request: Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical
Attachment #8622235 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 278•9 years ago
|
||
Comment on attachment 8622236 [details] MozReview Request: Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical
Attachment #8622236 -
Attachment description: MozReview Request: Bug 1143575. Replace ImageContainer Lock methods with simplified AutoLockImage. r=nical → MozReview Request: Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical
Attachment #8622236 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 279•9 years ago
|
||
Comment on attachment 8622237 [details] MozReview Request: Bug 1143575. Store composition time in Compositor. r=nical Bug 1143575. Store composition time in Compositor. r=nical We'll need this later so ImageHost can select the correct image to use. Adding a TimeStamp parameter to BeginFrame is a bit annoying since BeginFrame is overridden by every subclass. It's a bit more convenient to just call a separate non-virtual method just before we call BeginFrame.
Attachment #8622237 -
Attachment description: MozReview Request: Bug 1143575. Extend IPDL OpUseTexture to support multiple timestamped images. r=nical → MozReview Request: Bug 1143575. Store composition time in Compositor. r=nical
Attachment #8622237 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 280•9 years ago
|
||
Comment on attachment 8622238 [details] MozReview Request: Bug 1143575. Implement ImageHost support for multiple timed images. r=nical Bug 1143575. Implement ImageHost support for multiple timed images. r=nical
Attachment #8622238 -
Attachment description: MozReview Request: Bug 1143575. Store composition time in Compositor. r=nical → MozReview Request: Bug 1143575. Implement ImageHost support for multiple timed images. r=nical
Assignee | ||
Comment 281•9 years ago
|
||
Comment on attachment 8622239 [details] MozReview Request: Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical
Attachment #8622239 -
Attachment description: MozReview Request: Bug 1143575. Implement ImageHost support for multiple timed images. r=nical → MozReview Request: Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical
Attachment #8622239 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622240 -
Attachment description: MozReview Request: Bug 1143575. Ensure we schedule another composite if ImageHost has pending images. r=nical → MozReview Request: Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical
Attachment #8622240 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 282•9 years ago
|
||
Comment on attachment 8622240 [details] MozReview Request: Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical When ImageContainer and ImageClient are managing a list of images, the individual Image serial numbers are no longer enough to detect whether the state has changed.
Assignee | ||
Updated•9 years ago
|
Attachment #8622241 -
Attachment description: MozReview Request: Bug 1143575. Replace ImageClientSingle::UpdateImage's use of Image serial numbers with ImageContainer state generation counters, and switch it to use ImageContainer::GetCurrentImages. r=nical → MozReview Request: Bug 1143575. Remove ImageClientBridge::Updated. r=nical
Attachment #8622241 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 283•9 years ago
|
||
Comment on attachment 8622241 [details] MozReview Request: Bug 1143575. Remove ImageClientBridge::Updated. r=nical Bug 1143575. Remove ImageClientBridge::Updated. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622242 -
Attachment description: MozReview Request: Bug 1143575. Remove ImageClientBridge::Updated. r=nical → MozReview Request: Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical
Attachment #8622242 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 284•9 years ago
|
||
Comment on attachment 8622242 [details] MozReview Request: Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical
Assignee | ||
Comment 285•9 years ago
|
||
Comment on attachment 8622243 [details] MozReview Request: Bug 1143575. Fix some code formatting. r=nical Bug 1143575. Fix some code formatting. r=nical
Attachment #8622243 -
Attachment description: MozReview Request: Bug 1143575. ImageClient::UpdateImage should not return false when there's no image, because recreating the ImageClient won't help. r=nical → MozReview Request: Bug 1143575. Fix some code formatting. r=nical
Attachment #8622243 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 286•9 years ago
|
||
Comment on attachment 8622244 [details] MozReview Request: Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical,sotaro Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical,sotaro
Attachment #8622244 -
Attachment description: MozReview Request: Bug 1143575. Fix some code formatting. r=nical → MozReview Request: Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical,sotaro
Attachment #8622244 -
Flags: review?(sotaro.ikeda.g)
Attachment #8622244 -
Flags: review?(nical.bugzilla)
Attachment #8622244 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8622245 -
Attachment description: MozReview Request: Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical → MozReview Request: Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical
Attachment #8622245 -
Flags: review?(sotaro.ikeda.g)
Assignee | ||
Comment 287•9 years ago
|
||
Comment on attachment 8622245 [details] MozReview Request: Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622246 -
Attachment description: MozReview Request: Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical → MozReview Request: Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical
Attachment #8622246 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 288•9 years ago
|
||
Comment on attachment 8622246 [details] MozReview Request: Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical For frame statistics to work properly, we have to notify an ImageContainer when it has been composited. This requires a few changes, which have been lumped together in this patch: -- Create PImageContainer and ImageContainerParent/ImageContainerChild. -- Add mFrameID and mProducerID everywhere we're passing around images. -- Route composition notifications from the compositor back to ImageContainerChild.
Assignee | ||
Comment 289•9 years ago
|
||
Comment on attachment 8622247 [details] MozReview Request: Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow
Attachment #8622247 -
Attachment description: MozReview Request: Bug 1143575. Route ImageCompositeNotifications to ImageContainers. r=nical → MozReview Request: Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow
Attachment #8622247 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 290•9 years ago
|
||
Comment on attachment 8622248 [details] MozReview Request: Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow We need this change so that when ImageHost has a next image to display more than one composition-interval in the future, we skip the actual compositing work in those intermediate composition(s) if nothing else has changed. This change is a little bit scary since it breaks any code that was previously assuming ScheduleComposition would actually update the screen. However, that code was already broken for BasicCompositor.
Attachment #8622248 -
Attachment description: MozReview Request: Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow → MozReview Request: Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow
Attachment #8622248 -
Flags: review+ → review?(matt.woodrow)
Assignee | ||
Comment 291•9 years ago
|
||
Comment on attachment 8622249 [details] MozReview Request: Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow We need to remove this so that adding images to the end of the list of images for an ImageLayer doesn't force composition to happen even if nothing else has changed.
Attachment #8622249 -
Attachment description: MozReview Request: Bug 1143575. Exit composition early if nothing is invalid. r=mattwoodrow → MozReview Request: Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow
Attachment #8622249 -
Flags: review+ → review?(matt.woodrow)
Assignee | ||
Updated•9 years ago
|
Attachment #8622250 -
Attachment description: MozReview Request: Bug 1143575. Async image invalidation does not necessarily need to invalidate the layer; LayerTreeInvalidation will do that for us. r=mattwoodrow → MozReview Request: Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical
Attachment #8622250 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 292•9 years ago
|
||
Comment on attachment 8622250 [details] MozReview Request: Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical
Assignee | ||
Comment 293•9 years ago
|
||
Comment on attachment 8622251 [details] MozReview Request: Bug 1143575. Don't report negative frame delays. r=cpearce Bug 1143575. Don't report negative frame delays. r=cpearce
Attachment #8622251 -
Attachment description: MozReview Request: Bug 1143575. Pass a list of timestamped images to ImageContainer::SetCurrentImages. r=nical → MozReview Request: Bug 1143575. Don't report negative frame delays. r=cpearce
Attachment #8622251 -
Flags: review+ → review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622252 -
Attachment description: MozReview Request: Bug 1143575. Don't report negative frame delays. r=cpearce → MozReview Request: Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical
Attachment #8622252 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 294•9 years ago
|
||
Comment on attachment 8622252 [details] MozReview Request: Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical
Assignee | ||
Comment 295•9 years ago
|
||
Comment on attachment 8622253 [details] MozReview Request: Bug 1143575. Remove ClearAllImagesExceptFront because it doesn't do anything. r=nical Bug 1143575. Remove ClearAllImagesExceptFront because it doesn't do anything. r=nical ImageBridgeChild::FlushAllImages with aExceptFront==true does absolutely nothing, so remove the parameter and remove all callers which pass true.
Attachment #8622253 -
Attachment description: MozReview Request: Bug 1143575. Implement ImageContainer::GetPaintDelay. r=nical → MozReview Request: Bug 1143575. Remove ClearAllImagesExceptFront because it doesn't do anything. r=nical
Attachment #8622253 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 296•9 years ago
|
||
Comment on attachment 8622255 [details] MozReview Request: Bug 1143575. Clarify code by renaming method to ClearCurrentImageFromImageBridge. r=nical Bug 1143575. Clarify code by renaming method to ClearCurrentImageFromImageBridge. r=nical We need to make it clear that ClearCurrentImage is really an internal method of the ImageContainer implementation, not a method that ImageContainer users should call.
Attachment #8622255 -
Attachment description: MozReview Request: Bug 1143575. Remove ClearAllImagesExceptFront because it doesn't do anything. r=nical → MozReview Request: Bug 1143575. Clarify code by renaming method to ClearCurrentImageFromImageBridge. r=nical
Attachment #8622255 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 297•9 years ago
|
||
Comment on attachment 8622256 [details] MozReview Request: Bug 1143575. Implement ImageContainer::GetDroppedCount. r=nical Bug 1143575. Implement ImageContainer::GetDroppedCount. r=nical
Attachment #8622256 -
Attachment description: MozReview Request: Bug 1143575. Clarify code by renaming method to ClearCurrentImageFromImageBridge. r=nical → MozReview Request: Bug 1143575. Implement ImageContainer::GetDroppedCount. r=nical
Attachment #8622256 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 298•9 years ago
|
||
Comment on attachment 8622257 [details] MozReview Request: Bug 1143575. Reimplement ImageContainer::GetPaintCount to be composition-aware. r=nical Bug 1143575. Reimplement ImageContainer::GetPaintCount to be composition-aware. r=nical
Attachment #8622257 -
Attachment description: MozReview Request: Bug 1143575. Implement ImageContainer::GetDroppedCount. r=nical → MozReview Request: Bug 1143575. Reimplement ImageContainer::GetPaintCount to be composition-aware. r=nical
Attachment #8622257 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 299•9 years ago
|
||
Comment on attachment 8622258 [details] MozReview Request: Bug 1143575. Let callers of ImageContainer::SetCurrentImages specify frame IDs. r=nical Bug 1143575. Let callers of ImageContainer::SetCurrentImages specify frame IDs. r=nical
Attachment #8622258 -
Attachment description: MozReview Request: Bug 1143575. Reimplement ImageContainer::GetPaintCount to be composition-aware. r=nical → MozReview Request: Bug 1143575. Let callers of ImageContainer::SetCurrentImages specify frame IDs. r=nical
Attachment #8622258 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622259 -
Attachment description: MozReview Request: Bug 1143575. Let callers of ImageContainer::SetCurrentImages specify frame IDs. r=nical → MozReview Request: Bug 1143575. Let ImageContainer::SetCurrentImages accept multiple images. r=nical
Attachment #8622259 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 300•9 years ago
|
||
Comment on attachment 8622259 [details] MozReview Request: Bug 1143575. Let ImageContainer::SetCurrentImages accept multiple images. r=nical Bug 1143575. Let ImageContainer::SetCurrentImages accept multiple images. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622260 -
Attachment description: MozReview Request: Bug 1143575. Let ImageContainer::SetCurrentImages accept multiple images. r=nical → MozReview Request: Bug 1143575. Introduce VideoFrameContainer::ClearCurrentFrame(size), and don't increment mFrameID when clearing frames. r=cpearce
Attachment #8622260 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 301•9 years ago
|
||
Comment on attachment 8622260 [details] MozReview Request: Bug 1143575. Introduce VideoFrameContainer::ClearCurrentFrame(size), and don't increment mFrameID when clearing frames. r=cpearce Bug 1143575. Introduce VideoFrameContainer::ClearCurrentFrame(size), and don't increment mFrameID when clearing frames. r=cpearce
Assignee | ||
Comment 302•9 years ago
|
||
Comment on attachment 8622261 [details] MozReview Request: Bug 1143575. Introduce VideoFrameContainer::SetCurrentFrames. r=cpearce Bug 1143575. Introduce VideoFrameContainer::SetCurrentFrames. r=cpearce
Attachment #8622261 -
Attachment description: MozReview Request: Bug 1143575. Introduce VideoFrameContainer::ClearCurrentFrame(size), and don't increment mFrameID when clearing frames. r=cpearce → MozReview Request: Bug 1143575. Introduce VideoFrameContainer::SetCurrentFrames. r=cpearce
Attachment #8622261 -
Flags: review+ → review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622262 -
Attachment description: MozReview Request: Bug 1143575. Introduce VideoFrameContainer::SetCurrentFrames. r=cpearce → MozReview Request: Bug 1143575. Add MediaQueue::GetFirstElements. r=cpearce
Attachment #8622262 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 303•9 years ago
|
||
Comment on attachment 8622262 [details] MozReview Request: Bug 1143575. Add MediaQueue::GetFirstElements. r=cpearce Bug 1143575. Add MediaQueue::GetFirstElements. r=cpearce
Assignee | ||
Updated•9 years ago
|
Attachment #8622263 -
Attachment description: MozReview Request: Bug 1143575. Add MediaQueue::GetFirstElements. r=cpearce → MozReview Request: Bug 1143575. Add frame IDs to VideoData. r=cpearce
Attachment #8622263 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 304•9 years ago
|
||
Comment on attachment 8622263 [details] MozReview Request: Bug 1143575. Add frame IDs to VideoData. r=cpearce Bug 1143575. Add frame IDs to VideoData. r=cpearce
Assignee | ||
Updated•9 years ago
|
Attachment #8622264 -
Attachment description: MozReview Request: Bug 1143575. Add frame IDs to VideoData. r=cpearce → MozReview Request: Bug 1143575. Refactor UpdateRenderedVideoFrames to support pushing multiple frames from the VideoQueue to the ImageContainer. r=cpearce
Attachment #8622264 -
Flags: review+ → review?(cpearce)
Assignee | ||
Comment 305•9 years ago
|
||
Comment on attachment 8622264 [details] MozReview Request: Bug 1143575. Refactor UpdateRenderedVideoFrames to support pushing multiple frames from the VideoQueue to the ImageContainer. r=cpearce Bug 1143575. Refactor UpdateRenderedVideoFrames to support pushing multiple frames from the VideoQueue to the ImageContainer. r=cpearce
Assignee | ||
Updated•9 years ago
|
Attachment #8622265 -
Attachment description: MozReview Request: Bug 1143575. Refactor UpdateRenderedVideoFrames to support pushing multiple frames from the VideoQueue to the ImageContainer. r=cpearce → MozReview Request: Bug 1143575. Push all available frames to the compositor. r=cpearce
Attachment #8622265 -
Flags: review?(cpearce)
Assignee | ||
Comment 306•9 years ago
|
||
Comment on attachment 8622265 [details] MozReview Request: Bug 1143575. Push all available frames to the compositor. r=cpearce Bug 1143575. Push all available frames to the compositor. r=cpearce
Assignee | ||
Updated•9 years ago
|
Attachment #8622266 -
Attachment description: MozReview Request: Bug 1143575. Push all available frames to the compositor. r=cpearce → MozReview Request: Bug 1143575. Add a bias value to ImageHost to avoid unpredictable results when image times and compositor times are closely aligned. r=nical
Attachment #8622266 -
Flags: review+ → review?(nical.bugzilla)
Assignee | ||
Comment 307•9 years ago
|
||
Comment on attachment 8622266 [details] MozReview Request: Bug 1143575. Add a bias value to ImageHost to avoid unpredictable results when image times and compositor times are closely aligned. r=nical Bug 1143575. Add a bias value to ImageHost to avoid unpredictable results when image times and compositor times are closely aligned. r=nical
Assignee | ||
Updated•9 years ago
|
Attachment #8622267 -
Attachment is obsolete: true
Assignee | ||
Comment 308•9 years ago
|
||
I think for now we should just ignore the Bugzilla r? state and get updated reviews for the patches not already ticked green in https://reviewboard.mozilla.org/r/11211/.
Assignee | ||
Comment 309•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b1f9d7add728
Comment 310•9 years ago
|
||
Comment on attachment 8622244 [details] MozReview Request: Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical,sotaro https://reviewboard.mozilla.org/r/11295/#review11055 Ship It!
Attachment #8622244 -
Flags: review?(nical.bugzilla) → review+
Comment 311•9 years ago
|
||
Comment on attachment 8622204 [details] MozReview Request: Bug 1143575. Avoid including Android's GraphicBuffer.h from LayersTypes.h. r=nical https://reviewboard.mozilla.org/r/11215/#review11057 Ship It!
Attachment #8622204 -
Flags: review?(nical.bugzilla) → review+
Comment 312•9 years ago
|
||
Comment on attachment 8622238 [details] MozReview Request: Bug 1143575. Implement ImageHost support for multiple timed images. r=nical https://reviewboard.mozilla.org/r/11283/#review11059 Ship It!
Attachment #8622238 -
Flags: review?(nical.bugzilla) → review+
Updated•9 years ago
|
Attachment #8622245 -
Flags: review?(nical.bugzilla) → review+
Comment 313•9 years ago
|
||
Comment on attachment 8622245 [details] MozReview Request: Bug 1143575. Make ImageClientSingle handle multiple textures. r=nical https://reviewboard.mozilla.org/r/11297/#review11061 Ship It!
Updated•9 years ago
|
Flags: needinfo?(sotaro.ikeda.g)
Comment 314•9 years ago
|
||
Comment on attachment 8622247 [details] MozReview Request: Bug 1143575. Make LayerTreeInvalidation invalidate when an ImageLayerComposite's current frame has changed. r=mattwoodrow https://reviewboard.mozilla.org/r/11301/#review11067 Ship It!
Attachment #8622247 -
Flags: review?(matt.woodrow) → review+
Updated•9 years ago
|
Attachment #8622219 -
Flags: review?(matt.woodrow) → review+
Updated•9 years ago
|
Attachment #8622248 -
Flags: review?(matt.woodrow) → review+
Updated•9 years ago
|
Attachment #8622249 -
Flags: review?(matt.woodrow) → review+
Attachment #8622205 -
Flags: review?(bent.mozilla) → review+
Comment on attachment 8622205 [details] MozReview Request: Bug 1143575. Avoid use of COMPARE macro which can clash with Android headers. r=bent https://reviewboard.mozilla.org/r/11217/#review11075 Ship It!
Comment 316•9 years ago
|
||
Comment on attachment 8622244 [details] MozReview Request: Bug 1143575. Factor out AsyncTransactionWaiter from AsyncTransactionTracker so we'll be able to wait for multiple AsyncTransactionTrackers. r=nical,sotaro Looks good.
Attachment #8622244 -
Flags: review?(sotaro.ikeda.g) → review+
Comment 317•9 years ago
|
||
Comment on attachment 8622231 [details] MozReview Request: Bug 1143575. Remove unused MediaQueue::Empty. r=cpearce https://reviewboard.mozilla.org/r/11269/#review11099 Ship It!
Attachment #8622231 -
Flags: review?(cpearce) → review+
Comment 318•9 years ago
|
||
Comment on attachment 8622265 [details] MozReview Request: Bug 1143575. Push all available frames to the compositor. r=cpearce https://reviewboard.mozilla.org/r/11335/#review11101 Ship It!
Attachment #8622265 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 319•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2eaf88c0ceb2
Assignee | ||
Comment 320•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d0904a17d17e
Assignee | ||
Comment 321•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=df70ddcb8933
Assignee | ||
Comment 322•9 years ago
|
||
> Adding a TimeStamp parameter to BeginFrame is a bit annoying since BeginFrame
> is overridden by every subclass. It's a bit more convenient to just call a
> separate non-virtual method just before we call BeginFrame.
Actually even calling SetCompositionTime just before BeginFrame broke things. We need to set the composition time before we call ComputeEffectiveTransforms, since ImageHost must get the correct composition time to choose the right frame to render so it computes the transform needed to stretch that frame to the required size. So I moved SetCompositionTime to just before we call ComputeEffectiveTransforms in EndTransaction.
I have added a few other trivial patches to fix rot.
Assignee | ||
Comment 323•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e14d68e1e4a3 https://hg.mozilla.org/integration/mozilla-inbound/rev/ba169610eb8f https://hg.mozilla.org/integration/mozilla-inbound/rev/c7002e70d949 https://hg.mozilla.org/integration/mozilla-inbound/rev/c31af82d8b9d https://hg.mozilla.org/integration/mozilla-inbound/rev/47592e40ee7e https://hg.mozilla.org/integration/mozilla-inbound/rev/b05b5b0808e9 https://hg.mozilla.org/integration/mozilla-inbound/rev/4c68f224513a https://hg.mozilla.org/integration/mozilla-inbound/rev/b745c996a71d https://hg.mozilla.org/integration/mozilla-inbound/rev/3527e6013de9 https://hg.mozilla.org/integration/mozilla-inbound/rev/6ecf73959121 https://hg.mozilla.org/integration/mozilla-inbound/rev/a61363797ed6 https://hg.mozilla.org/integration/mozilla-inbound/rev/5e6f54bca784 https://hg.mozilla.org/integration/mozilla-inbound/rev/b88e4fdc2033 https://hg.mozilla.org/integration/mozilla-inbound/rev/64f323051975 https://hg.mozilla.org/integration/mozilla-inbound/rev/38a97a683d65 https://hg.mozilla.org/integration/mozilla-inbound/rev/1494a4e9ab4f https://hg.mozilla.org/integration/mozilla-inbound/rev/7a2b4b9c9571 https://hg.mozilla.org/integration/mozilla-inbound/rev/174624481eb3 https://hg.mozilla.org/integration/mozilla-inbound/rev/e72d2ec123b4 https://hg.mozilla.org/integration/mozilla-inbound/rev/c119db5addfe https://hg.mozilla.org/integration/mozilla-inbound/rev/e306e8d40085 https://hg.mozilla.org/integration/mozilla-inbound/rev/2fd5bc5beb76 https://hg.mozilla.org/integration/mozilla-inbound/rev/175dd79bf104 https://hg.mozilla.org/integration/mozilla-inbound/rev/1a1be20110d8 https://hg.mozilla.org/integration/mozilla-inbound/rev/7d318eb52845 https://hg.mozilla.org/integration/mozilla-inbound/rev/7c82d4f1b34a https://hg.mozilla.org/integration/mozilla-inbound/rev/2c5ee8e49c60 https://hg.mozilla.org/integration/mozilla-inbound/rev/3577cfb4353f https://hg.mozilla.org/integration/mozilla-inbound/rev/edc00000dd20 https://hg.mozilla.org/integration/mozilla-inbound/rev/780ea52b2d36 https://hg.mozilla.org/integration/mozilla-inbound/rev/30fbb3f4be2e https://hg.mozilla.org/integration/mozilla-inbound/rev/9eb9996b1cd4 https://hg.mozilla.org/integration/mozilla-inbound/rev/e587e6bd4d6f https://hg.mozilla.org/integration/mozilla-inbound/rev/44927f617616 https://hg.mozilla.org/integration/mozilla-inbound/rev/03a8e48f4134 https://hg.mozilla.org/integration/mozilla-inbound/rev/c7462889f82f https://hg.mozilla.org/integration/mozilla-inbound/rev/62f44f63b6a5 https://hg.mozilla.org/integration/mozilla-inbound/rev/98cece19d4fe https://hg.mozilla.org/integration/mozilla-inbound/rev/1fe6ff5de12e https://hg.mozilla.org/integration/mozilla-inbound/rev/11b18fa3141b https://hg.mozilla.org/integration/mozilla-inbound/rev/d1433100e89d https://hg.mozilla.org/integration/mozilla-inbound/rev/15bc0858e2be https://hg.mozilla.org/integration/mozilla-inbound/rev/3b2ed2e93f49 https://hg.mozilla.org/integration/mozilla-inbound/rev/8ee029e39ce4 https://hg.mozilla.org/integration/mozilla-inbound/rev/07ee1bf0f982 https://hg.mozilla.org/integration/mozilla-inbound/rev/2d10765f4a82 https://hg.mozilla.org/integration/mozilla-inbound/rev/73e94ff36e19 https://hg.mozilla.org/integration/mozilla-inbound/rev/b23a779852ae https://hg.mozilla.org/integration/mozilla-inbound/rev/bea2adc0a077 https://hg.mozilla.org/integration/mozilla-inbound/rev/84481760a4de https://hg.mozilla.org/integration/mozilla-inbound/rev/cf230e06ace0 https://hg.mozilla.org/integration/mozilla-inbound/rev/2c12696522b7 https://hg.mozilla.org/integration/mozilla-inbound/rev/1edd02746cd2 https://hg.mozilla.org/integration/mozilla-inbound/rev/057cca533e0b https://hg.mozilla.org/integration/mozilla-inbound/rev/d5aab92845f1 https://hg.mozilla.org/integration/mozilla-inbound/rev/ced4d3f1a189 https://hg.mozilla.org/integration/mozilla-inbound/rev/aa8932f93615 https://hg.mozilla.org/integration/mozilla-inbound/rev/8adcb7e36c7c https://hg.mozilla.org/integration/mozilla-inbound/rev/a851d6125035 https://hg.mozilla.org/integration/mozilla-inbound/rev/8679a837bb3d https://hg.mozilla.org/integration/mozilla-inbound/rev/bc557cdb8c1b https://hg.mozilla.org/integration/mozilla-inbound/rev/5ec0906f885c https://hg.mozilla.org/integration/mozilla-inbound/rev/30b267369242 https://hg.mozilla.org/integration/mozilla-inbound/rev/a082d7f6cfa2 https://hg.mozilla.org/integration/mozilla-inbound/rev/ef7cb62b5b23 https://hg.mozilla.org/integration/mozilla-inbound/rev/f10665af6c50
Comment 324•9 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) (offline July 8-10) from comment #322) > > Adding a TimeStamp parameter to BeginFrame is a bit annoying since BeginFrame > > is overridden by every subclass. It's a bit more convenient to just call a > > separate non-virtual method just before we call BeginFrame. > > Actually even calling SetCompositionTime just before BeginFrame broke > things. We need to set the composition time before we call > ComputeEffectiveTransforms, since ImageHost must get the correct composition > time to choose the right frame to render so it computes the transform needed > to stretch that frame to the required size. So I moved SetCompositionTime to > just before we call ComputeEffectiveTransforms in EndTransaction. Fair enough. If there are (non-trivial) dependencies between the order of method calls please explicit them in a comment. Having a SetCompositiongTime method is not my favourite style but it's not the end of the world :)
Assignee | ||
Updated•9 years ago
|
Attachment #8622203 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622206 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622207 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622208 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622209 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622210 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622211 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622212 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622213 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622214 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622215 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622216 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622217 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622218 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622220 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622221 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622222 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622223 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622224 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622225 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622226 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622227 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622228 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622229 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622230 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622232 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622233 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622234 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622235 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622236 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622237 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622239 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622240 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622241 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622242 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622243 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622246 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622250 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622251 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622252 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622253 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622255 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622256 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622257 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622258 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622259 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•9 years ago
|
Attachment #8622260 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622261 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622262 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622263 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622264 -
Flags: review?(cpearce)
Assignee | ||
Updated•9 years ago
|
Attachment #8622266 -
Flags: review?(nical.bugzilla)
Comment 325•9 years ago
|
||
Hi, in B2G Desktop (I haven't tried B2G) we don't get the "error" event in case a video does not load. Testcase: https://everlong.org/mozilla/test-video-error.html
Comment 326•9 years ago
|
||
Mmm sorry, it looks like I don't get the error event in previous versions either. Still investigating why this makes bug 1108507 not intermittent then...
Comment 327•9 years ago
|
||
I don't know yet if this is related to the issue we have in the unit tests, but I noticed that we have an invalidation problem. For example load any video inside Firefox or B2G with your patch: http://techslides.com/demos/sample-videos/small.webm The video does not update properly with the patch. It updates if I move the mouse cursor on top of the video. (I'm on Linux, in case this is useful).
Comment 328•9 years ago
|
||
Bug 1181201 filed to track the issues with tutorial_test.js.
https://hg.mozilla.org/mozilla-central/rev/e14d68e1e4a3 https://hg.mozilla.org/mozilla-central/rev/ba169610eb8f https://hg.mozilla.org/mozilla-central/rev/c7002e70d949 https://hg.mozilla.org/mozilla-central/rev/c31af82d8b9d https://hg.mozilla.org/mozilla-central/rev/47592e40ee7e https://hg.mozilla.org/mozilla-central/rev/b05b5b0808e9 https://hg.mozilla.org/mozilla-central/rev/4c68f224513a https://hg.mozilla.org/mozilla-central/rev/b745c996a71d https://hg.mozilla.org/mozilla-central/rev/3527e6013de9 https://hg.mozilla.org/mozilla-central/rev/6ecf73959121 https://hg.mozilla.org/mozilla-central/rev/a61363797ed6 https://hg.mozilla.org/mozilla-central/rev/5e6f54bca784 https://hg.mozilla.org/mozilla-central/rev/b88e4fdc2033 https://hg.mozilla.org/mozilla-central/rev/64f323051975 https://hg.mozilla.org/mozilla-central/rev/38a97a683d65 https://hg.mozilla.org/mozilla-central/rev/1494a4e9ab4f https://hg.mozilla.org/mozilla-central/rev/7a2b4b9c9571 https://hg.mozilla.org/mozilla-central/rev/174624481eb3 https://hg.mozilla.org/mozilla-central/rev/e72d2ec123b4 https://hg.mozilla.org/mozilla-central/rev/c119db5addfe https://hg.mozilla.org/mozilla-central/rev/e306e8d40085 https://hg.mozilla.org/mozilla-central/rev/2fd5bc5beb76 https://hg.mozilla.org/mozilla-central/rev/175dd79bf104 https://hg.mozilla.org/mozilla-central/rev/1a1be20110d8 https://hg.mozilla.org/mozilla-central/rev/7d318eb52845 https://hg.mozilla.org/mozilla-central/rev/7c82d4f1b34a https://hg.mozilla.org/mozilla-central/rev/2c5ee8e49c60 https://hg.mozilla.org/mozilla-central/rev/3577cfb4353f https://hg.mozilla.org/mozilla-central/rev/edc00000dd20 https://hg.mozilla.org/mozilla-central/rev/780ea52b2d36 https://hg.mozilla.org/mozilla-central/rev/30fbb3f4be2e https://hg.mozilla.org/mozilla-central/rev/9eb9996b1cd4 https://hg.mozilla.org/mozilla-central/rev/e587e6bd4d6f https://hg.mozilla.org/mozilla-central/rev/44927f617616 https://hg.mozilla.org/mozilla-central/rev/03a8e48f4134 https://hg.mozilla.org/mozilla-central/rev/c7462889f82f https://hg.mozilla.org/mozilla-central/rev/62f44f63b6a5 https://hg.mozilla.org/mozilla-central/rev/98cece19d4fe https://hg.mozilla.org/mozilla-central/rev/1fe6ff5de12e https://hg.mozilla.org/mozilla-central/rev/11b18fa3141b https://hg.mozilla.org/mozilla-central/rev/d1433100e89d https://hg.mozilla.org/mozilla-central/rev/15bc0858e2be https://hg.mozilla.org/mozilla-central/rev/3b2ed2e93f49 https://hg.mozilla.org/mozilla-central/rev/8ee029e39ce4 https://hg.mozilla.org/mozilla-central/rev/07ee1bf0f982 https://hg.mozilla.org/mozilla-central/rev/2d10765f4a82 https://hg.mozilla.org/mozilla-central/rev/73e94ff36e19 https://hg.mozilla.org/mozilla-central/rev/b23a779852ae https://hg.mozilla.org/mozilla-central/rev/bea2adc0a077 https://hg.mozilla.org/mozilla-central/rev/84481760a4de https://hg.mozilla.org/mozilla-central/rev/cf230e06ace0 https://hg.mozilla.org/mozilla-central/rev/2c12696522b7 https://hg.mozilla.org/mozilla-central/rev/1edd02746cd2 https://hg.mozilla.org/mozilla-central/rev/057cca533e0b https://hg.mozilla.org/mozilla-central/rev/d5aab92845f1 https://hg.mozilla.org/mozilla-central/rev/ced4d3f1a189 https://hg.mozilla.org/mozilla-central/rev/aa8932f93615 https://hg.mozilla.org/mozilla-central/rev/8adcb7e36c7c https://hg.mozilla.org/mozilla-central/rev/a851d6125035 https://hg.mozilla.org/mozilla-central/rev/8679a837bb3d https://hg.mozilla.org/mozilla-central/rev/bc557cdb8c1b https://hg.mozilla.org/mozilla-central/rev/5ec0906f885c https://hg.mozilla.org/mozilla-central/rev/30b267369242 https://hg.mozilla.org/mozilla-central/rev/a082d7f6cfa2 https://hg.mozilla.org/mozilla-central/rev/ef7cb62b5b23 https://hg.mozilla.org/mozilla-central/rev/f10665af6c50
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•