Web MIDI Linux: Fix enumeration when OPL3 devices are present, and fix manufacturer extraction bugs

media/midi/midi_manager_alsa.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/midi/midi_manager_alsa.h"
 
 #include <alsa/asoundlib.h>
 #include <stdlib.h>
 #include <algorithm>
 #include <string>
@@ skipping 243 matching lines @@
       device::udev_device_new_from_subsystem_sysname(
           outer->udev_.get(), kSoundClass, sysname.c_str()));
 
   // TODO(agoode): Move this to a new utility class in device/udev_linux?
 
   // Try to get the vendor string. Sometimes it is encoded.
   vendor = device::UdevDecodeString(
       device::UdevDeviceGetPropertyValue(udev_device.get(), kIdVendorEnc));
   // Sometimes it is not encoded.
   if (vendor.empty())
-    UdevDeviceGetPropertyOrSysattr(udev_device.get(), kIdVendor,
-                                   kSysattrVendorName);
+    vendor = UdevDeviceGetPropertyOrSysattr(udev_device.get(), kIdVendor,
+                                            kSysattrVendorName);
   // Also get the vendor string from the hardware database.
   vendor_from_database = device::UdevDeviceGetPropertyValue(
       udev_device.get(), kIdVendorFromDatabase);
 
   // Get the device path.
   path_ = device::UdevDeviceGetPropertyValue(udev_device.get(), kIdPath);
 
   // Get the bus.
   bus_ = device::UdevDeviceGetPropertyValue(udev_device.get(), kIdBus);
 
