Chromium Code Reviews
Help | Chromium Project | Sign in
(282)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by xians1
Modified:
1 year, 6 months ago
Reviewers:
DaleCurtis, scherkus
CC:
chromium-reviews_chromium.org, 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) Lint Patch
M media/audio/linux/alsa_wrapper.cc View 1 1 chunk +7 lines, -66 lines 0 comments 0 errors Download
Commit:

Messages

Total messages: 9
DaleCurtis
lgtm, but this is mostly a rubber stamp. Local testing shows this fixes the fast-forward ...
1 year, 6 months ago #1
xians1
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): ...
1 year, 6 months ago #2
scherkus
OOC have you reached you to pulseaudio community to see if there's a possible regression ...
1 year, 6 months ago #3
xians1
On 2012/10/01 19:52:59, scherkus wrote: > OOC have you reached you to pulseaudio community to ...
1 year, 6 months ago #4
xians1
Given we constantly get audio issue with pulseaudio, it makes me believe that it is ...
1 year, 6 months ago #5
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xians@chromium.org/11019010/2002
1 year, 6 months ago #6
I haz the power (commit-bot)
Change committed as 159680
1 year, 6 months ago #7
scherkus
I agree :) Know anyone who is available? On Oct 2, 2012 3:18 AM, <xians@chromium.org> ...
1 year, 6 months ago #8
xians1
1 year, 6 months ago #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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1275:d14800f88434