Break rtp_rtcp_format out of rtp_rtcp, to resolve circular dependencies

webrtc/modules/rtp_rtcp/source/byte_io.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.
  */
 
@@ skipping 18 matching lines @@
 // // Write an unsigned 8 byte integer in little endian format
 // ByteWriter<uint64_t>::WriteLittleEndian(buffer, val);
 //
 // Write an unsigned 40-bit (5 byte) integer in big endian format
 // ByteWriter<uint64_t, 5>::WriteBigEndian(buffer, val);
 //
 // These classes are implemented as recursive templetizations, inteded to make
 // it easy for the compiler to completely inline the reading/writing.
 
 
+#include <stddef.h>

[eladalon, 2017/09/12 14:45:07]

By the way, for my information, don't we prefer to include webrtc/typedefs.h?

[danilchap, 2017/09/12 15:17:18]

I've reverted this file alltogether,
but generally I think we do not:
typedefs.h defines macros like NO_RETURN/FALLTHROUGH etc.
this file doesn't need those macros so it shouldn't include it instead of c
header that it actually need (for uintX_t types)

Can't find rule that explicitly direct one way or another. So following generic
guidance 'include as little as you actually need'

[eladalon, 2017/09/12 15:25:24]

Acknowledged.

+#include <stdint.h>
+
 #include <limits>
 
-#include "webrtc/typedefs.h"
-
 namespace webrtc {
 
 // According to ISO C standard ISO/IEC 9899, section 6.2.6.2 (2), the three
 // representations of signed integers allowed are two's complement, one's
 // complement and sign/magnitude. We can detect which is used by looking at
 // the two last bits of -1, which will be 11 in two's complement, 10 in one's
 // complement and 01 in sign/magnitude.
 // TODO(sprang): In the unlikely event that we actually need to support a
 // platform that doesn't use two's complement, implement conversion to/from
 // wire format.
@@ skipping 343 matching lines @@
     data[1] = val >> 8;
     data[2] = val >> 16;
     data[3] = val >> 24;
     data[4] = val >> 32;
     data[5] = val >> 40;
     data[6] = val >> 48;
     data[7] = val >> 56;
   }
 };
 
+// Round up to the nearest size that is a multiple of 4.
+inline size_t Word32Align(size_t size) {
+  // Same as increment by 3 and clear last two bits.
+  return (size + 3) & ~0x3;

[eladalon, 2017/09/12 14:45:07]

Is this the safest way to do this?
1. There are some implicit conversions in play, and I am not 100% comfortable
with them. Given that size_t is always unsigned, perhaps we could take the
maximum value of it and subtract three instead? You get the same value us ~0x3,
but without any conversions.
2. Speaking of maximum values, what if (size+3) overflows?

[danilchap, 2017/09/12 15:17:17]

2. Moved inside rtp_header_extension_map where size is obviously way below
maximum value.

1. Changed to safer + and % from treating size_t as bitmask.

+}
+
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_RTP_RTCP_SOURCE_BYTE_IO_H_