New interface (AudioDecoderFactory), with an implementation

webrtc/modules/audio_coding/codecs/audio_format.h

+/*
+ *  Copyright (c) 2016 The WebRTC project authors. All Rights Reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_MODULES_AUDIO_CODING_CODECS_AUDIO_FORMAT_H_
+#define WEBRTC_MODULES_AUDIO_CODING_CODECS_AUDIO_FORMAT_H_
+
+#include <map>
+#include <ostream>
+#include <string>
+#include <utility>
+
+namespace webrtc {
+
+// SDP specification for a single audio codec.
+// NOTE: This class is still under development and may change without notice.
+struct SdpAudioFormat {

[the sun, 2016/04/27 20:34:14]

How about naming this EncodedAudioFormat instead?

[ossu, 2016/04/28 08:35:53]

The important distinction is that it is not an audio format like how you would
normally think of it, rather a set of parameters that's used in Sdp - actually,
I guess something like RtpAvp (the Audio/Video Profile for RTP that we're
implementing) would be more correct.

From the brief insight into QUIC we got yesterday, I believe this'll need to
change eventually - maybe sooner than we think - but I see no reason to block
this CL over it. It's clearly marked as being in flux.

[kwiberg-webrtc, 2016/04/28 12:35:52]

I'll stick with the current name for now. When we remove the "in flux" comments,
we can revisit the naming question and see if the naming question seems more
straight-forward then.

[the sun, 2016/04/28 13:02:42]

Acknowledged.

+  // TODO(kwiberg): Get rid of the default constructor when rtc::Optional no
+  // longer requires it.
+  SdpAudioFormat();

[the sun, 2016/04/27 20:34:14]

I'd prefer if you put all the "= default" in here.

[kwiberg-webrtc, 2016/04/28 12:35:52]

Can't---the clang style checks complain about complex constructors and
destructor being inline if I do. I agree it would aid readability, but that
check is making Chromium smaller. :-)

[the sun, 2016/04/28 13:02:42]

Oh, how sweet! :)

+  SdpAudioFormat(const SdpAudioFormat&);
+  SdpAudioFormat(SdpAudioFormat&&);
+  SdpAudioFormat(const char* name, int clockrate_hz, int num_channels);

[hlundin-webrtc, 2016/04/27 14:35:56]

I would prefer to rename clockrate_hz to rtp_timestamp_rate_hz.

[the sun, 2016/04/27 20:34:14]

If so, use the same name in the decoder factory unit test.

But: would this ever be used with QUIC?

[ossu, 2016/04/28 08:35:54]

Didn't sound like it, but this class was never designed with QUIC in mind. See
previous comment.

[kwiberg-webrtc, 2016/04/28 12:35:51]

Hmm. What's the relationship between SDP and RTP? Does a field called
rtp_timestamp_rate_hz make sense in a class called SdpAudioFormat?

[ossu, 2016/04/28 14:02:36]

Yeeeaaah, I mean, sort-of. Although the SDP RFC refers to this field as clock
rate, we use SDP to specifically set up communication over RTP (most of these
values are from the rtpmap attribute to a media stream).
If the type is going to be called SdpAudioFormat, then I prefer if the field is
still uses the term clock rate.
Calling the type RtpAudioFormat might be better, in which case changing the name
of clockrate_hz might be beneficial. On the other hand, why mention rtp_ in a
type that's already called RtpAudioFormat?

[kwiberg-webrtc, 2016/04/28 19:35:22]

I think I'll leave the names as they are for now, but it may make sense to call
the class RtpAudioFormat if RTP is where the parameters come from.

+  ~SdpAudioFormat();
+
+  SdpAudioFormat& operator=(const SdpAudioFormat&);
+  SdpAudioFormat& operator=(SdpAudioFormat&&);
+
+  using Parameters = std::map<std::string, std::string>;
+  std::string name;
+  int clockrate_hz;
+  int num_channels;
+  Parameters parameters;
+  // Parameters feedback_parameters; ??
+};
+
+void swap(SdpAudioFormat& a, SdpAudioFormat& b);
+std::ostream& operator<<(std::ostream& os, const SdpAudioFormat& saf);
+
+}  // namespace webrtc
+
+#endif  // WEBRTC_MODULES_AUDIO_CODING_CODECS_AUDIO_FORMAT_H_