Minor cleanup of DecoderStreamTraits.

Minor cleanup of DecoderStreamTraits.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291238

[rileya (GONE FROM CHROMIUM), 2014-08-20 06:10:23]

This moves some now-unnecessary DecoderStreamTraits logic back into
DecoderStream. Namely FinishInitialization is unneeded now that the Decoder
init_cb signatures match.

PS3 is a little iffy (see comments), so it's optional.

https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
media/filters/decoder_stream.cc:318:
statistics.*StreamTraits::media_bytes_decoded = buffer_size;
I like that we can move this logic back in here, rather than duplicating in
DecoderStreamTraits, but this pointer-to-member business isn't exactly pretty...

https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
File media/filters/decoder_stream_traits.h (right):

https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
media/filters/decoder_stream_traits.h:24: typedef uint32
(PipelineStatistics::*PipelineStatisticsBytes);
I'm not sure about this, but by using it we can remove ReportStatistics.

This may be the first time I've found a use for a pointer-to-member in C++...

[scherkus (not reviewing), 2014-08-20 16:47:48]

https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
media/filters/decoder_stream.cc:318:
statistics.*StreamTraits::media_bytes_decoded = buffer_size;
On 2014/08/20 06:10:23, rileya wrote:
> I like that we can move this logic back in here, rather than duplicating in
> DecoderStreamTraits, but this pointer-to-member business isn't exactly
pretty...

I'd rather keep this CL to cleaning up initialization and keep stats as is

PipelineStatistics itself could stand for some cleanup (e.g., break it down into
its specific parts for decoders and renderers as well as see whether we can drop
this whole callback business considering that everything runs on the same
thread)

[rileya (GONE FROM CHROMIUM), 2014-08-20 17:23:35]

On 2014/08/20 16:47:48, scherkus wrote:
>
https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
> File media/filters/decoder_stream.cc (right):
> 
>
https://codereview.chromium.org/491733002/diff/40001/media/filters/decoder_st...
> media/filters/decoder_stream.cc:318:
> statistics.*StreamTraits::media_bytes_decoded = buffer_size;
> On 2014/08/20 06:10:23, rileya wrote:
> > I like that we can move this logic back in here, rather than duplicating in
> > DecoderStreamTraits, but this pointer-to-member business isn't exactly
> pretty...
> 
> I'd rather keep this CL to cleaning up initialization and keep stats as is
> 
> PipelineStatistics itself could stand for some cleanup (e.g., break it down
into
> its specific parts for decoders and renderers as well as see whether we can
drop
> this whole callback business considering that everything runs on the same
> thread)

sgtm, removed stats changes.

[scherkus (not reviewing), 2014-08-20 17:26:54]

lgtm w/ nit

https://codereview.chromium.org/491733002/diff/60001/media/filters/decoder_st...
File media/filters/decoder_stream_traits.cc (right):

https://codereview.chromium.org/491733002/diff/60001/media/filters/decoder_st...
media/filters/decoder_stream_traits.cc:64: DCHECK(decoder);
nit: these types of checks where we immediately dereference a pointer aren't
needed

[rileya (GONE FROM CHROMIUM), 2014-08-20 22:15:39]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-08-20 22:17:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rileya@chromium.org/491733002/80001

[xhwang, 2014-08-20 23:33:55]

lgtm

[commit-bot: I haz the power, 2014-08-21 00:01:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[commit-bot: I haz the power, 2014-08-21 00:05:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-21 00:05:55]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[rileya (GONE FROM CHROMIUM), 2014-08-21 00:44:03]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-08-21 00:45:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rileya@chromium.org/491733002/80001

[commit-bot: I haz the power, 2014-08-21 01:00:16]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[commit-bot: I haz the power, 2014-08-21 01:03:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-21 01:03:20]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)

[rileya (GONE FROM CHROMIUM), 2014-08-21 22:55:45]

The CQ bit was checked by rileya@chromium.org

[commit-bot: I haz the power, 2014-08-21 22:59:27]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/rileya@chromium.org/491733002/80001

[commit-bot: I haz the power, 2014-08-21 23:26:42]

Committed patchset #4 (80001) as 291238