Introduce USB MIDI descriptor parser

media/midi/usb_midi_descriptor_parser.cc

Index: media/midi/usb_midi_descriptor_parser.cc
diff --git a/media/midi/usb_midi_descriptor_parser.cc b/media/midi/usb_midi_descriptor_parser.cc
new file mode 100644
index 0000000000000000000000000000000000000000..73c0b01543ae74a4dbb9729a05bdec9d4efb5443
--- /dev/null
+++ b/media/midi/usb_midi_descriptor_parser.cc
@@ -0,0 +1,217 @@
+// Copyright (c) 2013 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/usb_midi_descriptor_parser.h"
+
+#include "base/logging.h"
+
+namespace media {
+
+namespace {
+
+enum DescriptorType {
+  DEVICE = 1,

[Takashi Toyoshima, 2013/12/19 05:54:23]

optional: I prefer to have TYPE_ prefix, e.g. TYPE_DEVICE, and so on. But it's
up to you.

[yhirano, 2013/12/19 07:15:05]

Done.

+  CONFIGURATION = 2,
+  STRING = 3,
+  INTERFACE = 4,
+  ENDPOINT = 5,
+  DEVICE_QUALIFIER = 6,
+  OTHER_SPEED_CONFIGURATION = 7,
+  INTERFACE_POWER = 8,
+
+  CS_INTERFACE = 36,
+  CS_ENDPOINT = 37,
+};
+
+enum DescriptorSubType {
+  MS_DESCRIPTOR_UNDEFINED = 0,
+  MS_HEADER = 1,
+  MIDI_IN_JACK = 2,
+  MIDI_OUT_JACK = 3,
+  ELEMENT = 4,
+};
+
+enum JackType {
+  JACK_TYPE_UNDEFINED = 0,
+  EMBEDDED = 1,
+  EXTERNAL = 2,
+};
+
+const uint8 kAudioInterfaceClass = 1;

[Takashi Toyoshima, 2013/12/19 05:54:23]

Can you add some comments on them for someone who don't know about USB spec.
"These values are defined in USB spec" or something is enough, I think.

[yhirano, 2013/12/19 07:15:05]

Done in L13

+const uint8 kAudioMidiInterfaceSubclass = 3;
+
+}  // namespace
+
+UsbMidiDescriptorParser::UsbMidiDescriptorParser()
+    : is_parsing_usb_midi_interface_(false),
+      current_endpoint_address_(0),
+      current_cable_number_(0) {}
+
+UsbMidiDescriptorParser::~UsbMidiDescriptorParser() {}
+
+bool UsbMidiDescriptorParser::Parse(UsbMidiDevice* device,
+                                    const uint8* data,
+                                    size_t size) {
+  Clear();
+  bool result = ParseInternal(device, data, size);
+  if (result)
+    ClearExceptJacks();
+  else
+    Clear();
+  return result;
+}
+
+bool UsbMidiDescriptorParser::ParseInternal(UsbMidiDevice* device,
+                                            const uint8* data,
+                                            size_t size) {

[Takashi Toyoshima, 2013/12/19 05:54:23]

If this function assumes that jacks_ is empty, it's better to add DCHECK here,
or to move Clear() from Parse() to here.

[yhirano, 2013/12/19 07:15:05]

Done.

+  const uint8* current = data;
+  while (current < &data[size]) {
+    uint8 length = current[0];
+    if (length <= 1) {
+      DVLOG(1) << "Descriptor Type is not accessible.";
+      return false;
+    }
+    if (&current[length] > &data[size]) {
+      DVLOG(1) << "The header size is incorrect.";
+      return false;
+    }
+    uint8 descriptor_type = current[1];

[Takashi Toyoshima, 2013/12/19 05:54:23]

Why don't you use DescriptorType instead of uint8?

[yhirano, 2013/12/19 07:15:05]

Done.

+    switch (descriptor_type) {
+      case INTERFACE:
+        if (!ParseInterface(device, current, length)) return false;

[Takashi Toyoshima, 2013/12/19 05:54:23]

I don't think this style is common in chromium.
Can you insert a line break before "return false"?

FYI;
% git grep 'if (.*) return' | wc -l
110
All of them are placed in cast/* or mp4/*

[yhirano, 2013/12/19 07:15:05]

Done.

+        break;
+      case CS_INTERFACE:
+        if (!ParseCSInterface(device, current, length)) return false;
+        break;
+      case ENDPOINT:
+        if (!ParseEndpoint(device, current, length)) return false;
+        break;
+      case CS_ENDPOINT:
+        if (!ParseCSEndpoint(device, current, length)) return false;
+        break;
+      default:
+        break;
+    }
+    current += length;
+  }
+  return true;
+}
+
+bool UsbMidiDescriptorParser::ParseInterface(UsbMidiDevice* device,
+                                             const uint8* data,
+                                             size_t size) {
+  if (size != 9) {
+    DVLOG(1) << "INTERFACE header size is incorrect.";
+    return false;
+  }
+  incomplete_jacks_.clear();
+
+  uint8 interface_class = data[5];
+  uint8 interface_subclass = data[6];
+
+  is_parsing_usb_midi_interface_ =
+      (interface_class == kAudioInterfaceClass &&
+       interface_subclass == kAudioMidiInterfaceSubclass);
+  return true;
+}
+
+bool UsbMidiDescriptorParser::ParseCSInterface(UsbMidiDevice* device,
+                                               const uint8* data,
+                                               size_t size) {
+  if (!is_parsing_usb_midi_interface_) return true;

[Takashi Toyoshima, 2013/12/19 05:54:23]

style

[yhirano, 2013/12/19 07:15:05]

Done.

+
+  // Descriptor Type and Descriptor Subtype should be accessible.
+  if (size <= 2) {
+    DVLOG(1) << "CS_INTERFACE header size is incorrect.";
+    return false;
+  }
+
+  uint8 subtype = data[2];

[Takashi Toyoshima, 2013/12/19 05:54:23]

DescriptorSubType

[yhirano, 2013/12/19 07:15:05]

Done.

+
+  switch (subtype) {
+    case MIDI_IN_JACK:
+      // FALL_THROUGH
+    case MIDI_OUT_JACK: {
+      if (size < 6) {
+        DVLOG(1) << "CS_INTERFACE (MIDI JACK) header size is incorrect.";
+        return false;
+      }
+      uint8 jack_type = data[3];
+      uint8 id = data[4];
+      if (jack_type == EMBEDDED) {
+        // We can't determine the associated endpoint now.
+        incomplete_jacks_.insert(
+            std::make_pair(id, UsbMidiJack(device, id, 0, 0)));
+      }
+    }
+      break;

[Takashi Toyoshima, 2013/12/19 05:54:23]

break at line 148 should be between line 146 and 147.

[yhirano, 2013/12/19 07:15:05]

Done.

+    default:
+      break;
+  }
+  return true;
+}
+
+bool UsbMidiDescriptorParser::ParseEndpoint(UsbMidiDevice* device,
+                                            const uint8* data,
+                                            size_t size) {
+  if (!is_parsing_usb_midi_interface_) return true;
+
+  if (size < 4) {
+    DVLOG(1) << "ENDPOINT header size is incorrect.";
+    return false;
+  }
+  current_endpoint_address_ = data[2];
+  current_cable_number_ = 0;
+  return true;
+}
+
+bool UsbMidiDescriptorParser::ParseCSEndpoint(UsbMidiDevice* device,
+                                              const uint8* data,
+                                              size_t size) {
+  if (!is_parsing_usb_midi_interface_) return true;
+  // CS_ENDPOINT must be of size 4 + n where n is the number of associated
+  // jacks.
+  if (size < 4) {
+    DVLOG(1) << "CS_ENDPOINT header size is incorrect.";
+    return false;
+  }
+  uint8 num_jacks = data[3];
+  if (size != 4 + num_jacks) {
+    DVLOG(1) << "CS_ENDPOINT header size is incorrect.";
+    return false;
+  }
+
+  for (size_t i = 0; i < num_jacks; ++i) {
+    uint8 jack = data[4 + i];
+    base::hash_map<uint8, UsbMidiJack>::iterator it =
+        incomplete_jacks_.find(jack);
+    if (it == incomplete_jacks_.end()) {
+      DVLOG(1) << "A non-existing MIDI jack is associated.";
+      return false;
+    }
+    if (current_cable_number_ >= 0x10) {
+      DVLOG(1) << "Cable number should range from 0x0 to 0xf.";
+      return false;
+    }

[Takashi Toyoshima, 2013/12/19 05:54:23]

Can we assume CS_ENDPOINT always follows associated CS_INTERFACE and ENDPOINT?
If so, it's better to have comment that spec ensure their descriptor order.

[yhirano, 2013/12/19 07:15:05]

Done here and at L121

+    it->second.cable_number = current_cable_number_++;
+    it->second.endpoint_address = current_endpoint_address_;
+    jacks_.push_back(it->second);
+    incomplete_jacks_.erase(it);
+  }
+  return true;
+}
+
+void UsbMidiDescriptorParser::Clear() {
+  ClearExceptJacks();
+  jacks_.clear();
+}
+
+void UsbMidiDescriptorParser::ClearExceptJacks() {
+  is_parsing_usb_midi_interface_ = false;
+  current_endpoint_address_ = 0;
+  current_cable_number_ = 0;
+  incomplete_jacks_.clear();
+}
+
+}  // namespace media