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

webrtc/modules/rtp_rtcp/source/rtp_format_video_generic.h

 /*
  *  Copyright (c) 2013 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_VIDEO_GENERIC_H_
 #define WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_FORMAT_VIDEO_GENERIC_H_
 
 #include <string>
 
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/common_types.h"
 #include "webrtc/modules/rtp_rtcp/source/rtp_format.h"
 #include "webrtc/typedefs.h"
 
 namespace webrtc {
 namespace RtpFormatVideoGeneric {
 static const uint8_t kKeyFrameBit = 0x01;
 static const uint8_t kFirstPacketBit = 0x02;
 }  // namespace RtpFormatVideoGeneric
 
 class RtpPacketizerGeneric : public RtpPacketizer {

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

It is likely good idea to add some tests,
specially since you change logic.

[ilnik, 2017/05/12 14:46:08]

Yes, will add them later. However, this generic packetizer is used in a lot of
video engine tests, so it's already covered quite well.

  public:
   // Initialize with payload from encoder.
   // The payload_data must be exactly one encoded generic frame.
-  RtpPacketizerGeneric(FrameType frametype, size_t max_payload_len);
+  RtpPacketizerGeneric(FrameType frametype,
+                       size_t max_payload_len,
+                       size_t last_packet_extension_len);
 
   virtual ~RtpPacketizerGeneric();
 
-  void SetPayloadData(const uint8_t* payload_data,
-                      size_t payload_size,
-                      const RTPFragmentationHeader* fragmentation) override;
+  // Returns total number of packets to be generated.
+  size_t SetPayloadData(const uint8_t* payload_data,
+                        size_t payload_size,
+                        const RTPFragmentationHeader* fragmentation) override;
 
   // Get the next payload with generic 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.
-  bool NextPacket(RtpPacketToSend* packet, bool* last_packet) override;
+  bool NextPacket(RtpPacketToSend* packet) override;
 
   ProtectionType GetProtectionType() override;
 
   StorageType GetStorageType(uint32_t retransmission_settings) override;
 
   std::string ToString() override;
 
  private:
   const uint8_t* payload_data_;
   size_t payload_size_;
   const size_t max_payload_len_;
+  const size_t last_packet_extension_len_;
   FrameType frame_type_;
-  size_t payload_length_;
+  size_t payload_per_packet_;

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

payload_length_per_packet_?

[ilnik, 2017/05/12 14:46:08]

Done.

   uint8_t generic_header_;
+  size_t num_packets_;

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

Likely this member deserve a comment:
it is not obvious which direction this variable is changed during calls to
NextPacket

[ilnik, 2017/05/12 14:46:08]

Changed to num_packets_left_. Now it's clear that it may be decreased.
Still, added a comment.

+  size_t smaller_packets_;

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

can you add comment what this member is for?
https://google.github.io/styleguide/cppguide.html#Variable_Comments

[ilnik, 2017/05/12 14:46:08]

Done.

 
   RTC_DISALLOW_COPY_AND_ASSIGN(RtpPacketizerGeneric);
 };
 
 // Depacketizer for generic codec.
 class RtpDepacketizerGeneric : public RtpDepacketizer {
  public:
   virtual ~RtpDepacketizerGeneric() {}
 
   bool Parse(ParsedPayload* parsed_payload,
              const uint8_t* payload_data,
              size_t payload_data_length) override;
 };
 }  // namespace webrtc
 #endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_RTP_FORMAT_VIDEO_GENERIC_H_