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

webrtc/modules/rtp_rtcp/source/rtp_format.h

 /*
  *  Copyright (c) 2014 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_RTP_RTCP_SOURCE_RTP_FORMAT_H_
 #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_FORMAT_H_
 
 #include <string>
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/modules/include/module_common_types.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_rtcp_defines.h"
 
 namespace webrtc {
 class RtpPacketToSend;
 
 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);
 
   virtual ~RtpPacketizer() {}
 
   virtual void SetPayloadData(const uint8_t* payload_data,
                               size_t payload_size,
                               const RTPFragmentationHeader* fragmentation) = 0;
 
   // 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;
 
   virtual StorageType GetStorageType(uint32_t retransmission_settings) = 0;
 
   virtual std::string ToString() = 0;
 };
 
 // TODO(sprang): Update the depacketizer to return a std::unqie_ptr with a copy
 // of the parsed payload, rather than just a pointer into the incoming buffer.
@@ skipping 12 matching lines @@
 
   virtual ~RtpDepacketizer() {}
 
   // Parses the RTP payload, parsed result will be saved in |parsed_payload|.
   virtual bool Parse(ParsedPayload* parsed_payload,
                      const uint8_t* payload_data,
                      size_t payload_data_length) = 0;
 };
 }  // namespace webrtc
 #endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_FORMAT_H_