H.264 packetization mode 0 (try 3)

webrtc/media/base/codec.cc

Index: webrtc/media/base/codec.cc
diff --git a/webrtc/media/base/codec.cc b/webrtc/media/base/codec.cc
index 835cf77b3e77a87e3e31e7df747e09300cbd363b..76441a2c5c9ffb29543ea0250e8802fc3ed0ce15 100644
--- a/webrtc/media/base/codec.cc
+++ b/webrtc/media/base/codec.cc
@@ -82,7 +82,9 @@ bool FeedbackParams::HasDuplicateEntries() const {
 }
 
 Codec::Codec(int id, const std::string& name, int clockrate)
-    : id(id), name(name), clockrate(clockrate) {}
+    : id(id), name(name), clockrate(clockrate) {
+  SetDefaultParameters();
+}
 
 Codec::Codec() : id(0), clockrate(0) {}
 
@@ -107,6 +109,16 @@ bool Codec::Matches(const Codec& codec) const {
              : (_stricmp(name.c_str(), codec.name.c_str()) == 0);
 }
 
+void Codec::SetDefaultParameters() {
+  if (_stricmp(kH264CodecName, name.c_str()) == 0) {
+    // TODO(magjed): Move setting these parameters into webrtc::H264Encoder
+    // instead.
+    SetParam(kH264FmtpProfileLevelId, kH264ProfileLevelConstrainedBaseline);
+    SetParam(kH264FmtpLevelAsymmetryAllowed, "1");
+    SetParam(kH264FmtpPacketizationMode, "1");
+  }

[sprang_webrtc, 2016/12/01 16:58:56]

Why was this moved here?

[hta-webrtc, 2016/12/01 18:26:27]

Because it was the simplest way to set a default value for packetization mode
that would apply everywhere the cricket::Codec object was created by its
constructor (which is most places).

It did not cover all cases, unfortunately (see the comment next to the value
assignment for the packetization mode enum).

[magjed_webrtc, 2016/12/02 12:34:25]

This is a step in the wrong direction. Why can't you track down all places that
needs this and add it there? I couldn't find the comment you mention.

[hta-webrtc, 2016/12/02 12:48:47]

Why is it a step in the wrong direction?

Previous to this CL, all cases of creating an H.264 codec were implicitly
packetization mode 1.
Subsequent to this CL, we can have two states:

- All codecs with no params are implicitly packetization mode 1. This means that
we have a different meaning of "no parameter" than SDP assigns to it, and would
have to handle that in SDP generation (emitting a parameter when there's none in
the codec struct, setting an explicit parameter with mode 0 when there's none in
the SDP)

- Unless the user specifically removes it, all H.264 codecs have a
"packetization-mode=1" parameter. This is the same as SDP, and SHOULD produce
the same effect as alternative 1 (IF all instances that create a codec use the
constructor).
That's the approach implemented by this CL.

The comment refers to
https://codereview.webrtc.org/2528343002/patch/430001/440005 - it seems that
there's *something* that creates H.264 codecs without calling the constructor.
It's likely in Java, and it's likely only running on Android.

+}
+
 bool Codec::GetParam(const std::string& name, std::string* out) const {
   CodecParameterMap::const_iterator iter = params.find(name);
   if (iter == params.end())