Fix packetization logic to leave space for extensions in the last packet

webrtc/modules/rtp_rtcp/source/rtp_format.h

Index: webrtc/modules/rtp_rtcp/source/rtp_format.h
diff --git a/webrtc/modules/rtp_rtcp/source/rtp_format.h b/webrtc/modules/rtp_rtcp/source/rtp_format.h
index 3b6004b9a13f84e30a5e1ba1611a1c3c68e8696c..6dc933dfe446c414489b63d32fcb0181dd23bbd8 100644
--- a/webrtc/modules/rtp_rtcp/source/rtp_format.h
+++ b/webrtc/modules/rtp_rtcp/source/rtp_format.h
@@ -24,6 +24,7 @@ class RtpPacketizer {
  public:
   static RtpPacketizer* Create(RtpVideoCodecTypes type,
                                size_t max_payload_len,
+                               size_t last_packet_extensions_len,

[danilchap, 2017/05/12 08:34:40]

Can it be simpler to use last_packet_payload_len instead?

[ilnik, 2017/05/12 09:17:57]

No, it considerately simplifies logic to know by what size last packet payload
is smaller than the rest.

[danilchap, 2017/05/12 13:56:10]

I meant the interface, not the implementation (each implementation can easily
convert to format it finds more comfortable)

E.g. why packetizer need to know anything about extensions?

[ilnik, 2017/05/12 13:59:20]

It actually doesn't need to know about extensions. Just the size. I can rename
the parameter somehow. Is the last_packet_reduction_len fine?

[danilchap, 2017/05/12 18:56:15]

Found reason why you want this variable in the interface: when Packetizer make
packets similar size, it should aim at semi-equal total packet size, not payload
size.
On the other side one may argue that it should try to make 'free space'
semi-equal across packets. From that point of view last_packet_payload_len and
last_packet_reduction_len are equally ok values to provide.

But name a bit misleading because value counts only additional extensions.
last_packet_reduction_len is better.

[ilnik, 2017/05/15 09:38:33]

Yes, I agree. 

                                const RTPVideoTypeHeader* rtp_type_header,
                                FrameType frame_type);
 
@@ -35,10 +36,12 @@ class RtpPacketizer {
 
   // Get the next payload with payload header.
   // Write payload and set marker bit of the |packet|.
-  // The parameter |last_packet| is true for the last packet of the frame, false
-  // otherwise (i.e., call the function again to get the next packet).
   // Returns true on success, false otherwise.
-  virtual bool NextPacket(RtpPacketToSend* packet, bool* last_packet) = 0;
+  virtual bool NextPacket(RtpPacketToSend* packet) = 0;
+
+  // Returns total number of packets which would be produced by the packetizer.
+  // Valid only before the first NextPacket() call.

[danilchap, 2017/05/12 08:34:40]

may be it will be logical for SetPayloadData to return this value instead?

[ilnik, 2017/05/12 09:17:57]

Yes, you are right. I will change the interface to that.

+  virtual size_t TotalPackets() = 0;
 
   virtual ProtectionType GetProtectionType() = 0;