@@ skipping 61 matching lines @@
     const std::string& udev_id_vendor_from_database,
     const std::string& alsa_name,
     const std::string& alsa_longname) {
   // Let's try to determine the manufacturer. Here is the ordered preference
   // in extraction:
   //  1. Vendor name from the hardware device string, from udev properties
   //     or sysattrs.
   //  2. Vendor name from the udev database (property ID_VENDOR_FROM_DATABASE).
   //  3. Heuristic from ALSA.
 
-  // Is the vendor string not just the vendor hex id?
-  if (udev_id_vendor != udev_id_vendor_id) {
+  // Is the vendor string present and not just the vendor hex id?
+  if (!udev_id_vendor.empty() && (udev_id_vendor != udev_id_vendor_id)) {
     return udev_id_vendor;
   }
 
   // Is there a vendor string in the hardware database?
   if (!udev_id_vendor_from_database.empty()) {
     return udev_id_vendor_from_database;
   }
 
   // Ok, udev gave us nothing useful, or was unavailable. So try a heuristic.
   // We assume that card longname is in the format of
@@ skipping 75 matching lines @@
   uint8 hash[crypto::kSHA256Length];
   crypto::SHA256HashString(JSONValue(), &hash, sizeof(hash));
   return base::HexEncode(&hash, sizeof(hash));
 }
 
 // TODO(agoode): Add a client->card/rawmidi mapping to the kernel to avoid
 //               needing to probe in this way.
 ScopedVector<MidiManagerAlsa::AlsaRawmidi> MidiManagerAlsa::AllAlsaRawmidis() {
   ScopedVector<AlsaRawmidi> devices;
   snd_ctl_card_info_t* card;
-  snd_rawmidi_info_t* midi_out;
-  snd_rawmidi_info_t* midi_in;
+  snd_hwdep_info_t* hwdep;
   snd_ctl_card_info_alloca(&card);
-  snd_rawmidi_info_alloca(&midi_out);
-  snd_rawmidi_info_alloca(&midi_in);
+  snd_hwdep_info_alloca(&hwdep);
   for (int card_index = -1; !snd_card_next(&card_index) && card_index >= 0;) {
     const std::string id = base::StringPrintf("hw:CARD=%i", card_index);
     snd_ctl_t* handle;
     int err = snd_ctl_open(&handle, id.c_str(), 0);
     if (err != 0) {
       VLOG(1) << "snd_ctl_open fails: " << snd_strerror(err);
       continue;
     }
     err = snd_ctl_card_info(handle, card);
     if (err != 0) {
       VLOG(1) << "snd_ctl_card_info fails: " << snd_strerror(err);
       snd_ctl_close(handle);
       continue;
     }
-    // Enumerate any rawmidi devices (not subdevices) and extract AlsaRawmidi.
+    std::string name = snd_ctl_card_info_get_name(card);
+    std::string longname = snd_ctl_card_info_get_longname(card);
+    std::string driver = snd_ctl_card_info_get_driver(card);
+
+    // Count rawmidi devices (not subdevices).
     for (int device = -1;
          !snd_ctl_rawmidi_next_device(handle, &device) && device >= 0;) {
-      bool output;
-      bool input;
-      snd_rawmidi_info_set_device(midi_out, device);
-      snd_rawmidi_info_set_subdevice(midi_out, 0);
-      snd_rawmidi_info_set_stream(midi_out, SND_RAWMIDI_STREAM_OUTPUT);
-      output = snd_ctl_rawmidi_info(handle, midi_out) == 0;
-      snd_rawmidi_info_set_device(midi_in, device);
-      snd_rawmidi_info_set_subdevice(midi_in, 0);
-      snd_rawmidi_info_set_stream(midi_in, SND_RAWMIDI_STREAM_INPUT);
-      input = snd_ctl_rawmidi_info(handle, midi_in) == 0;
-      if (!output && !input)
-        continue;
+      devices.push_back(new AlsaRawmidi(this, name,

[Takashi Toyoshima, 2015/03/23 06:45:38]

this indent style isn't bad, but how about using the same indent style with line
480?

[Adam Goode, 2015/03/24 01:39:34]

Done.

+                                        longname, driver, card_index));
+    }
 
-      // Compute and save ALSA and udev properties.
-      snd_rawmidi_info_t* midi = midi_out ? midi_out : midi_in;
-      devices.push_back(new AlsaRawmidi(this, snd_rawmidi_info_get_name(midi),
-                                        snd_ctl_card_info_get_longname(card),
-                                        snd_ctl_card_info_get_driver(card),
-                                        card_index));
+    // Count any hwdep synths that become MIDI devices.
+    for (int device = -1;
+         !snd_ctl_hwdep_next_device(handle, &device) && device >= 0;) {
+      snd_ctl_hwdep_info(handle, hwdep);
+      snd_hwdep_iface_t iface = snd_hwdep_info_get_iface(hwdep);
+      if (iface == SND_HWDEP_IFACE_OPL2 || iface == SND_HWDEP_IFACE_OPL3 ||

[Takashi Toyoshima, 2015/03/23 06:45:38]

Hum... I'm not sure this is the right check to find a MIDI emulating device. Is
this a OPL series specific heuristics? If so, comment is needed.

I guess these flags are originally set for indicating that the device has an
interface to access internal FM sound chips directly. But I'm not sure which
component of ALSA emulates MIDI device on the top of the FM chip.

[Adam Goode, 2015/03/24 01:39:34]

Yes, explanation is needed. I've put a comment explaining the situation.

To summarize: any kernel driver can create an ALSA client (visible to us). With
modern hardware, only rawmidi devices do this. Kernel drivers create rawmidi
devices and the rawmidi subsystem makes seq clients. But the OPL3 driver is
special, it does not make a rawmidi device but a seq client directly. (I
verified this is the only one to worry about in the kernel code.)

OPL3 is very old (but still possible to get in new hardware, which is how I am
testing it). It is unlikely that new drivers would not use rawmidi and defeat
our heuristic.

Longer term, I need to add support in the kernel to expose a direct link from
card->client so that all these heuristics will be obsolete. Once that is there,
we can assume our old heuristics will work on old kernels and the new robust
code will be used on new. Then we will not need to worry about kernel internals
breaking our code.

If we wanted to forego manufacturer detection and robust device reconnection
(meaning that we won't be able to detect device reconnection as the same device
in as many cases), I can remove all the heuristics and be left with simple code.
Once the kernel is updated with the direct support, I can add simple code to
Chrome to add back manufacturer detection and robust device reconnection.

This would cause a bit of a disadvantage to Chrome OS users though, as the
heuristics will work there just fine, since only USB MIDI devices are possible
with current hardware.

+          iface == SND_HWDEP_IFACE_OPL4)
+        devices.push_back(
+            new AlsaRawmidi(this, name, longname, driver, card_index));
     }
     snd_ctl_close(handle);
   }
 
   return devices.Pass();
 }
 
 void MidiManagerAlsa::EnumeratePorts() {
   ScopedVector<AlsaRawmidi> devices = AllAlsaRawmidis();
 
@@ skipping 232 matching lines @@
       }
     }
   }
 }
 
 MidiManager* MidiManager::Create() {
   return new MidiManagerAlsa();
 }
 
 }  // namespace media