Replace SequencedTaskChecker in RTPSenderVideo

webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc

 /*
  *  Copyright (c) 2012 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 37 matching lines @@
     : rtp_sender_(rtp_sender),
       clock_(clock),
       video_type_(kRtpVideoGeneric),
       retransmission_settings_(kRetransmitBaseLayer),
       last_rotation_(kVideoRotation_0),
       red_payload_type_(-1),
       ulpfec_payload_type_(-1),
       delta_fec_params_{0, 1, kFecMaskRandom},
       key_fec_params_{0, 1, kFecMaskRandom},
       fec_bitrate_(1000, RateStatistics::kBpsScale),
-      video_bitrate_(1000, RateStatistics::kBpsScale) {
-  encoder_checker_.Detach();
-}
+      video_bitrate_(1000, RateStatistics::kBpsScale) { }
 
 RTPSenderVideo::~RTPSenderVideo() {}
 
 void RTPSenderVideo::SetVideoCodecType(RtpVideoCodecTypes video_type) {
   video_type_ = video_type;
 }
 
 RtpVideoCodecTypes RTPSenderVideo::VideoCodecType() const {
   return video_type_;
 }
@@ skipping 167 matching lines @@
 
 bool RTPSenderVideo::SendVideo(RtpVideoCodecTypes video_type,
                                FrameType frame_type,
                                int8_t payload_type,
                                uint32_t rtp_timestamp,
                                int64_t capture_time_ms,
                                const uint8_t* payload_data,
                                size_t payload_size,
                                const RTPFragmentationHeader* fragmentation,
                                const RTPVideoHeader* video_header) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_checker_);
   if (payload_size == 0)
     return false;
 
   // Create header that will be reused in all packets.
   std::unique_ptr<RtpPacketToSend> rtp_header = rtp_sender_->AllocatePacket();
   rtp_header->SetPayloadType(payload_type);
   rtp_header->SetTimestamp(rtp_timestamp);
   rtp_header->set_capture_time_ms(capture_time_ms);
   // According to
   // http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/
   // ts_126114v120700p.pdf Section 7.4.5:
   // The MTSI client shall add the payload bytes as defined in this clause
   // onto the last RTP packet in each group of packets which make up a key
   // frame (I-frame or IDR frame in H.264 (AVC), or an IRAP picture in H.265
   // (HEVC)). The MTSI client may also add the payload bytes onto the last RTP
   // packet in each group of packets which make up another type of frame
   // (e.g. a P-Frame) only if the current value is different from the previous
   // value sent.
   if (video_header) {
     // Set rotation when key frame or when changed (to follow standard).
     // Or when different from 0 (to follow current receiver implementation).
+    rtc::CritScope cs(&crit_);

[magjed_webrtc, 2016/11/10 10:04:03]

Can you easily merge this crit scope with the other crit scope in this function?
From a quick glance, it looks safe to move that crit scope up here.

[danilchap, 2016/11/10 10:14:53]

no, it is not that easy unfortunately:
extensions need to be set for accurate calculate of max_data_payload_length,
that one required for packetizer.

It should be easier to merge this lock with hidden lock in FecPacketOverhead
function though.

[danilchap, 2016/11/10 10:17:32]

on a second though I'm wrong, it is not hard: just remember
retransmission_settings_ inside a lock,

packetizer->GetStorateType can be called outside lock after that.

[kthelgason, 2016/11/10 10:38:26]

Is there a good reason to not just lock during the whole function scope?

[danilchap, 2016/11/10 10:56:44]

It is too dangerous:
SendVideoPacket* functions may call transport function internally, you
definitely want to exclude that.

calls to rtp_sender have own locks, creating lock dependencies i.e. increasing
chance of deadlock.

creating packetizer may be can be under lock, but since it is not hard to avoid
that, better to.


May it is ok to land as it is and solve 'too many locks' problem in a different
CL.

     VideoRotation current_rotation = video_header->rotation;
     if (frame_type == kVideoFrameKey || current_rotation != last_rotation_ ||
         current_rotation != kVideoRotation_0)
       rtp_header->SetExtension<VideoOrientation>(current_rotation);
     last_rotation_ = current_rotation;
   }
 
   size_t packet_capacity = rtp_sender_->MaxPayloadLength() -
                            FecPacketOverhead() -
                            (rtp_sender_->RtxStatus() ? kRtxHeaderSize : 0);
@@ skipping 80 matching lines @@
   rtc::CritScope cs(&crit_);
   return retransmission_settings_;
 }
 
 void RTPSenderVideo::SetSelectiveRetransmissions(uint8_t settings) {
   rtc::CritScope cs(&crit_);
   retransmission_settings_ = settings;
 }
 
 }  // namespace webrtc