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

Issue 540003: Revert 35837 - Failed media tests - linux: grab device name before closing it... (Closed)

Created:
10 years, 11 months ago by Robert Sesek
Modified:
9 years, 7 months ago
Reviewers:
Evan Martin
CC:
chromium-reviews_googlegroups.com, scherkus (not reviewing), fbarchard, awong, Alpha Left Google
Visibility:
Public.

Description

Revert 35837 - Failed media tests - linux: grab device name before closing it PcmClose frees the handle regardless of whether there's an error while shutting down, so grab the PcmName result first in case we want to use it in an error message. BUG=20006 Review URL: http://codereview.chromium.org/538005 TBR=evan@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=35848

Patch Set 1 #

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

Messages

Total messages: 5 (0 generated)
Robert Sesek
10 years, 11 months ago (2010-01-08 23:42:46 UTC) #1
Robert Sesek
Broke media tests on Linux and ChromeOS: http://build.chromium.org/buildbot/waterfall/builders/Modules%20Linux/builds/15024/steps/media_unittests/logs/stdio unknown file: Failure Uninteresting mock function call ...
10 years, 11 months ago (2010-01-08 23:44:00 UTC) #2
Evan Martin
Hate computers. Nobody knows how this mock crap works either so now I get to ...
10 years, 11 months ago (2010-01-08 23:50:35 UTC) #3
Evan Martin
(Thank you for reverting quickly and sorry for the trouble.) On Fri, Jan 8, 2010 ...
10 years, 11 months ago (2010-01-08 23:51:31 UTC) #4
awong
10 years, 11 months ago (2010-01-08 23:54:36 UTC) #5
Evan, I can fix it!!!!

On Fri, Jan 8, 2010 at 3:51 PM, Evan Martin <evan@chromium.org> wrote:

> (Thank you for reverting quickly and sorry for the trouble.)
>
> On Fri, Jan 8, 2010 at 3:50 PM, Evan Martin <evan@chromium.org> wrote:
> > Hate computers.  Nobody knows how this mock crap works either so now I
> > get to dig through a new API.
> >
> > On Fri, Jan 8, 2010 at 3:42 PM,  <rsesek@chromium.org> wrote:
> >> Reviewers: Evan Martin,
> >>
> >> Description:
> >> Revert 35837 - Failed media tests - linux: grab device name before
> closing
> >> it
> >>
> >> PcmClose frees the handle regardless of whether there's an error
> >> while shutting down, so grab the PcmName result first in case we
> >> want to use it in an error message.
> >>
> >> BUG=20006
> >>
> >> Review URL: http://codereview.chromium.org/538005
> >>
> >> TBR=evan@chromium.org
> >>
> >> Please review this at http://codereview.chromium.org/540003
> >>
> >> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
> >>
> >> Affected files:
> >>  M     media/audio/linux/alsa_output.cc
> >>
> >>
> >> Index: media/audio/linux/alsa_output.cc
> >> ===================================================================
> >> --- media/audio/linux/alsa_output.cc    (revision 35846)
> >> +++ media/audio/linux/alsa_output.cc    (working copy)
> >> @@ -756,11 +756,10 @@
> >>  }
> >>
> >>  bool AlsaPcmOutputStream::CloseDevice(snd_pcm_t* handle) {
> >> -  std::string name = wrapper_->PcmName(handle);
> >>   int error = wrapper_->PcmClose(handle);
> >>   if (error < 0) {
> >> -    LOG(ERROR) << "Error closing audio device (" << name << "): "
> >> -               << wrapper_->StrError(error);
> >> +    LOG(ERROR) << "Cannot close audio device (" <<
> >> wrapper_->PcmName(handle)
> >> +               << "): " << wrapper_->StrError(error);
> >>     return false;
> >>   }
> >>
> >>
> >>
> >>
> >
>

Powered by Google App Engine
This is Rietveld 408576698