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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1126983007: MidiManagerAlsa: Enable manufacturer again, now with hotplug (Closed)

Created:
5 years, 7 months ago by Adam Goode
Modified:
5 years, 7 months ago
Reviewers:
Takashi Toyoshima
CC:
chromium-reviews, feature-media-reviews_chromium.org, toyoshim+midi_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@magical-sound-furnace
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

MidiManagerAlsa: Enable manufacturer again, now with hotplug BUG=476543 Committed: https://crrev.com/51e929027280b71fee09829661c8895863c16888 Cr-Commit-Position: refs/heads/master@{#329198}

Patch Set 1 #

Patch Set 2 : Fix midi_device for user clients #

Total comments: 2

Patch Set 3 : Rename and clarify variables associated with ALSA <-> udev synchronized state #

Unified diffs Side-by-side diffs Delta from patch set Stats (+412 lines, -52 lines) Patch
M media/midi/midi_manager_alsa.h View 1 2 5 chunks +70 lines, -9 lines 0 comments Download
M media/midi/midi_manager_alsa.cc View 1 2 11 chunks +234 lines, -7 lines 0 comments Download
M media/midi/midi_manager_alsa_unittest.cc View 1 2 2 chunks +108 lines, -36 lines 0 comments Download

Messages

Total messages: 11 (3 generated)
Adam Goode
5 years, 7 months ago (2015-05-11 07:55:50 UTC) #2
Takashi Toyoshima
lgtm with one suggestion
5 years, 7 months ago (2015-05-11 10:13:56 UTC) #3
Adam Goode
On 2015/05/11 10:13:56, Takashi Toyoshima (chromium) wrote: > lgtm with one suggestion Sorry, I don't ...
5 years, 7 months ago (2015-05-11 12:27:54 UTC) #4
Takashi Toyoshima
Sorry, I missed to publish my draft. https://codereview.chromium.org/1126983007/diff/20001/media/midi/midi_manager_alsa.cc File media/midi/midi_manager_alsa.cc (right): https://codereview.chromium.org/1126983007/diff/20001/media/midi/midi_manager_alsa.cc#newcode1154 media/midi/midi_manager_alsa.cc:1154: if (alsa_card_midi_count_ ...
5 years, 7 months ago (2015-05-11 12:32:04 UTC) #5
Adam Goode
https://codereview.chromium.org/1126983007/diff/20001/media/midi/midi_manager_alsa.cc File media/midi/midi_manager_alsa.cc (right): https://codereview.chromium.org/1126983007/diff/20001/media/midi/midi_manager_alsa.cc#newcode1154 media/midi/midi_manager_alsa.cc:1154: if (alsa_card_midi_count_ != alsa_seq_state_.kernel_client_count()) On 2015/05/11 12:32:04, Takashi Toyoshima ...
5 years, 7 months ago (2015-05-11 15:17:26 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1126983007/40001
5 years, 7 months ago (2015-05-11 18:17:25 UTC) #9
commit-bot: I haz the power
Committed patchset #3 (id:40001)
5 years, 7 months ago (2015-05-11 18:40:07 UTC) #10
commit-bot: I haz the power
5 years, 7 months ago (2015-05-11 18:41:02 UTC) #11
Message was sent while issue was closed. 
Patchset 3 (id:??) landed as
https://crrev.com/51e929027280b71fee09829661c8895863c16888
Cr-Commit-Position: refs/heads/master@{#329198}

Powered by Google App Engine
This is Rietveld 408576698