From cb99ccd244dbd5194813e47a62e5521f39e1b36e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Tue, 5 Jul 2022 15:44:48 +0200 Subject: [PATCH] Update/delete old TODOs Bug: webrtc:10198 Change-Id: I0341e068d792bc0b143db86e675988f4cd07ff2e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/267822 Reviewed-by: Harald Alvestrand Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/main@{#37454} --- call/rtp_stream_receiver_controller.h | 8 +++----- call/rtp_stream_receiver_controller_interface.h | 11 +++++------ call/rtp_transport_controller_send.h | 3 --- modules/audio_device/include/fake_audio_device.h | 5 +++-- modules/rtp_rtcp/source/receive_statistics_impl.cc | 1 - modules/rtp_rtcp/source/rtp_rtcp_impl.cc | 3 --- modules/video_capture/video_capture_impl.cc | 2 -- modules/video_coding/codecs/h264/h264_encoder_impl.cc | 1 - .../multiplex/multiplex_encoded_image_packer.cc | 2 +- modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc | 2 -- modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc | 2 -- modules/video_coding/packet.cc | 1 - modules/video_coding/session_info.h | 1 - modules/video_coding/utility/quality_scaler.cc | 1 - video/adaptation/overuse_frame_detector.cc | 2 +- 15 files changed, 13 insertions(+), 32 deletions(-) diff --git a/call/rtp_stream_receiver_controller.h b/call/rtp_stream_receiver_controller.h index 284c9fa12f..38b2f11d9e 100644 --- a/call/rtp_stream_receiver_controller.h +++ b/call/rtp_stream_receiver_controller.h @@ -22,10 +22,8 @@ class RtpPacketReceived; // This class represents the RTP receive parsing and demuxing, for a // single RTP session. -// TODO(nisse): Add RTCP processing, we should aim to terminate RTCP -// and not leave any RTCP processing to individual receive streams. -// TODO(nisse): Extract per-packet processing, including parsing and -// demuxing, into a separate class. +// TODO(bugs.webrtc.org/7135): Add RTCP processing, we should aim to terminate +// RTCP and not leave any RTCP processing to individual receive streams. class RtpStreamReceiverController : public RtpStreamReceiverControllerInterface { public: @@ -41,7 +39,7 @@ class RtpStreamReceiverController bool AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink) override; size_t RemoveSink(const RtpPacketSinkInterface* sink) override; - // TODO(nisse): Not yet responsible for parsing. + // TODO(bugs.webrtc.org/7135): Not yet responsible for parsing. bool OnRtpPacket(const RtpPacketReceived& packet); private: diff --git a/call/rtp_stream_receiver_controller_interface.h b/call/rtp_stream_receiver_controller_interface.h index a5e5295c31..68a9e7daa9 100644 --- a/call/rtp_stream_receiver_controller_interface.h +++ b/call/rtp_stream_receiver_controller_interface.h @@ -18,12 +18,11 @@ namespace webrtc { // An RtpStreamReceiver is responsible for the rtp-specific but // media-independent state needed for receiving an RTP stream. -// TODO(nisse): Currently, only owns the association between ssrc and -// the stream's RtpPacketSinkInterface. Ownership of corresponding -// objects from modules/rtp_rtcp/ should move to this class (or -// rather, the corresponding implementation class). We should add -// methods for getting rtp receive stats, and for sending RTCP -// messages related to the receive stream. +// TODO(bugs.webrtc.org/7135): Currently, only owns the association between ssrc +// and the stream's RtpPacketSinkInterface. Ownership of corresponding objects +// from modules/rtp_rtcp/ should move to this class (or rather, the +// corresponding implementation class). We should add methods for getting rtp +// receive stats, and for sending RTCP messages related to the receive stream. class RtpStreamReceiverInterface { public: virtual ~RtpStreamReceiverInterface() {} diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index abe13eef36..37a5c1e1bb 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -41,9 +41,6 @@ class Clock; class FrameEncryptorInterface; class RtcEventLog; -// TODO(nisse): When we get the underlying transports here, we should -// have one object implementing RtpTransportControllerSendInterface -// per transport, sharing the same congestion controller. class RtpTransportControllerSend final : public RtpTransportControllerSendInterface, public RtcpBandwidthObserver, diff --git a/modules/audio_device/include/fake_audio_device.h b/modules/audio_device/include/fake_audio_device.h index 9949627a73..2322ce0263 100644 --- a/modules/audio_device/include/fake_audio_device.h +++ b/modules/audio_device/include/fake_audio_device.h @@ -19,8 +19,9 @@ namespace webrtc { class FakeAudioDeviceModule : public webrtc_impl::AudioDeviceModuleDefault { public: - // TODO(nisse): Fix all users of this class to managed references using - // scoped_refptr. Current code doesn't always use refcounting for this class. + // TODO(bugs.webrtc.org/12701): Fix all users of this class to managed + // references using scoped_refptr. Current code doesn't always use refcounting + // for this class. void AddRef() const override {} rtc::RefCountReleaseStatus Release() const override { return rtc::RefCountReleaseStatus::kDroppedLastRef; diff --git a/modules/rtp_rtcp/source/receive_statistics_impl.cc b/modules/rtp_rtcp/source/receive_statistics_impl.cc index d2dba66ec5..1e8e399f4d 100644 --- a/modules/rtp_rtcp/source/receive_statistics_impl.cc +++ b/modules/rtp_rtcp/source/receive_statistics_impl.cc @@ -176,7 +176,6 @@ void StreamStatisticianImpl::EnableRetransmitDetection(bool enable) { RtpReceiveStats StreamStatisticianImpl::GetStats() const { RtpReceiveStats stats; stats.packets_lost = cumulative_loss_; - // TODO(nisse): Can we return a float instead? // Note: internal jitter value is in Q4 and needs to be scaled by 1/16. stats.jitter = jitter_q4_ >> 4; if (receive_counters_.last_packet_received_timestamp_ms.has_value()) { diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc index ae4bf5187a..c3f7151c58 100644 --- a/modules/rtp_rtcp/source/rtp_rtcp_impl.cc +++ b/modules/rtp_rtcp/source/rtp_rtcp_impl.cc @@ -308,9 +308,6 @@ RTCPSender::FeedbackState ModuleRtpRtcpImpl::GetFeedbackState() { return state; } -// TODO(nisse): This method shouldn't be called for a receive-only -// stream. Delete rtp_sender_ check as soon as all applications are -// updated. int32_t ModuleRtpRtcpImpl::SetSendingStatus(const bool sending) { if (rtcp_sender_.Sending() != sending) { // Sends RTCP BYE when going from true to false diff --git a/modules/video_capture/video_capture_impl.cc b/modules/video_capture/video_capture_impl.cc index fcee2be594..234c2e131e 100644 --- a/modules/video_capture/video_capture_impl.cc +++ b/modules/video_capture/video_capture_impl.cc @@ -152,8 +152,6 @@ int32_t VideoCaptureImpl::IncomingFrame(uint8_t* videoFrame, // Setting absolute height (in case it was negative). // In Windows, the image starts bottom left, instead of top left. // Setting a negative source height, inverts the image (within LibYuv). - - // TODO(nisse): Use a pool? rtc::scoped_refptr buffer = I420Buffer::Create( target_width, target_height, stride_y, stride_uv, stride_uv); diff --git a/modules/video_coding/codecs/h264/h264_encoder_impl.cc b/modules/video_coding/codecs/h264/h264_encoder_impl.cc index ce976ee02d..f4bda6526f 100644 --- a/modules/video_coding/codecs/h264/h264_encoder_impl.cc +++ b/modules/video_coding/codecs/h264/h264_encoder_impl.cc @@ -110,7 +110,6 @@ static void RtpFragmentize(EncodedImage* encoded_image, SFrameBSInfo* info) { required_capacity += layerInfo.pNalLengthInByte[nal]; } } - // TODO(nisse): Use a cache or buffer pool to avoid allocation? auto buffer = EncodedImageBuffer::Create(required_capacity); encoded_image->SetEncodedData(buffer); diff --git a/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc index 6bc306dda8..0f05d1a89c 100644 --- a/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc +++ b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc @@ -116,7 +116,7 @@ MultiplexImageComponentHeader UnpackFrameHeader(const uint8_t* buffer) { ByteReader::ReadBigEndian(buffer + offset); offset += sizeof(uint32_t); - // TODO(nisse): This makes the wire format depend on the numeric values of the + // This makes the wire format depend on the numeric values of the // VideoCodecType and VideoFrameType enum constants. frame_header.codec_type = static_cast( ByteReader::ReadBigEndian(buffer + offset)); diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index cef9ee970b..d6e95e09af 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -1125,8 +1125,6 @@ int LibvpxVp8Encoder::GetEncodedPartitions(const VideoFrame& input_image, } } - // TODO(nisse): Introduce some buffer cache or buffer pool, to reduce - // allocations and/or copy operations. auto buffer = EncodedImageBuffer::Create(encoded_size); iter = NULL; diff --git a/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc b/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc index 35675f5cbd..609990e641 100644 --- a/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc +++ b/modules/video_coding/codecs/vp9/libvpx_vp9_encoder.cc @@ -1706,8 +1706,6 @@ void LibvpxVp9Encoder::GetEncodedLayerFrame(const vpx_codec_cx_pkt* pkt) { DeliverBufferedFrame(end_of_picture); } - // TODO(nisse): Introduce some buffer cache or buffer pool, to reduce - // allocations and/or copy operations. encoded_image_.SetEncodedData(EncodedImageBuffer::Create( static_cast(pkt->data.frame.buf), pkt->data.frame.sz)); diff --git a/modules/video_coding/packet.cc b/modules/video_coding/packet.cc index 324248ab36..f1bac4a305 100644 --- a/modules/video_coding/packet.cc +++ b/modules/video_coding/packet.cc @@ -58,7 +58,6 @@ VCMPacket::VCMPacket(const uint8_t* ptr, completeNALU = kNaluIncomplete; } - // TODO(nisse): Delete? // Playout decisions are made entirely based on first packet in a frame. if (!is_first_packet_in_frame()) { video_header.playout_delay = {-1, -1}; diff --git a/modules/video_coding/session_info.h b/modules/video_coding/session_info.h index 846352a8ae..6079dbbb72 100644 --- a/modules/video_coding/session_info.h +++ b/modules/video_coding/session_info.h @@ -49,7 +49,6 @@ class VCMSessionInfo { // Returns the number of bytes deleted from the session. size_t MakeDecodable(); - // TODO(nisse): Used by tests only. size_t SessionLength() const; int NumPackets() const; bool HaveFirstPacket() const; diff --git a/modules/video_coding/utility/quality_scaler.cc b/modules/video_coding/utility/quality_scaler.cc index 53b770ba15..6f66b4d1d8 100644 --- a/modules/video_coding/utility/quality_scaler.cc +++ b/modules/video_coding/utility/quality_scaler.cc @@ -31,7 +31,6 @@ namespace webrtc { namespace { -// TODO(nisse): Delete, delegate to encoders. // Threshold constant used until first downscale (to permit fast rampup). static const int kMeasureMs = 2000; static const float kSamplePeriodScaleFactor = 2.5; diff --git a/video/adaptation/overuse_frame_detector.cc b/video/adaptation/overuse_frame_detector.cc index 38cf4a23c1..9836a466b5 100644 --- a/video/adaptation/overuse_frame_detector.cc +++ b/video/adaptation/overuse_frame_detector.cc @@ -526,7 +526,7 @@ OveruseFrameDetector::OveruseFrameDetector( : options_(field_trials), metrics_observer_(metrics_observer), num_process_times_(0), - // TODO(nisse): Use absl::optional + // TODO(bugs.webrtc.org/9078): Use absl::optional last_capture_time_us_(-1), num_pixels_(0), max_framerate_(kDefaultFrameRate),