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

Issue 10837032: Fix playback rate w/o audio. Remove unused SetPlaybackRate(). (Closed)

Created:
8 years, 4 months ago by DaleCurtis
Modified:
8 years, 4 months ago
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, feature-media-reviews_chromium.org
Visibility:
Public.

Description

Fix playback rate w/o audio. Remove unused SetPlaybackRate(). NullAudioSink doesn't need to worry about playback rate since AudioRendererAlgorithm already compresses/expands the data as necessary. The issue is easily seen by playing any of the sync clips and timing how fast it takes for each rotation of the "clock" in the video. Setting playback rate to 2 will actually end up being 4x as fast currently. As a bonus, dropping this allows us to remove the now unused AudioRendererSink::SetPlaybackRate() method. BUG=none (?) TEST=Play sync clip, time "clock" rotation at various speeds. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149452

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -25 lines) Patch
M content/renderer/media/render_audiosourceprovider.h View 1 chunk +0 lines, -1 line 0 comments Download
M content/renderer/media/render_audiosourceprovider.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M media/audio/null_audio_sink.h View 2 chunks +0 lines, -2 lines 0 comments Download
M media/audio/null_audio_sink.cc View 4 chunks +2 lines, -9 lines 0 comments Download
M media/base/audio_renderer_sink.h View 1 chunk +0 lines, -4 lines 0 comments Download
M media/filters/audio_renderer_impl.cc View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
DaleCurtis
PTAL. Turns out including playback rate was not correct.
8 years, 4 months ago (2012-08-01 00:38:19 UTC) #1
acolwell GONE FROM CHROMIUM
LGTM
8 years, 4 months ago (2012-08-01 04:38:39 UTC) #2
vrk (LEFT CHROMIUM)
LGTM thanks dale!!
8 years, 4 months ago (2012-08-01 16:53:01 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/10837032/1
8 years, 4 months ago (2012-08-01 17:22:50 UTC) #4
commit-bot: I haz the power
8 years, 4 months ago (2012-08-01 18:38:21 UTC) #5
Change committed as 149452

Powered by Google App Engine
This is Rietveld 408576698