Replace VideoDecoder::media_format() with significantly simpler width()/height() methods.

Replace VideoDecoder::media_format() with significantly simpler width()/height() methods.

Clients don't really care about the surface type during initialization, hence its removal.

Also removed a few methods from VideoRendererBase that didn't need to be there.

Technically it's possible to do without width()/height() methods entirely, forcing clients to inspect VideoFrame objects for changes in dimensions and react accordingly, but that'll be a change for a different day.

BUG=28206
TEST=media_unittests, unit_tests, layout_tests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=93278

[scherkus (not reviewing), 2011-07-20 03:14:08]

wjia: RTC changes
fischman: everything else

[Ami GONE FROM CHROMIUM, 2011-07-20 16:30:22]

LGTM mod nits

http://codereview.chromium.org/7461016/diff/1/content/renderer/media/rtc_vide...
File content/renderer/media/rtc_video_decoder.cc (right):

http://codereview.chromium.org/7461016/diff/1/content/renderer/media/rtc_vide...
content/renderer/media/rtc_video_decoder.cc:160: width_ = width;
Before this CL, width/height changes through calls to SetSize() after a call to
Initialize() would not get reflected in the media_format() accessor.  After your
CL these changes are reflected in width()/height().  
Is this an inadvertent bugfix?

http://codereview.chromium.org/7461016/diff/1/media/filters/video_renderer_ba...
File media/filters/video_renderer_base_unittest.cc (right):

http://codereview.chromium.org/7461016/diff/1/media/filters/video_renderer_ba...
media/filters/video_renderer_base_unittest.cc:64: EXPECT_CALL(*decoder_,
width())
FWIW these look like they'll fit in 80-cols.

http://codereview.chromium.org/7461016/diff/1/webkit/glue/media/video_rendere...
File webkit/glue/media/video_renderer_impl.cc (right):

http://codereview.chromium.org/7461016/diff/1/webkit/glue/media/video_rendere...
webkit/glue/media/video_renderer_impl.cc:24: bitmap_.allocPixels();
CHECK it?

[scherkus (not reviewing), 2011-07-20 16:51:52]

http://codereview.chromium.org/7461016/diff/1/content/renderer/media/rtc_vide...
File content/renderer/media/rtc_video_decoder.cc (right):

http://codereview.chromium.org/7461016/diff/1/content/renderer/media/rtc_vide...
content/renderer/media/rtc_video_decoder.cc:160: width_ = width;
On 2011/07/20 16:30:22, Ami Fischman wrote:
> Before this CL, width/height changes through calls to SetSize() after a call
to
> Initialize() would not get reflected in the media_format() accessor.  After
your
> CL these changes are reflected in width()/height().  
> Is this an inadvertent bugfix?

Don't the lines I removed below reflect width/height changes in media_format_?

There's a unit test for this scenario as well.

http://codereview.chromium.org/7461016/diff/1/media/filters/video_renderer_ba...
File media/filters/video_renderer_base_unittest.cc (right):

http://codereview.chromium.org/7461016/diff/1/media/filters/video_renderer_ba...
media/filters/video_renderer_base_unittest.cc:64: EXPECT_CALL(*decoder_,
width())
On 2011/07/20 16:30:22, Ami Fischman wrote:
> FWIW these look like they'll fit in 80-cols.

Done.

http://codereview.chromium.org/7461016/diff/1/webkit/glue/media/video_rendere...
File webkit/glue/media/video_renderer_impl.cc (right):

http://codereview.chromium.org/7461016/diff/1/webkit/glue/media/video_rendere...
webkit/glue/media/video_renderer_impl.cc:24: bitmap_.allocPixels();
On 2011/07/20 16:30:22, Ami Fischman wrote:
> CHECK it?

most chromium code simply assumes the call will work

if allocation fails inside this method the whole process is killed anyway :(

[Ami GONE FROM CHROMIUM, 2011-07-20 16:56:25]

Like I said, LGTM.

http://codereview.chromium.org/7461016/diff/1/content/renderer/media/rtc_vide...
File content/renderer/media/rtc_video_decoder.cc (right):

http://codereview.chromium.org/7461016/diff/1/content/renderer/media/rtc_vide...
content/renderer/media/rtc_video_decoder.cc:160: width_ = width;
On 2011/07/20 16:51:52, scherkus wrote:
> On 2011/07/20 16:30:22, Ami Fischman wrote:
> > Before this CL, width/height changes through calls to SetSize() after a call
> to
> > Initialize() would not get reflected in the media_format() accessor.  After
> your
> > CL these changes are reflected in width()/height().  
> > Is this an inadvertent bugfix?
> 
> Don't the lines I removed below reflect width/height changes in media_format_?
> 
> There's a unit test for this scenario as well.

D'oh.  Yep.  nm.

http://codereview.chromium.org/7461016/diff/1/webkit/glue/media/video_rendere...
File webkit/glue/media/video_renderer_impl.cc (right):

http://codereview.chromium.org/7461016/diff/1/webkit/glue/media/video_rendere...
webkit/glue/media/video_renderer_impl.cc:24: bitmap_.allocPixels();
On 2011/07/20 16:51:52, scherkus wrote:
> On 2011/07/20 16:30:22, Ami Fischman wrote:
> > CHECK it?
> 
> most chromium code simply assumes the call will work
> 
> if allocation fails inside this method the whole process is killed anyway :(

I wish code like this had a compile-time flag that s/bool/void/ for build
configs like chromium's.

[wjia(left Chromium), 2011-07-20 16:58:46]

lgtm. are you going to land this soon? I have a pending patch using
media_format.