[WebMIDI] [Android] Set appropriate port properties.

media/midi/midi_manager_usb.cc

Index: media/midi/midi_manager_usb.cc
diff --git a/media/midi/midi_manager_usb.cc b/media/midi/midi_manager_usb.cc
index cefbc1aac35c7865afb2be47f52701d7e5629f5b..bb07580f465caea2b9155d5f9f3ff1c3dcd89569 100644
--- a/media/midi/midi_manager_usb.cc
+++ b/media/midi/midi_manager_usb.cc
@@ -18,6 +18,16 @@
 
 namespace media {
 
+namespace {
+
+int DecodeBcd(uint8 byte) {
+  DCHECK_LT((byte & 0xf0) >> 4, 0xa);
+  DCHECK_LT(byte & 0x0f, 0xa);
+  return ((byte & 0xf0) >> 4) * 10 + (byte & 0x0f);
+}
+
+}  // namespace
+
 MidiManagerUsb::MidiManagerUsb(scoped_ptr<UsbMidiDevice::Factory> factory)
     : device_factory_(factory.Pass()) {
 }
@@ -124,7 +134,7 @@ void MidiManagerUsb::OnEnumerateDevicesDone(bool result,
 
 bool MidiManagerUsb::AddPorts(UsbMidiDevice* device, int device_id) {
   UsbMidiDescriptorParser parser;
-  std::vector<uint8> descriptor = device->GetDescriptor();
+  std::vector<uint8> descriptor = device->GetDescriptors();
   const uint8* data = descriptor.size() > 0 ? &descriptor[0] : NULL;
   std::vector<UsbMidiJack> jacks;
   bool parse_result = parser.Parse(device,
@@ -134,30 +144,29 @@ bool MidiManagerUsb::AddPorts(UsbMidiDevice* device, int device_id) {
   if (!parse_result)
     return false;
 
+  std::string manufacturer(device->GetManufacturer());
+  std::string product_name(device->GetProductName());
+  uint16 bcd_version(device->GetDeviceVersion());
+  std::string version =

[Takashi Toyoshima, 2015/04/22 05:25:13]

Can you add a comment as the endian is converted in
UsbMidiDescriptorParser::ParseDevice()? Or it may be enough that
GetDeviceVersion() returns a string so that this class does not need to care for
what's BCD.

[yhirano, 2015/04/22 06:11:09]

Done.

+      base::StringPrintf("%d.%02d",
+                         DecodeBcd(bcd_version >> 8),
+                         DecodeBcd(bcd_version & 0xff));
+
   for (size_t j = 0; j < jacks.size(); ++j) {
+    // Port ID must be unique in a MIDI manager. This ID setting is
+    // sufficiently unique although there is no user-friendly meaning.

[Takashi Toyoshima, 2015/04/22 05:25:13]

Can we use SHA256 from crypto/sha2.h to hash this "port-%d-%ld" string?
ALSA port already uses it and it's nice to provide consistent format ID over the
platforms. I'd gradually change other platforms to be consistent.

[yhirano, 2015/04/22 06:11:08]

Should I care about hash collision?

[Takashi Toyoshima, 2015/04/22 06:52:27]

I think we can safely ignore it.
Also, it would be better to concat other factors, e.g. manufacturer,
product_name, and so on.

Also, it's fine to split the hash change to another CL.

[yhirano, 2015/04/22 08:07:42]

Added TODO.

+    std::string id(base::StringPrintf("port-%d-%ld",
+                                      device_id,
+                                      static_cast<long>(j)));
     if (jacks[j].direction() == UsbMidiJack::DIRECTION_OUT) {
       output_streams_.push_back(new UsbMidiOutputStream(jacks[j]));
-      // TODO(yhirano): Set appropriate properties.
-      // TODO(yhiran): Port ID should contain product ID / vendor ID.
-      // Port ID must be unique in a MIDI manager. This (and the below) ID
-      // setting is sufficiently unique although there is no user-friendly
-      // meaning.
-      MidiPortInfo port;
-      port.state = MIDI_PORT_OPENED;
-      port.id = base::StringPrintf("port-%d-%ld",
-                                   device_id,
-                                   static_cast<long>(j));
-      AddOutputPort(port);
+      AddOutputPort(MidiPortInfo(
+          id, manufacturer, product_name, version, MIDI_PORT_OPENED));
     } else {
       DCHECK_EQ(jacks[j].direction(), UsbMidiJack::DIRECTION_IN);
       input_stream_->Add(jacks[j]);
-      // TODO(yhirano): Set appropriate properties.
-      MidiPortInfo port;
-      port.state = MIDI_PORT_OPENED;
-      port.id = base::StringPrintf("port-%d-%ld",
-                                   device_id,
-                                   static_cast<long>(j));
-      AddInputPort(port);
+      AddInputPort(MidiPortInfo(
+          id, manufacturer, product_name, version, MIDI_PORT_OPENED));
     }
   }
   return true;