Collapse FFmpegVideoDecodeEngine into FFmpegVideoDecoder and remove VideoDecodeEngine.

Collapse FFmpegVideoDecodeEngine into FFmpegVideoDecoder and remove VideoDecodeEngine.

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

[acolwell GONE FROM CHROMIUM, 2011-12-05 18:36:02]

LGTM w/ minor nits

http://codereview.chromium.org/8772069/diff/1/media/filters/ffmpeg_video_deco...
File media/filters/ffmpeg_video_decoder.cc (right):

http://codereview.chromium.org/8772069/diff/1/media/filters/ffmpeg_video_deco...
media/filters/ffmpeg_video_decoder.cc:92: static const int kMaxDecodeThreads =
16;
Don't we typically put constants at the top of the file?

http://codereview.chromium.org/8772069/diff/1/media/filters/ffmpeg_video_deco...
media/filters/ffmpeg_video_decoder.cc:118: if ((!threads.empty() &&
nit: Consider moving this logic to a GetDecoderThreadCount() helper function so
it reduces the clutter in this function.

[Timur Iskhodzhanov, 2011-12-07 10:37:57]

Please run Valgrind & TSan next time you make changes to classes known to have
suppressed reports:
http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%20Tests%20%2...
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20Mac%20%28...
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28t...
or at least don't commit it late in the evening.

[Timur Iskhodzhanov, 2011-12-07 10:38:54]

The suppressions were updated in
http://src.chromium.org/viewvc/chrome?view=rev&revision=113374
but please re-visit them as I was doing it mostly blindly (w/o knowing the code)

On 2011/12/07 10:37:57, Timur Iskhodzhanov wrote:
> Please run Valgrind & TSan next time you make changes to classes known to have
> suppressed reports:
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%2520Tests%25...
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%2520Mac%252...
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%2520Tests%2520...
> or at least don't commit it late in the evening.

[scherkus (not reviewing), 2011-12-07 17:49:26]

Sorry about that -- I ran valgrind and updated suppressions in
http://crrev.com/113357 but didn't notice the tsan suppressions :(

On 2011/12/07 10:38:54, Timur Iskhodzhanov wrote:
> The suppressions were updated in
> http://src.chromium.org/viewvc/chrome?view=rev&revision=113374
> but please re-visit them as I was doing it mostly blindly (w/o knowing the
code)
> 
> On 2011/12/07 10:37:57, Timur Iskhodzhanov wrote:
> > Please run Valgrind & TSan next time you make changes to classes known to
have
> > suppressed reports:
> >
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Windows%252520Tests%...
> >
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%252520Mac%2...
> >
>
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%252520Tests%25...
> > or at least don't commit it late in the evening.

[scherkus (not reviewing), 2011-12-07 17:49:47]

http://codereview.chromium.org/8772069/diff/1/media/filters/ffmpeg_video_deco...
File media/filters/ffmpeg_video_decoder.cc (right):

http://codereview.chromium.org/8772069/diff/1/media/filters/ffmpeg_video_deco...
media/filters/ffmpeg_video_decoder.cc:92: static const int kMaxDecodeThreads =
16;
On 2011/12/05 18:36:04, acolwell wrote:
> Don't we typically put constants at the top of the file?

We do!

FFVDE had them here but I'll move them up to the top

http://codereview.chromium.org/8772069/diff/1/media/filters/ffmpeg_video_deco...
media/filters/ffmpeg_video_decoder.cc:118: if ((!threads.empty() &&
On 2011/12/05 18:36:04, acolwell wrote:
> nit: Consider moving this logic to a GetDecoderThreadCount() helper function
so
> it reduces the clutter in this function.

Done.