Update old TODO comments

Bug: None
Change-Id: I96850df6cfa19303043108a59ef60d7b686ec747
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267661
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37436}

api/transport/BUILD.gn

@@ -58,7 +58,6 @@ rtc_library("field_trial_based_config") {
   absl_deps = [ "//third_party/abseil-cpp/absl/strings" ]
 }
 
-# TODO(nisse): Rename?
 rtc_source_set("datagram_transport_interface") {
   visibility = [ "*" ]
   sources = [ "data_channel_transport_interface.h" ]

api/video/encoded_image.h

@@ -78,9 +78,8 @@ class RTC_EXPORT EncodedImage {
   EncodedImage& operator=(EncodedImage&&);
   EncodedImage& operator=(const EncodedImage&);
 
-  // TODO(nisse): Change style to timestamp(), set_timestamp(), for consistency
+  // TODO(bugs.webrtc.org/9378): Change style to timestamp(), set_timestamp(),
-  // with the VideoFrame class.
+  // for consistency with the VideoFrame class. Set frame timestamp (90kHz).
-  // Set frame timestamp (90kHz).
   void SetTimestamp(uint32_t timestamp) { timestamp_rtp_ = timestamp; }
 
   // Get frame timestamp (90kHz).

api/video/i420_buffer.h

@@ -65,8 +65,8 @@ class RTC_EXPORT I420Buffer : public I420BufferInterface {
   // quirks in memory checkers
   // (https://bugs.chromium.org/p/libyuv/issues/detail?id=377) and
   // ffmpeg (http://crbug.com/390941).
-  // TODO(nisse): Deprecated. Should be deleted if/when those issues
+  // TODO(https://crbug.com/390941): Deprecated. Should be deleted if/when those
-  // are resolved in a better way. Or in the mean time, use SetBlack.
+  // issues are resolved in a better way. Or in the mean time, use SetBlack.
   void InitializeData();
 
   int width() const override;

api/video/i422_buffer.h

@@ -61,8 +61,8 @@ class RTC_EXPORT I422Buffer : public I422BufferInterface {
   // quirks in memory checkers
   // (https://bugs.chromium.org/p/libyuv/issues/detail?id=377) and
   // ffmpeg (http://crbug.com/390941).
-  // TODO(nisse): Deprecated. Should be deleted if/when those issues
+  // TODO(https://crbug.com/390941): Deprecated. Should be deleted if/when those
-  // are resolved in a better way. Or in the mean time, use SetBlack.
+  // issues are resolved in a better way. Or in the mean time, use SetBlack.
   void InitializeData();
 
   int width() const override;

api/video/i444_buffer.h

@@ -58,8 +58,8 @@ class RTC_EXPORT I444Buffer : public I444BufferInterface {
   // quirks in memory checkers
   // (https://bugs.chromium.org/p/libyuv/issues/detail?id=377) and
   // ffmpeg (http://crbug.com/390941).
-  // TODO(nisse): Deprecated. Should be deleted if/when those issues
+  // TODO(https://crbug.com/390941): Deprecated. Should be deleted if/when those
-  // are resolved in a better way. Or in the mean time, use SetBlack.
+  // issues are resolved in a better way. Or in the mean time, use SetBlack.
   void InitializeData();
 
   int width() const override;

api/video/nv12_buffer.h

@@ -52,8 +52,8 @@ class RTC_EXPORT NV12Buffer : public NV12BufferInterface {
   // quirks in memory checkers
   // (https://bugs.chromium.org/p/libyuv/issues/detail?id=377) and
   // ffmpeg (http://crbug.com/390941).
-  // TODO(nisse): Deprecated. Should be deleted if/when those issues
+  // TODO(https://crbug.com/390941): Deprecated. Should be deleted if/when those
-  // are resolved in a better way. Or in the mean time, use SetBlack.
+  // issues are resolved in a better way. Or in the mean time, use SetBlack.
   void InitializeData();
 
   // Scale the cropped area of `src` to the size of `this` buffer, and

api/video/video_frame.h

@@ -166,19 +166,15 @@ class RTC_EXPORT VideoFrame {
   int64_t timestamp_us() const { return timestamp_us_; }
   void set_timestamp_us(int64_t timestamp_us) { timestamp_us_ = timestamp_us; }
 
-  // TODO(nisse): After the cricket::VideoFrame and webrtc::VideoFrame
-  // merge, timestamps other than timestamp_us will likely be
-  // deprecated.
-
   // Set frame timestamp (90kHz).
   void set_timestamp(uint32_t timestamp) { timestamp_rtp_ = timestamp; }
 
   // Get frame timestamp (90kHz).
   uint32_t timestamp() const { return timestamp_rtp_; }
 
-  // For now, transport_frame_id and rtp timestamp are the same.
+  [[deprecated("Use timestamp()")]] uint32_t transport_frame_id() const {
-  // TODO(nisse): Must be handled differently for QUIC.
+    return timestamp();
-  uint32_t transport_frame_id() const { return timestamp(); }
+  }
 
   // Set capture ntp time in milliseconds.
   void set_ntp_time_ms(int64_t ntp_time_ms) { ntp_time_ms_ = ntp_time_ms; }
@@ -219,7 +215,6 @@ class RTC_EXPORT VideoFrame {
   }
 
   // Get render time in milliseconds.
-  // TODO(nisse): Deprecated. Migrate all users to timestamp_us().
   int64_t render_time_ms() const;
 
   // Return the underlying buffer. Never nullptr for a properly
@@ -229,7 +224,6 @@ class RTC_EXPORT VideoFrame {
   void set_video_frame_buffer(
       const rtc::scoped_refptr<VideoFrameBuffer>& buffer);
 
-  // TODO(nisse): Deprecated.
   // Return true if the frame is stored in a texture.
   bool is_texture() const {
     return video_frame_buffer()->type() == VideoFrameBuffer::Type::kNative;

api/video/video_stream_encoder_interface.h

@@ -74,8 +74,8 @@ class VideoStreamEncoderInterface {
   // or frame rate may be reduced. The VideoStreamEncoder registers itself with
   // `source`, and signals adaptation decisions to the source in the form of
   // VideoSinkWants.
-  // TODO(nisse): When adaptation logic is extracted from this class,
+  // TODO(bugs.webrtc.org/14246): When adaptation logic is extracted from this
-  // it no longer needs to know the source.
+  // class, it no longer needs to know the source.
   virtual void SetSource(
       rtc::VideoSourceInterface<VideoFrame>* source,
       const DegradationPreference& degradation_preference) = 0;

api/video/video_stream_encoder_observer.h

@@ -53,7 +53,6 @@ class VideoStreamEncoderObserver : public CpuOveruseMetricsObserver {
     bool framerate_scaling_enabled;
   };
 
-  // TODO(nisse): Duplicates enum EncodedImageCallback::DropReason.
   enum class DropReason {
     kSource,
     kEncoderQueue,
@@ -66,7 +65,7 @@ class VideoStreamEncoderObserver : public CpuOveruseMetricsObserver {
 
   virtual void OnIncomingFrame(int width, int height) = 0;
 
-  // TODO(nisse): Merge into one callback per encoded frame.
+  // TODO(bugs.webrtc.org/8504): Merge into one callback per encoded frame.
   using CpuOveruseMetricsObserver::OnEncodedFrameTimeMeasured;
   virtual void OnSendEncodedImage(const EncodedImage& encoded_image,
                                   const CodecSpecificInfo* codec_info) = 0;
@@ -105,10 +104,10 @@ class VideoStreamEncoderObserver : public CpuOveruseMetricsObserver {
   // down.
   virtual void OnEncoderInternalScalerUpdate(bool is_scaled) {}
 
-  // TODO(nisse): VideoStreamEncoder wants to query the stats, which makes this
+  // TODO(bugs.webrtc.org/14246): VideoStreamEncoder wants to query the stats,
-  // not a pure observer. GetInputFrameRate is needed for the cpu adaptation, so
+  // which makes this not a pure observer. GetInputFrameRate is needed for the
-  // can be deleted if that responsibility is moved out to a VideoStreamAdaptor
+  // cpu adaptation, so can be deleted if that responsibility is moved out to a
-  // class.
+  // VideoStreamAdaptor class.
   virtual int GetInputFrameRate() const = 0;
 };
 

api/video_codecs/video_encoder.h

@@ -66,6 +66,10 @@ class RTC_EXPORT EncodedImageCallback {
   // kDroppedByMediaOptimizations - dropped by MediaOptimizations (for rate
   // limiting purposes).
   // kDroppedByEncoder - dropped by encoder's internal rate limiter.
+  // TODO(bugs.webrtc.org/10164): Delete this enum? It duplicates the more
+  // general VideoStreamEncoderObserver::DropReason. Also,
+  // kDroppedByMediaOptimizations is not produced by any encoder, but by
+  // VideoStreamEncoder.
   enum class DropReason : uint8_t {
     kDroppedByMediaOptimizations,
     kDroppedByEncoder
@@ -96,11 +100,9 @@ class RTC_EXPORT VideoEncoder {
     struct KOff {};
 
    public:
-    // TODO(nisse): Would be nicer if kOff were a constant ScalingSettings
+    // TODO(bugs.webrtc.org/9078): Since absl::optional should be trivially copy
-    // rather than a magic value. However, absl::optional is not trivially copy
+    // constructible, this magic value can likely be replaced by a constexpr
-    // constructible, and hence a constant ScalingSettings needs a static
+    // ScalingSettings value.
-    // initializer, which is strongly discouraged in Chrome. We can hopefully
-    // fix this when we switch to absl::optional or std::optional.
     static constexpr KOff kOff = {};
 
     ScalingSettings(int low, int high);

api/video_codecs/video_encoder_config.h

@@ -141,7 +141,7 @@ class VideoEncoderConfig {
   ~VideoEncoderConfig();
   std::string ToString() const;
 
-  // TODO(nisse): Consolidate on one of these.
+  // TODO(bugs.webrtc.org/6883): Consolidate on one of these.
   VideoCodecType codec_type;
   SdpVideoFormat video_format;
 

audio/channel_receive.cc

@@ -644,7 +644,8 @@ void ChannelReceive::OnRtpPacket(const RtpPacketReceived& packet) {
   const auto& it = payload_type_frequencies_.find(packet.PayloadType());
   if (it == payload_type_frequencies_.end())
     return;
-  // TODO(nisse): Set payload_type_frequency earlier, when packet is parsed.
+  // TODO(bugs.webrtc.org/7135): Set payload_type_frequency earlier, when packet
+  // is parsed.
   RtpPacketReceived packet_copy(packet);
   packet_copy.set_payload_type_frequency(it->second);
 

audio/channel_send.cc

@@ -62,7 +62,7 @@ class ChannelSend : public ChannelSendInterface,
                     public RtcpPacketTypeCounterObserver {
  public:
   // TODO(nisse): Make OnUplinkPacketLossRate public, and delete friend
-  // declaration.
+  // declaration. Or delete indirection via VoERtcpObserver.
   friend class VoERtcpObserver;
 
   ChannelSend(Clock* clock,

call/call.cc

@@ -400,8 +400,8 @@ class Call final : public webrtc::Call,
       RTC_GUARDED_BY(worker_thread_);
   std::set<VideoReceiveStream2*> video_receive_streams_
       RTC_GUARDED_BY(worker_thread_);
-  // TODO(nisse): Should eventually be injected at creation,
+  // TODO(bugs.webrtc.org/7135, bugs.webrtc.org/9719): Should eventually be
-  // with a single object in the bundled case.
+  // injected at creation, with a single object in the bundled case.
   RtpStreamReceiverController audio_receiver_controller_
       RTC_GUARDED_BY(worker_thread_);
   RtpStreamReceiverController video_receiver_controller_
@@ -1550,12 +1550,6 @@ void Call::NotifyBweOfReceivedPacket(const RtpPacketReceived& packet,
 
   if (!use_send_side_bwe && header.extension.hasTransportSequenceNumber) {
     // Inconsistent configuration of send side BWE. Do nothing.
-    // TODO(nisse): Without this check, we may produce RTCP feedback
-    // packets even when not negotiated. But it would be cleaner to
-    // move the check down to RTCPSender::SendFeedbackPacket, which
-    // would also help the PacketRouter to select an appropriate rtp
-    // module in the case that some, but not all, have RTCP feedback
-    // enabled.
     return;
   }
   // For audio, we only support send side BWE.

call/rtp_config.h

@@ -104,7 +104,7 @@ struct RtpConfig {
   // changing codec without recreating the VideoSendStream. Then these
   // fields must be removed, and association between payload type and codec
   // must move above the per-stream level. Ownership could be with
-  // RtpTransportControllerSend, with a reference from PayloadRouter, where
+  // RtpTransportControllerSend, with a reference from RtpVideoSender, where
   // the latter would be responsible for mapping the codec type of encoded
   // images to the right payload type.
   std::string payload_name;

modules/video_coding/codecs/h264/h264_decoder_impl.cc

@@ -114,8 +114,8 @@ int H264DecoderImpl::AVGetBuffer2(AVCodecContext* context,
 
   // FFmpeg expects the initial allocation to be zero-initialized according to
   // http://crbug.com/390941. Our pool is set up to zero-initialize new buffers.
-  // TODO(nisse): Delete that feature from the video pool, instead add
+  // TODO(https://crbug.com/390941): Delete that feature from the video pool,
-  // an explicit call to InitializeData here.
+  // instead add an explicit call to InitializeData here.
   rtc::scoped_refptr<PlanarYuvBuffer> frame_buffer;
   rtc::scoped_refptr<I444Buffer> i444_buffer;
   rtc::scoped_refptr<I420Buffer> i420_buffer;