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

Issue 11019010: replace snd_pcm_hw_params_set_* with snd_pcm_set_params. (Closed)

Created:
8 years, 2 months ago by no longer working on chromium
Modified:
8 years, 2 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org
Visibility:
Public.

Description

It seems that snd_pcm_hw_params_set_* API does not work well any more in the latest ubuntu like 12.04. My tests show that it will freeze the pulseaudio for a while and get into underrun state. And it seems snd_pcm_set_params works better for the same purpose. This patch also fixes the choppy audio issue for webrtc in ubuntu 12.04. BUG=152232, 153505 TEST=use pepper flash (./out/Debug/chrome -ppapi-flash-path=/your/path/libpepflashplayer.so) open link http://www.youtube.com/watch?v=F7pYHN9iC9I&feature=youtu.be in two tabs, audio should be good. uses apprtc.appspot.com to make a loopback call and verify the audio is good on ubuntu 12.04 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=159680

Patch Set 1 #

Total comments: 2

Patch Set 2 : used the passed variable #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -66 lines) Patch
M media/audio/linux/alsa_wrapper.cc View 1 1 chunk +7 lines, -66 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
DaleCurtis
lgtm, but this is mostly a rubber stamp. Local testing shows this fixes the fast-forward ...
8 years, 2 months ago (2012-10-01 18:59:15 UTC) #1
no longer working on chromium
I will update the code and description before land it. thanks. http://codereview.chromium.org/11019010/diff/1/media/audio/linux/alsa_wrapper.cc File media/audio/linux/alsa_wrapper.cc (right): ...
8 years, 2 months ago (2012-10-01 19:51:13 UTC) #2
scherkus (not reviewing)
OOC have you reached you to pulseaudio community to see if there's a possible regression ...
8 years, 2 months ago (2012-10-01 19:52:59 UTC) #3
no longer working on chromium
On 2012/10/01 19:52:59, scherkus wrote: > OOC have you reached you to pulseaudio community to ...
8 years, 2 months ago (2012-10-02 10:09:47 UTC) #4
no longer working on chromium
Given we constantly get audio issue with pulseaudio, it makes me believe that it is ...
8 years, 2 months ago (2012-10-02 10:18:02 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xians@chromium.org/11019010/2002
8 years, 2 months ago (2012-10-02 10:18:16 UTC) #6
commit-bot: I haz the power
Change committed as 159680
8 years, 2 months ago (2012-10-02 12:12:41 UTC) #7
scherkus (not reviewing)
I agree :) Know anyone who is available? On Oct 2, 2012 3:18 AM, <xians@chromium.org> ...
8 years, 2 months ago (2012-10-02 13:18:22 UTC) #8
no longer working on chromium
8 years, 2 months ago (2012-10-03 09:10:24 UTC) #9
On Tue, Oct 2, 2012 at 3:18 PM, Andrew Scherkus <scherkus@chromium.org>wrote:

> I agree :)
>
> Know anyone who is available?
>
>
I have already spent some of my spare time on putting the code together,
and have CLs which work with basic functionalities like recording and
playing sound.
But they only work on my lucid, but not gprecise (recording is choppy
there).

My plan is to start with the input side, since I have more control here. If
it works well, then we can extend it to the output.

But honestly this won't be ready for review within a few weeks, since I
need to finish some UI and media stream work first.

Powered by Google App Engine
This is Rietveld 408576698