Added piping for sending audio packets from host to client.

remoting/protocol/content_description.cc

Index: remoting/protocol/content_description.cc
diff --git a/remoting/protocol/content_description.cc b/remoting/protocol/content_description.cc
index c88a36262df02dd7cc7d81d1657216b14bd7bd46..f0ce9587471f354365dfe4ff4a17f879f01e251b 100644
--- a/remoting/protocol/content_description.cc
+++ b/remoting/protocol/content_description.cc
@@ -27,6 +27,7 @@ const char kDescriptionTag[] = "description";
 const char kControlTag[] = "control";
 const char kEventTag[] = "event";
 const char kVideoTag[] = "video";
+const char kAudioTag[] = "audio";
 const char kDeprecatedResolutionTag[] = "initial-resolution";
 
 const char kTransportAttr[] = "transport";
@@ -37,10 +38,12 @@ const char kDeprecatedHeightAttr[] = "height";
 
 const char kStreamTransport[] = "stream";
 const char kDatagramTransport[] = "datagram";
+const char kNoneTransport[] = "none";
 
 const char kVerbatimCodec[] = "verbatim";
 const char kVp8Codec[] = "vp8";
 const char kZipCodec[] = "zip";
+const char kVorbisCodec[] = "vorbis";

[Wez, 2012/06/22 18:29:53]

Have you checked that all calling codesites cope gracefully with unexpected
codecs, e.g. if I specify a vorbis video tag, will the host correctly reject the
config?

[kxing, 2012/06/22 19:12:31]

I think so:

remoting/client/rectangle_update_decoder.cc checks for it correctly
remoting/host/chromoting_host.cc checks for it correctly
remoting/host/simple_host_process.cc checks for it correctly
remoting/protocol/video_reader.cc checks for it correctly

Let me know if I'm missing something.

 
 const char* GetTransportName(ChannelConfig::TransportType type) {
   switch (type) {
@@ -48,6 +51,8 @@ const char* GetTransportName(ChannelConfig::TransportType type) {
       return kStreamTransport;
     case ChannelConfig::TRANSPORT_DATAGRAM:
       return kDatagramTransport;
+    case ChannelConfig::TRANSPORT_NONE:
+      return kNoneTransport;
   }
   NOTREACHED();
   return NULL;
@@ -61,6 +66,8 @@ const char* GetCodecName(ChannelConfig::Codec type) {
       return kVp8Codec;
     case ChannelConfig::CODEC_ZIP:
       return kZipCodec;
+    case ChannelConfig::CODEC_VORBIS:
+      return kVorbisCodec;
     default:
       break;
   }
@@ -97,6 +104,8 @@ bool ParseTransportName(const std::string& value,
     *transport = ChannelConfig::TRANSPORT_STREAM;
   } else if (value == kDatagramTransport) {
     *transport = ChannelConfig::TRANSPORT_DATAGRAM;
+  } else if (value == kNoneTransport) {
+    *transport = ChannelConfig::TRANSPORT_NONE;
   } else {
     return false;
   }
@@ -110,6 +119,8 @@ bool ParseCodecName(const std::string& value, ChannelConfig::Codec* codec) {
     *codec = ChannelConfig::CODEC_VP8;
   } else if (value == kZipCodec) {
     *codec = ChannelConfig::CODEC_ZIP;
+  } else if (value == kVorbisCodec) {
+    *codec = ChannelConfig::CODEC_VORBIS;
   } else {
     return false;
   }
@@ -163,6 +174,7 @@ ContentDescription* ContentDescription::Copy() const {
 //     <control transport="stream" version="1" />
 //     <event transport="datagram" version="1" />
 //     <video transport="stream" codec="vp8" version="1" />
+//     <audio transport="stream" codec="vorbis" version="1" />
 //     <authentication>
 //      Message created by Authenticator implementation.
 //     </authentication>
@@ -189,6 +201,11 @@ XmlElement* ContentDescription::ToXml() const {
     root->AddElement(FormatChannelConfig(*it, kVideoTag));
   }
 
+  for (it = config()->audio_configs().begin();
+       it != config()->audio_configs().end(); ++it) {
+    root->AddElement(FormatChannelConfig(*it, kAudioTag));
+  }
+
   // Older endpoints require an initial-resolution tag, but otherwise ignore it.
   XmlElement* resolution_tag = new XmlElement(
       QName(kChromotingXmlNamespace, kDeprecatedResolutionTag));
@@ -245,6 +262,25 @@ ContentDescription* ContentDescription::ParseXml(
       child = child->NextNamed(video_tag);
     }
 
+    // <audio> tags.
+    QName audio_tag(kChromotingXmlNamespace, kAudioTag);
+    child = element->FirstNamed(audio_tag);
+    if (!child) {
+      // If there's no mention of audio, implicitly assume
+      // TRANSPORT_NONE for the audio_channel.
+      ChannelConfig no_audio(ChannelConfig::TRANSPORT_NONE,
+                             kDefaultStreamVersion,
+                             ChannelConfig::CODEC_UNDEFINED);
+      config->mutable_audio_configs()->push_back(no_audio);
+    }
+    while (child) {
+      ChannelConfig channel_config;
+      if (!ParseChannelConfig(child, false, &channel_config))
+        return NULL;
+      config->mutable_audio_configs()->push_back(channel_config);
+      child = child->NextNamed(audio_tag);
+    }

[Wez, 2012/06/22 18:29:53]

nit: We've got four all-but-identical blocks of code here; can we fold them into
a single block w/ iterator over the known channel names?  Since calling code
can't cope with "none" for anything but the audio-channel, the audio-channel
special-case can be handled by checking the number of audio_configs() after
we've processed the XML, and adding a "none" config if that is zero.

[kxing, 2012/06/22 19:12:31]

This might be a bit tricky, since video tags require a codec (the second
parameter to ParseChannelConfig is true). Do you think doing special case
handling in the middle of the loop is worth it?

[Wez, 2012/06/22 21:14:06]

Good point.  You also want audio tags to have a codec property, I think, but
you're passing false to ParseChannelConfig() for those right now.

You'd need a structure to iterate over that contains the channel name and a
pointer to the mutable configs structure for it, so adding a needs-codec flag to
that would seem reasonable. Alternatively you could just have a static method
that, given a channel name, returns true if that channel needs a codec, false
otherwise.

[kxing, 2012/06/22 21:21:37]

Sure. I'll do that in https://code.google.com/p/chromium/issues/detail?id=134222

+
     scoped_ptr<XmlElement> authenticator_message;
     child = Authenticator::FindAuthenticatorMessage(element);
     if (child)