Commit 8d8984c0 authored by mark a. foltz's avatar mark a. foltz Committed by Commit Bot

[Mirroring Service] Update TODOs.

This converts TODO(foo@) to TODO(crbug.com/XXX) in the Mirroring Service code.
It also ensures that TODOs reference an open crbug.

No functional change.

Change-Id: I34ea1dfea02d016fba87bf7125a90e08be92a8de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1867092Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarYuri Wiitala <miu@chromium.org>
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707973}
parent ee42fe4b
......@@ -297,7 +297,7 @@ void CastMirroringServiceHost::CreateAudioStream(
media::mojom::AudioInputStreamPtr stream,
media::mojom::AudioInputStreamClientRequest client_request,
media::mojom::ReadOnlyAudioDataPipePtr data_pipe) {
// TODO(xjz): Remove |initially_muted| argument from
// TODO(crbug.com/1015488): Remove |initially_muted| argument from
// mojom::AudioStreamCreatorClient::StreamCreated().
mojo::Remote<mojom::AudioStreamCreatorClient> audio_client(
std::move(client));
......
......@@ -25,8 +25,8 @@
#include "mojo/public/cpp/bindings/remote.h"
#include "ui/gfx/geometry/size.h"
// TODO(https://crbug.com/879012): Remove the build flag. OffscreenTab should
// not only be defined when extension is enabled.
// TODO(crbug.com/879012): Remove the build flag. OffscreenTab should not only
// be defined when extension is enabled.
#if BUILDFLAG(ENABLE_EXTENSIONS)
#include "chrome/browser/media/offscreen_tab.h"
#endif // BUILDFLAG(ENABLE_EXTENSIONS)
......
......@@ -369,7 +369,7 @@ void CastRemotingConnector::StartRemotingIfPermitted() {
remoter_->Start();
}
} else {
// TODO(xjz): Add an extra reason for this failure.
// TODO(crbug.com/1015486): Add an extra reason for this failure.
active_bridge_->OnStartFailed(RemotingStartFailReason::ROUTE_TERMINATED);
active_bridge_->OnSinkGone();
active_bridge_ = nullptr;
......
......@@ -78,8 +78,8 @@ class MediaRouter;
// reference for how CastRemotingConnector and a MediaRemoter interact to
// start/execute/stop remoting sessions.
//
// TODO(xjz): Remove media::mojom::MirrorServiceRemotingSource interface and
// implementation after Mirroring Service is launched.
// TODO(crbug.com/1015486): Remove media::mojom::MirrorServiceRemotingSource
// interface and implementation after Mirroring Service is launched.
class CastRemotingConnector : public base::SupportsUserData::Data,
public media::mojom::MirrorServiceRemotingSource,
public media::mojom::RemotingSource {
......@@ -235,7 +235,7 @@ class CastRemotingConnector : public base::SupportsUserData::Data,
// pointing to the RemotingBridge being used to communicate with the source.
RemotingBridge* active_bridge_;
// TODO(xjz): Remove these after Mirroring Service is launched.
// TODO(crbug.com/1015486): Remove these after Mirroring Service is launched.
mojo::Receiver<media::mojom::MirrorServiceRemotingSource>
deprecated_receiver_{this};
mojo::Remote<media::mojom::MirrorServiceRemoter> deprecated_remoter_;
......
......@@ -128,12 +128,13 @@ class MockRemotingSource : public media::mojom::RemotingSource {
mojo::Receiver<media::mojom::RemotingSource> receiver_{this};
};
// TODO(xjz): Remove the media::mojom::MirrorServiceRemoter implementation after
// Mirroring Service is launched.
// TODO(crbug.com/1015486): Remove the media::mojom::MirrorServiceRemoter
// implementation after Mirroring Service is launched.
class MockMediaRemoter final : public media::mojom::MirrorServiceRemoter,
public media::mojom::Remoter {
public:
// TODO(xjz): Remove this ctor after Mirroring Service is launched.
// TODO(crbug.com/1015486): Remove this ctor after Mirroring Service is
// launched.
explicit MockMediaRemoter(FakeMediaRouter* media_router) {
mojo::PendingRemote<media::mojom::MirrorServiceRemoter> pending_remoter;
deprecated_receiver_.Bind(pending_remoter.InitWithNewPipeAndPassReceiver());
......@@ -219,7 +220,7 @@ class MockMediaRemoter final : public media::mojom::MirrorServiceRemoter,
video_sender_receiver));
private:
// TODO(xjz): Remove these after Mirroring Service is launched.
// TODO(crbug.com/1015486): Remove these after Mirroring Service is launched.
mojo::Receiver<media::mojom::MirrorServiceRemoter> deprecated_receiver_{this};
mojo::Remote<media::mojom::MirrorServiceRemotingSource> deprecated_source_;
......@@ -638,7 +639,8 @@ INSTANTIATE_TEST_SUITE_P(,
EXTERNAL_FAILURE,
USER_DISABLED));
// TODO(xjz): Remove the following tests after Mirroring Service is launched.
// TODO(crbug.com/1015486): Remove the following tests after Mirroring Service
// is launched.
TEST_F(CastRemotingConnectorTest,
DeprecatedNotifiesWhenSinkIsAvailableAndThenGone) {
MockRemotingSource source;
......
......@@ -24,8 +24,6 @@ class CastTransportHostFilterTest : public testing::Test {
->InitializeNoOpWakeLockForTesting();
// 127.0.0.1:7 is the local echo service port, which
// is probably not going to respond, but that's ok.
// TODO(hubbe): Open up an UDP port and make sure
// we can send and receive packets.
receive_endpoint_ = net::IPEndPoint(net::IPAddress::IPv4Localhost(), 7);
}
......
......@@ -30,8 +30,8 @@ namespace {
// Global map for looking-up CastRemotingSender instances by their
// |rtp_stream_id|.
// TODO(xjz): Remove this global look-up map when mirror service
// refactoring is done. http://crbug.com/734672
// TODO(crbug.com/1015477): Remove this global look-up map when mirror service
// refactoring is done.
using CastRemotingSenderMap = std::map<int32_t, mirroring::CastRemotingSender*>;
base::LazyInstance<CastRemotingSenderMap>::Leaky g_sender_map =
LAZY_INSTANCE_INITIALIZER;
......@@ -230,7 +230,7 @@ void CastRemotingSender::ResendForKickstart() {
transport_->ResendFrameForKickstart(ssrc_, last_sent_frame_id_);
}
// TODO(xjz): We may need to count in the frames acknowledged in
// TODO(crbug.com/1015477): We may need to count in the frames acknowledged in
// RtcpCastMessage::received_later_frames for more accurate calculation on
// available bandwidth. Same logic should apply on
// media::cast::FrameSender::GetUnacknowledgedFrameCount().
......@@ -277,7 +277,8 @@ void CastRemotingSender::OnReceivedCastMessage(
} else {
duplicate_ack_counter_ = 0;
}
// TODO(miu): The values "2" and "3" should be derived from configuration.
// TODO(crbug.com/1015477): The values "2" and "3" should be derived from
// configuration.
if (duplicate_ack_counter_ >= 2 && duplicate_ack_counter_ % 3 == 2) {
ResendForKickstart();
}
......@@ -475,7 +476,7 @@ void CastRemotingSender::TrySendFrame() {
if (!frame_event_cb_.is_null()) {
media::cast::FrameEvent remoting_event;
remoting_event.timestamp = remoting_frame.reference_time;
// TODO(xjz): Use a new event type for remoting.
// TODO(crbug.com/1015477): Use a new event type for remoting.
remoting_event.type = media::cast::FRAME_ENCODED;
remoting_event.media_type =
is_audio_ ? media::cast::AUDIO_EVENT : media::cast::VIDEO_EVENT;
......@@ -502,11 +503,11 @@ void CastRemotingSender::TrySendFrame() {
void CastRemotingSender::CancelInFlightData() {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
// TODO(miu): The following code is something we want to do as an
// TODO(crbug.com/647423): The following code is something we want to do as an
// optimization. However, as-is, it's not quite correct. We can only cancel
// frames where no packets have actually hit the network yet. Said another
// way, we can only cancel frames the receiver has definitely not seen any
// part of (including kickstarting!). http://crbug.com/647423
// part of (including kickstarting!).
#if 0
if (latest_acked_frame_id_ < last_sent_frame_id_) {
std::vector<media::cast::FrameId> frames_to_cancel;
......
......@@ -21,6 +21,6 @@ interface MirroringServiceHost {
pending_remote<CastMessageChannel> outbound_channel,
pending_receiver<CastMessageChannel> inbound_channel);
// TODO(xjz): Add interfaces to get the streaming events/stats and mirroring
// logs.
// TODO(crbug.com/1015461): Add interfaces to get the streaming events/stats
// and mirroring logs.
};
......@@ -91,7 +91,8 @@ void MediaRemoter::OnMirroringResumed() {
void MediaRemoter::OnRemotingFailed() {
DCHECK(state_ == STARTING_REMOTING || state_ == REMOTING_STARTED);
if (state_ == STARTING_REMOTING) {
// TODO(xjz): Rename SERVICE_NOT_CONNECTED to INVALID_ANSWER_MESSAGE.
// TODO(crbug.com/1015473): Rename SERVICE_NOT_CONNECTED to
// INVALID_ANSWER_MESSAGE.
remoting_source_->OnStartFailed(
media::mojom::RemotingStartFailReason::SERVICE_NOT_CONNECTED);
}
......
......@@ -28,7 +28,7 @@ constexpr base::TimeDelta kMinPlayoutDelay =
// Maximum end-to-end latency. Currently, this is kMinPlayoutDelay, effectively
// disabling adaptive latency control, because of audio playout regressions
// (b/32876644).
// TODO(https://crbug.com/openscreen/44): Re-enable in port to Open Screen.
// TODO(openscreen/44): Re-enable in port to Open Screen.
constexpr base::TimeDelta kMaxPlayoutDelay =
base::TimeDelta::FromMilliseconds(400);
......
......@@ -20,9 +20,10 @@ namespace mirroring {
// Holds the default settings for a mirroring session. This class provides the
// audio/video configs that this sender supports. And also provides the
// audio/video constraints used for capturing.
// TODO(xjz): Add the function to generate the audio capture contraints.
// TODO(xjz): Add setters to the settings that might be overriden by integration
// tests.
// TODO(crbug.com/1015458): Add the function to generate the audio capture
// contraints.
// TODO(crbug.com/1015458): Add setters to the settings that might be overriden
// by integration tests.
class COMPONENT_EXPORT(MIRRORING_SERVICE) MirrorSettings {
public:
MirrorSettings();
......
......@@ -54,11 +54,11 @@ void RemotingSender::SendFrame(uint32_t frame_size) {
void RemotingSender::CancelInFlightData() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// TODO(miu): The following code is something we want to do as an
// TODO(crbug.com/647423): The following code is something we want to do as an
// optimization. However, as-is, it's not quite correct. We can only cancel
// frames where no packets have actually hit the network yet. Said another
// way, we can only cancel frames the receiver has definitely not seen any
// part of (including kickstarting!). http://crbug.com/647423
// part of (including kickstarting!).
#if 0
if (latest_acked_frame_id_ < last_sent_frame_id_) {
std::vector<media::cast::FrameId> frames_to_cancel;
......
......@@ -48,8 +48,8 @@ class COMPONENT_EXPORT(MIRRORING_SERVICE) RtpStreamClient {
virtual void CreateVideoEncodeAccelerator(
const media::cast::ReceiveVideoEncodeAcceleratorCallback& callback) = 0;
// TODO(xjz): Remove this interface. Instead, create the shared memory in
// external video encoder through mojo::ScopedSharedBufferHandle.
// TODO(crbug.com/1015472): Remove this interface. Instead, create the shared
// memory in external video encoder through mojo::ScopedSharedBufferHandle.
virtual void CreateVideoEncodeMemory(
size_t size,
const media::cast::ReceiveVideoEncodeMemoryCallback& callback) = 0;
......
......@@ -153,10 +153,9 @@ bool IsHardwareVP8EncodingSupported(
bool IsHardwareH264EncodingSupported(
const std::vector<media::VideoEncodeAccelerator::SupportedProfile>&
profiles) {
// TODO(miu): Look into why H.264 hardware encoder on MacOS is broken.
// http://crbug.com/596674
// TODO(emircan): Look into HW encoder initialization issues on Win.
// https://crbug.com/636064
// TODO(crbug.com/1015482): Look into why H.264 hardware encoder on MacOS is
// broken.
// TODO(crbug.com/1015482): Look into HW encoder initialization issues on Win.
#if !defined(OS_MACOSX) && !defined(OS_WIN)
for (const auto& vea_profile : profiles) {
if (vea_profile.profile >= media::H264PROFILE_MIN &&
......@@ -317,7 +316,7 @@ media::mojom::RemotingSinkMetadata ToRemotingSinkMetadata(
// Enable remoting 1080p 30fps or higher resolution/fps content for Chromecast
// Ultra receivers only.
// TODO(xjz): Receiver should report this capability.
// TODO(crbug.com/1015467): Receiver should report this capability.
if (params.receiver_model_name == "Chromecast Ultra") {
sink_metadata.video_capabilities.push_back(
RemotingSinkVideoCapability::SUPPORT_4K);
......@@ -352,8 +351,8 @@ class Session::AudioCapturingCallback final
base::TimeTicks audio_capture_time,
double volume,
bool key_pressed) override {
// TODO(xjz): Don't copy the audio data. Instead, send |audio_bus| directly
// to the encoder.
// TODO(crbug.com/1015467): Don't copy the audio data. Instead, send
// |audio_bus| directly to the encoder.
std::unique_ptr<media::AudioBus> captured_audio =
media::AudioBus::Create(audio_bus->channels(), audio_bus->frames());
audio_bus->CopyTo(captured_audio.get());
......@@ -520,8 +519,8 @@ void Session::OnEncoderStatusChange(OperationalStatus status) {
case OperationalStatus::STATUS_UNINITIALIZED:
case OperationalStatus::STATUS_CODEC_REINIT_PENDING:
// Not an error.
// TODO(miu): As an optimization, signal the client to pause sending more
// frames until the state becomes STATUS_INITIALIZED again.
// TODO(crbug.com/1015467): As an optimization, signal the client to pause
// sending more frames until the state becomes STATUS_INITIALIZED again.
case OperationalStatus::STATUS_INITIALIZED:
break;
case OperationalStatus::STATUS_INVALID_CONFIGURATION:
......@@ -707,9 +706,9 @@ void Session::OnAnswer(const std::vector<FrameSenderConfig>& audio_configs,
audio_stream_ = std::make_unique<AudioRtpStream>(
std::move(audio_sender), weak_factory_.GetWeakPtr());
DCHECK(!audio_capturing_callback_);
// TODO(xjz): Elliminate the thread hops. The audio data is thread-hopped
// from the audio thread, and later thread-hopped again to the encoding
// thread.
// TODO(crbug.com/1015467): Eliminate the thread hops. The audio data is
// thread-hopped from the audio thread, and later thread-hopped again to
// the encoding thread.
audio_capturing_callback_ = std::make_unique<AudioCapturingCallback>(
media::BindToCurrentLoop(base::BindRepeating(
&AudioRtpStream::InsertAudio, audio_stream_->AsWeakPtr())),
......@@ -787,7 +786,7 @@ void Session::OnAnswer(const std::vector<FrameSenderConfig>& audio_configs,
}
void Session::OnResponseParsingError(const std::string& error_message) {
// TODO(xjz): Log the |error_message| in the mirroring logs.
// TODO(crbug.com/1015467): Log the |error_message| in the mirroring logs.
}
void Session::CreateAudioStream(
......
......@@ -348,8 +348,8 @@ std::string SessionMonitor::GetEventLogsAndReset(
result.resize(media::cast::kMaxSerializedBytes);
int output_bytes;
// TODO(xjz): media::cast::SerializeEvents() shouldn't require the caller to
// pre-allocate the memory. It should return a string result.
// TODO(crbug.com/1015471): media::cast::SerializeEvents() shouldn't require
// the caller to pre-allocate the memory. It should return a string result.
if (media::cast::SerializeEvents(metadata, frame_events, packet_events,
true /* compress */,
media::cast::kMaxSerializedBytes,
......
......@@ -70,8 +70,8 @@ class COMPONENT_EXPORT(MIRRORING_SERVICE) UdpSocketClient final
// Set by SendPacket() when the sending is not allowed. Once set, SendPacket()
// can only be called again when a previous sending completes successfully.
// TODO(xjz): Change the callback to a base::OnceClosure as well as in the
// cast::PacketTransport SendPacket().
// TODO(crbug.com/1015479): Change the callback to a base::OnceClosure as well
// as in the cast::PacketTransport SendPacket().
base::RepeatingClosure resume_send_callback_;
// Total numbe of bytes written to the data pipe.
......
......@@ -160,7 +160,7 @@ void VideoCaptureClient::OnBufferReady(int32_t buffer_id,
// If the timestamp is not prepared, we use reference time to make a rough
// estimate. e.g. ThreadSafeCaptureOracle::DidCaptureFrame().
// TODO(crbug/618407): Fix upstream capturers to always set timestamp and
// TODO(crbug.com/618407): Fix upstream capturers to always set timestamp and
// reference time.
if (info->timestamp.is_zero())
info->timestamp = reference_time - first_frame_ref_time_;
......@@ -179,7 +179,7 @@ void VideoCaptureClient::OnBufferReady(int32_t buffer_id,
scoped_refptr<media::VideoFrame> frame;
BufferFinishedCallback buffer_finished_callback;
if (buffer_iter->second->is_shared_buffer_handle()) {
// TODO(https://crbug.com/843117): Remove this case after migrating
// TODO(crbug.com/843117): Remove this case after migrating
// media::VideoCaptureDeviceClient to the new shared memory API.
auto mapping_iter = mapped_buffers_.find(buffer_id);
const size_t buffer_size =
......
......@@ -80,9 +80,9 @@ class COMPONENT_EXPORT(MIRRORING_SERVICE) VideoCaptureClient
mojo::Receiver<media::mojom::VideoCaptureObserver> receiver_{this};
// TODO(https://crbug.com/843117): Store the
// base::ReadOnlySharedMemoryRegion instead after migrating the
// media::VideoCaptureDeviceClient to the new shared memory API.
// TODO(crbug.com/843117): Store the base::ReadOnlySharedMemoryRegion instead
// after migrating the media::VideoCaptureDeviceClient to the new shared
// memory API.
using ClientBufferMap =
base::flat_map<int32_t, media::mojom::VideoBufferHandlePtr>;
// Stores the buffer handler on OnBufferCreated(). |buffer_id| is the key.
......@@ -95,7 +95,7 @@ class COMPONENT_EXPORT(MIRRORING_SERVICE) VideoCaptureClient
// The callback to deliver the received frame.
FrameDeliverCallback frame_deliver_callback_;
// TODO(https://crbug.com/843117): Remove the MappingMap after migrating
// TODO(crbug.com/843117): Remove the MappingMap after migrating
// media::VideoCaptureDeviceClient to the new shared memory API.
using MappingAndSize = std::pair<mojo::ScopedSharedBufferMapping, uint32_t>;
using MappingMap = base::flat_map<int32_t, MappingAndSize>;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment