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

webrtc/modules/rtp_rtcp/source/rtp_format_vp8.h

 /*
  *  Copyright (c) 2011 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.
  */
 
@@ skipping 34 matching lines @@
   kNumModes,
 };
 
 // Packetizer for VP8.
 class RtpPacketizerVp8 : public RtpPacketizer {
  public:
   // Initialize with payload from encoder and fragmentation info.
   // The payload_data must be exactly one encoded VP8 frame.
   RtpPacketizerVp8(const RTPVideoHeaderVP8& hdr_info,
                    size_t max_payload_len,
+                   size_t last_packet_reduction_len,
                    VP8PacketizerMode mode);
 
   // Initialize without fragmentation info. Mode kEqualSize will be used.
   // The payload_data must be exactly one encoded VP8 frame.
-  RtpPacketizerVp8(const RTPVideoHeaderVP8& hdr_info, size_t max_payload_len);
+  RtpPacketizerVp8(const RTPVideoHeaderVP8& hdr_info,
+                   size_t max_payload_len,
+                   size_t last_packet_reduction_len);
 
   virtual ~RtpPacketizerVp8();
 
-  void SetPayloadData(const uint8_t* payload_data,
-                      size_t payload_size,
-                      const RTPFragmentationHeader* fragmentation) override;
+  size_t SetPayloadData(const uint8_t* payload_data,
+                        size_t payload_size,
+                        const RTPFragmentationHeader* fragmentation) override;
 
   // Get the next payload with VP8 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:
   typedef struct {
     size_t payload_start_pos;
     size_t size;
     bool first_fragment;
     size_t first_partition_ix;
   } InfoStruct;
   typedef std::queue<InfoStruct> InfoQueue;
   enum AggregationMode {
     kAggrNone = 0,    // No aggregation.
     kAggrPartitions,  // Aggregate intact partitions.
     kAggrFragments    // Aggregate intact and fragmented partitions.
   };
 
-  static const AggregationMode aggr_modes_[kNumModes];
-  static const bool balance_modes_[kNumModes];
-  static const bool separate_first_modes_[kNumModes];
   static const int kXBit = 0x80;
   static const int kNBit = 0x20;
   static const int kSBit = 0x10;
   static const int kPartIdField = 0x0F;
   static const int kKeyIdxField = 0x1F;
   static const int kIBit = 0x80;
   static const int kLBit = 0x40;
   static const int kTBit = 0x20;
   static const int kKBit = 0x10;
   static const int kYBit = 0x20;
 
-  // Calculate size of next chunk to send. Returns 0 if none can be sent.
-  size_t CalcNextSize(size_t max_payload_len,
-                      size_t remaining_bytes,
-                      bool split_payload) const;
-
   // Calculate all packet sizes and load to packet info queue.
   int GeneratePackets();
 
-  // Calculate all packet sizes using Vp8PartitionAggregator and load to packet
-  // info queue.
-  int GeneratePacketsBalancedAggregates();
+  // Helper function which splits given part of payload (one or more partitions)
+  // to packets with a given capacity. If |last| flag is set then the packet
+  // should be reduced by last_packet_reduction_len_.
+  void SplitPayloadBalanced(size_t payload_offset,

[danilchap, 2017/05/19 12:03:46]

may be name it with GeneratePackets prefix:
function with only input parameter and no return value made me wonder how it
output it's result.
GeneratePackets would imply it fills packets_ member [using QueuePacket].

[ilnik, 2017/05/19 12:57:45]

Done.

+                            size_t payload_len,
+                            size_t capacity,
+                            bool last,

[danilchap, 2017/05/19 12:03:46]

may be last_partition?

[ilnik, 2017/05/19 12:57:45]

Done.

+                            size_t part_ix);

[danilchap, 2017/05/19 12:03:46]

do you mind renaming parameter to part_idx? (or partition_index) to make it
clearer.

[ilnik, 2017/05/19 12:57:45]

Done.

 
-  // Helper function to GeneratePacketsBalancedAggregates(). Find all
-  // continuous sets of partitions smaller than the max payload size (not
-  // max_size), and aggregate them into balanced packets. The result is written
-  // to partition_vec, which is of the same length as the number of partitions.
-  // A value of -1 indicates that the partition is too large and must be split.
-  // Aggregates are numbered 0, 1, 2, etc. For each set of small partitions,
-  // the aggregate numbers restart at 0. Output values min_size and max_size
-  // will hold the smallest and largest resulting aggregates (i.e., not counting
-  // those that must be split).
-  void AggregateSmallPartitions(std::vector<int>* partition_vec,
-                                int* min_size,
-                                int* max_size);
+  // Helper function. Aggregates partitions starting at |part_ix| to packets of

[danilchap, 2017/05/19 12:03:46]

nit: not sure if mentioning that this is Helper function is beneficial: It is
private function which imply it is a helper.
Probably better to start comment directly with description what this function
do: save a bit of time for the reader.

[ilnik, 2017/05/19 12:57:45]

Done.

+  // given capacity. Last packet, if containing last partition of the frame
+  // should be reduced by last_packet_reduction_len_.
+  // Returns the first unaggregated partition index.
+  size_t AggregatePartitions(size_t part_ix, size_t capacity);
 
   // Insert packet into packet queue.
   void QueuePacket(size_t start_pos,
                    size_t packet_size,
                    size_t first_partition_in_packet,
                    bool start_on_new_fragment);
 
   // Write the payload header and copy the payload to the buffer.
   // The info in packet_info determines which part of the payload is written
   // and what to write in the header fields.
@@ skipping 46 matching lines @@
   bool TIDFieldPresent() const;
   bool KeyIdxFieldPresent() const;
   bool TL0PicIdxFieldPresent() const;
   bool PictureIdPresent() const { return (PictureIdLength() > 0); }
 
   const uint8_t* payload_data_;
   size_t payload_size_;
   RTPFragmentationHeader part_info_;
   const size_t vp8_fixed_payload_descriptor_bytes_;  // Length of VP8 payload
                                                      // descriptors' fixed part.
-  const AggregationMode aggr_mode_;
-  const bool balance_;
-  const bool separate_first_;
+  const VP8PacketizerMode mode_;
   const RTPVideoHeaderVP8 hdr_info_;
   size_t num_partitions_;
   const size_t max_payload_len_;
+  const size_t last_packet_reduction_len_;
   InfoQueue packets_;
-  bool packets_calculated_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(RtpPacketizerVp8);
 };
 
 // Depacketizer for VP8.
 class RtpDepacketizerVp8 : public RtpDepacketizer {
  public:
   virtual ~RtpDepacketizerVp8() {}
 
   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_VP8_H_