Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(52)

Issue 21037: Almost complete implementation of the Chrome video renderer. Still needs to ... (Closed)

Created:
11 years, 10 months ago by ralphl
Modified:
9 years, 7 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Almost complete implementation of the Chrome video renderer. Still needs to implement color space conversion for final bitblt. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=9575

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 48

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 19

Patch Set 5 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+435 lines, -29 lines) Patch
M chrome/renderer/media/video_renderer_impl.h View 1 2 3 4 2 chunks +136 lines, -14 lines 0 comments Download
M chrome/renderer/media/video_renderer_impl.cc View 1 2 3 4 1 chunk +285 lines, -13 lines 0 comments Download
M media/base/pipeline.h View 1 chunk +14 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
ralphl
Here's the (almost) complete video renderer. This change relies on the previous change that I ...
11 years, 10 months ago (2009-02-04 07:53:46 UTC) #1
cpu_(ooo_6.6-7.5)
I haven't got to the meat of the CL and I have to go soon ...
11 years, 10 months ago (2009-02-05 02:10:55 UTC) #2
scherkus (not reviewing)
few comments http://codereview.chromium.org/21037/diff/1001/9 File chrome/renderer/media/video_renderer_impl.cc (right): http://codereview.chromium.org/21037/diff/1001/9#newcode8 Line 8: #include "media/base/buffers.h" alphabetize includes http://codereview.chromium.org/21037/diff/1001/9#newcode22 Line ...
11 years, 10 months ago (2009-02-06 00:59:40 UTC) #3
cpu_(ooo_6.6-7.5)
http://codereview.chromium.org/21037/diff/1001/9 File chrome/renderer/media/video_renderer_impl.cc (right): http://codereview.chromium.org/21037/diff/1001/9#newcode18 Line 18: base::TimeDelta::FromMicroseconds(kNoCurrentFrame)) { too much indent http://codereview.chromium.org/21037/diff/1001/9#newcode77 Line 77: ...
11 years, 10 months ago (2009-02-06 21:33:30 UTC) #4
ralphl
I've addressed all of the comments by Carlos and Andrew. I cleaned up the Paint() ...
11 years, 10 months ago (2009-02-09 00:25:53 UTC) #5
scherkus (not reviewing)
Some additional comments. http://codereview.chromium.org/21037/diff/15/16 File chrome/renderer/media/video_renderer_impl.cc (right): http://codereview.chromium.org/21037/diff/15/16#newcode10 Line 10: using media::MediaFormat; not sure if ...
11 years, 10 months ago (2009-02-10 23:29:08 UTC) #6
ralphl
Incorporated Andrew's comments, and documented the various things the |lock_| is protecting. http://codereview.chromium.org/21037/diff/15/16 File chrome/renderer/media/video_renderer_impl.cc ...
11 years, 10 months ago (2009-02-11 01:14:28 UTC) #7
scherkus (not reviewing)
11 years, 10 months ago (2009-02-11 02:20:12 UTC) #8
LGTM

any one else have reviews?

Powered by Google App Engine
This is Rietveld 408576698