Issue 8201018: Rename media::AudioRendererImpl to media::ReferenceAudioRendererImpl.

Issue 8201018: Rename media::AudioRendererImpl to media::ReferenceAudioRendererImpl. (Closed)

Created:
9 years, 2 months ago by Ami GONE FROM CHROMIUM

Modified:
9 years, 2 months ago

Reviewers:
scherkus (not reviewing)

CC:
chromium-reviews, hclam+watch_chromium.org, ddorwin+watch_chromium.org, fischman+watch_chromium.org, acolwell+watch_chromium.org, annacc+watch_chromium.org, pam+watch_chromium.org, ajwong+watch_chromium.org, vrk (LEFT CHROMIUM), scherkus (not reviewing)

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Rename media::AudioRendererImpl to media::ReferenceAudioRendererImpl. This removes the name-collision with ::AudioRendererImpl, which lives in content/renderer/media, and is actually the implementation used in chrome. BUG=none TEST=trybots Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=104587

Patch Set 1 : . #

Total comments: 4

Patch Set 2 : scherkus CR. #

Created: 9 years, 2 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+32 lines, -34 lines)			Patch
A +	media/filters/reference_audio_renderer.h	View	1	3 chunks	+10 lines, -11 lines	0 comments	Download
A +	media/filters/reference_audio_renderer.cc	View	1	6 chunks	+15 lines, -15 lines	0 comments	Download
M	media/media.gyp	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download
M	media/tools/player_wtl/movie.cc	View	1	2 chunks	+3 lines, -3 lines	0 comments	Download
M	media/tools/player_wtl/player_wtl.cc	View		1 chunk	+0 lines, -1 line	0 comments	Download
M	media/tools/player_x11/player_x11.cc	View	1	2 chunks	+2 lines, -2 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

scherkus (not reviewing)

LGTM w/ nits http://codereview.chromium.org/8201018/diff/8/media/filters/reference_audio_renderer_impl.cc File media/filters/reference_audio_renderer_impl.cc (right): http://codereview.chromium.org/8201018/diff/8/media/filters/reference_audio_renderer_impl.cc#newcode17 media/filters/reference_audio_renderer_impl.cc:17: static const size_t kSamplesPerBuffer = 8*1024; ...

9 years, 2 months ago (2011-10-07 22:35:12 UTC) #2

Ami GONE FROM CHROMIUM

9 years, 2 months ago (2011-10-07 23:38:16 UTC) #3

http://codereview.chromium.org/8201018/diff/8/media/filters/reference_audio_r...
File media/filters/reference_audio_renderer_impl.cc (right):

http://codereview.chromium.org/8201018/diff/8/media/filters/reference_audio_r...
media/filters/reference_audio_renderer_impl.cc:17: static const size_t
kSamplesPerBuffer = 8*1024;
On 2011/10/07 22:35:12, scherkus wrote:
> want to fix this spacing?

Done.

http://codereview.chromium.org/8201018/diff/8/media/filters/reference_audio_r...
File media/filters/reference_audio_renderer_impl.h (right):

http://codereview.chromium.org/8201018/diff/8/media/filters/reference_audio_r...
media/filters/reference_audio_renderer_impl.h:26: class MEDIA_EXPORT
ReferenceAudioRendererImpl
On 2011/10/07 22:35:12, scherkus wrote:
> imo ditch the "Impl" suffix -- I find it's redundant and re-enforces the
> somewhat whacky stereotype of overly-suffixed-chrome-class-names
> 
> for example, we have NullAudioRenderer, FileDataSource, FFmpegDemuxer, etc etc
> etc -- there are all implementations but they don't have Impl suffixes

Done.

Expand Messages | Collapse Messages