0
Commit Graph

49 Commits

Author SHA1 Message Date
Peter Boström
40ccfac42d Migrate to NOTREACHED() in remoting/
NOTREACHED() and NOTREACHED_IN_MIGRATION() are both CHECK-fatal now.
The former is [[noreturn]] so this CL also performs dead-code removal
after the NOTREACHED().

This CL does not attempt to do additional rewrites of any surrounding
code, like:

if (!foo) {
  NOTREACHED();
}

to CHECK(foo);

Those transforms take a non-trivial amount of time (and there are
thousands of instances). Cleanup can be left as an exercise for the
reader.

Bug: 40580068
Low-Coverage-Reason: OTHER Should-be-unreachable code
Change-Id: I8cd87f774e5fec8ec893dcc7286b0979643ea627
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5873742
Reviewed-by: Joe Downing <joedow@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1357911}
2024-09-19 23:56:11 +00:00
Peter Boström
e72e8c8dfb Use NOTREACHED_IN_MIGRATION() in remoting/
This was generated by replacing "  NOTREACHED()" with
"  NOTREACHED_IN_MIGRATION()" and running git cl format.

This prepares for making NOTREACHED() [[noreturn]] alongside
NotReachedIsFatal migration of existing inventory.

Bug: 40580068
Change-Id: I57cfd1b2a4e6c91529ab221cf7a7da622be26913
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5540125
Commit-Queue: Lei Zhang <thestig@chromium.org>
Owners-Override: Lei Zhang <thestig@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Auto-Submit: Peter Boström <pbos@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1300942}
2024-05-14 22:40:32 +00:00
Alison Gale
71bd8f1596 Migrate TODOs referencing old crbug IDs to the new issue tracker IDs
The canonical bug format is TODO(crbug.com/<id>). TODOs of the
following forms will all be migrated to the new format:

- TODO(crbug.com/<old id>)
- TODO(https://crbug.com/<old id>)
- TODO(crbug/<old id>)
- TODO(crbug/monorail/<old id>)
- TODO(<old id>)
- TODO(issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/<old id>)
- TODO(https://issues.chromium.org/u/1/issues/<old id>)
- TODO(bugs.chromium.org/<old id>)

Bug id mapping is sourced from go/chrome-on-buganizer-prod-issues.
See go/crbug-todo-migration for details.

#crbug-todo-migration

Bug: b/321899722
Change-Id: I050f2f811b9f50e8aa29d2474da33746d17d59fb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5494188
Owners-Override: Alison Gale <agale@chromium.org>
Auto-Submit: Alison Gale <agale@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1293300}
2024-04-26 22:38:20 +00:00
Markus Handell
3a245e6091 s/[set_]timestamp/[set_]rtp_timestamp
This CL updates Chrome to avoid the deprecated naming related to
the RTP timestamps in the webrtc::VideoFrame.

Bug: webrtc:13756
Change-Id: I394d23f2a9b547b3fcba7d6d3a6af1a8c386e462
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5372044
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Commit-Queue: Markus Handell <handellm@google.com>
Reviewed-by: Erik Jensen <rkjnsn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1275147}
2024-03-19 20:25:22 +00:00
Joe Downing
2837c38786 Allow client to enable/disable AV1 active_map
The active_map feature provides a way for us to tell the encoder
which regions of a frame have changed so it doesn't need to check
the entire frame for changes. With VP8 this was a neccesity and VP9
also benefits from having this feature enabled however our impl
for AV1 still shows a performance hit when enabling it.

I would like to add a Client toggle to allow for testing and
tuning of the feature to see if we can further improve performance
for AV1 in CRD using an active_map.

Example output:
Creating AV1 encoder - Profile: 0, Speed: 11, ActiveMap: 1
Creating AV1 encoder - Profile: 1, Speed: default, ActiveMap: default

Change-Id: I675b4b6cd7668a69d32f87e6af07c72c8b794b85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5360244
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Auto-Submit: Joe Downing <joedow@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1271130}
2024-03-11 20:01:22 +00:00
Arthur Sonzogni
59ac8227c5 Rename {absl => std}::optional in minor top-level dirs
Automated patch, intended to be effectively a no-op.

This patch gather the changes for every top-level directories with less
than 150 files modified:
 #  directory
--- ---------------
150 remoting
98  gpu
87  chromecast
79  mojo
70  storage
65  fuchsia_web
46  sandbox
44  android_webview
38  google_apis
27  pdf
25  printing
20  headless
13  ipc
11  crypto
10  sql
3   dbus
2   testing
2   skia
2   gin
2   apps
1   rlz
1   codelabs

Context:
https://groups.google.com/a/chromium.org/g/cxx/c/nBD_1LaanTc/m/ghh-ZZhWAwAJ?utm_medium=email&utm_source=footer

As of https://crrev.com/1204351, absl::optional is now a type alias for
std::optional. We should migrate toward it.

Script:
```

function replace {
  echo "Replacing $1 by $2"
  git grep -l "$1" \
		| cut -f1 -d: \
		| grep \
			-e "^codelabs" \
			-e "^rlz" \
			-e "^apps" \
			-e "^gin" \
			-e "^skia" \
			-e "^testing" \
			-e "^dbus" \
			-e "^sql" \
			-e "^crypto" \
			-e "^ipc" \
			-e "^headless" \
			-e "^printing" \
			-e "^pdf" \
			-e "^google_apis" \
			-e "^android_webview" \
			-e "^sandbox" \
			-e "^fuchsia_web" \
			-e "^storage" \
			-e "^mojo" \
			-e "^chromecast" \
			-e "^gpu" \
			-e "^remoting" \
		| sort \
		| uniq \
		| grep \
			-e "\.h" \
			-e "\.cc" \
			-e "\.mm" \
			-e "\.py" \
	  | xargs sed -i "s/$1/$2/g"
}

replace "absl::make_optional" "std::make_optional"
replace "absl::optional" "std::optional"
replace "absl::nullopt" "std::nullopt"
replace "absl::in_place" "std::in_place"
replace "absl::in_place_t" "std::in_place_t"
replace "\"third_party\/abseil-cpp\/absl\/types\/optional.h\"" "<optional>"
git cl format
```

CQ_INCLUDE_TRYBOTS=luci.chrome.try:chromeos-betty-pi-arc-chrome

Bug: chromium:1500249
Change-Id: I0eca8ff96f5712ba746ac8d8da93d03a86d8292c
AX-Relnotes: n/a.
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5009410
Reviewed-by: danakj <danakj@chromium.org>
Owners-Override: danakj <danakj@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1222826}
2023-11-10 09:46:54 +00:00
Danil Chapovalov
d19ebfc489 Rename webrtc::EncodedImage::SetTimestamp to SetRtpTimestamp
To clarify this setter represents capture time as rtp timestamp rather than as webrtc::Timestamp or another time type.
The old name will be removed in WebRTC.

Bug: webrtc:9378
Change-Id: I4ed00e603fa81e6cc35e8322afd85f45297a1324
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4894126
Auto-Submit: Danil Chapovalov <danilchap@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1201808}
2023-09-26 21:12:58 +00:00
Danil Chapovalov
dce5b12e61 Set playout delay in webrtc using accessor and dedicated factory
To unblock refactoring webrtc::VideoPlayoutDelay to use stricter time types

Bug: webrtc:13756
Change-Id: I11b3d470c171948a4d732317ffdd9c7374cf628b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4797849
Commit-Queue: Danil Chapovalov <danilchap@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1185897}
2023-08-21 16:07:17 +00:00
Joe Downing
f63b2cc9cb Remove hack for crash testing
This won't be needed and we are rapidly approaching branch point
so it's time to remove this hack.

Change-Id: I600e72ed283015b432bc4aa7d160fefebf35d7dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4546979
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1146736}
2023-05-19 21:57:35 +00:00
Joe Downing
afa13f84a3 Add temporary code for testing crash integration
I'm adding a deterministic crash to the CRD host when a specific,
invalid encoder speed value is passed. I am going to use this for
release build testing and will remove it before branch point.

Change-Id: Ia1e8134e45b22309d502e7dc5cf70bda2ac1cd28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4528915
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1145337}
2023-05-17 14:42:43 +00:00
Joe Downing
ac8554f5f2 Use base::Time for the last_frame_received_timestamp_
This is a follow-up for feedback from the change I landed earlier today.

Change-Id: I437b0b85823ff178d4fa3655fd7279530de2d652
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4477490
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Joe Downing <joedow@chromium.org>
Auto-Submit: Joe Downing <joedow@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1135682}
2023-04-26 02:05:36 +00:00
Joe Downing
8b71f8b2a9 Set target framerate using a Video control message
The current implementation uses SDP format parameters to adjust
the framerate. This works well for VP8 and VP9 but is not the
supported way of adjusting the AV1 framerate and isn't possible
for H.264. Also we've seen that other browsers occasionally
mangle the SDP (or even set their own max-fr values) which further
complicates this logic.

I'd like to have a consistent way to set the target framerate for
the host, one which will work regardless of the currently
selected encoder so my new approach is to use a control message.

I will likely extend this in the future to control the encoder
speed as well though that is not in scope for this CL.

Bug: 217468304
Change-Id: I690420f5af02c5ed11f96bad5a7646cf9eb2cc95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4448768
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1135337}
2023-04-25 17:32:09 +00:00
Ho Cheung
175b2d7513 Convert base::clamp() to std::clamp() in remoting/
According to the instructions at crbug.com/1373621,
we should convert base::clamp() to std::clamp() in the code.

This is a continuation of the previous CLs
https://chromium-review.googlesource.com/c/chromium/src/+/4410943
https://chromium-review.googlesource.com/c/chromium/src/+/4461000
https://chromium-review.googlesource.com/c/chromium/src/+/4462333
https://chromium-review.googlesource.com/c/chromium/src/+/4462797
https://chromium-review.googlesource.com/c/chromium/src/+/4460969
https://chromium-review.googlesource.com/c/chromium/src/+/4462837
https://chromium-review.googlesource.com/c/chromium/src/+/4461080
https://chromium-review.googlesource.com/c/chromium/src/+/4462838
https://chromium-review.googlesource.com/c/chromium/src/+/4461296
https://chromium-review.googlesource.com/c/chromium/src/+/4461002
https://chromium-review.googlesource.com/c/chromium/src/+/4463737
https://chromium-review.googlesource.com/c/chromium/src/+/4463738
https://chromium-review.googlesource.com/c/chromium/src/+/4461779

Bug: 1373621
Change-Id: I447f4e46395883c44d9241f128f37bcd90705f5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4463757
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Auto-Submit: Ho Cheung <uioptt24@gmail.com>
Cr-Commit-Position: refs/heads/main@{#1134623}
2023-04-24 15:52:38 +00:00
Xiaohan Wang
156e33ceac base: Add BindPostTaskToCurrentDefault()
This CLs moves media::BindToCurrentLoop() to base/ and renames it to
BindPostTaskToCurrentDefault() to be consistent with BindPostTask().
It calls BindPostTask() with the current default SequencedTaskRunner
which is convenient in many cases (as can be seen in media/ code).

Note that BindPostTask() was also moved/renamed/modified previously
from media::BindToLoop(). So this is the continuation of that move.

Bug: 1140582, 1411753
Change-Id: I1ca6fe33ea2898c7e220b97bc18944d98d9b47bb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3088279
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Simeon Anfinrud <sanfin@chromium.org>
Owners-Override: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100826}
2023-02-03 03:43:53 +00:00
Joe Downing
0dab8a5d85 Allow client to set AV1 encoder speed via SessionOptions
This CL allows the client to set a new encoder speed (via
AOME_SET_CPUUSED) by adding a field in the SessionOptions when it
connects to the host. At the moment there isn't a better way to
do this as SDP does not define any method of switching encoder
speeds.

This CL also updates the default encoder speed for AV1 to '9'. This
is because I have noticed some quality issues when using 10 and I
think the quality improvement is worth the performance trade-off.

Bug: b:217468304
Change-Id: I0644fe68abef437862bfafee2bca4e9d2e2f152c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4189362
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Auto-Submit: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1095953}
2023-01-24 01:25:10 +00:00
Avi Drissman
135261e8e2 Update header includes for /base/functional in /r*
bind.h, callback.h, callback_forward.h, and callback_helpers.h
moved into /base/functional/. Update the include paths to
directly include them in their new location.

Bug: 1364441
Change-Id: I7cace513100cdf727d14419cf5817fda92941012
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4157696
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Owners-Override: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1091527}
2023-01-11 22:43:15 +00:00
Sean Maher
e672a665ff task posting v3: remove includes of runner handles and IWYU task runners
Turns out there's a lot of includes, so these will have to be removed
before deleting the implementation of the task runner handles.

To allow the deletion of the task runner handle headers, add
the sequenced/thread task runner handles where they are used in
the codebase with scripts.

This was done with an automated change, with a few touchups afterwards.
The code for the mass-refactor changes are here:
python:
https://paste.googleplex.com/5534570878337024
shell:
https://paste.googleplex.com/6466750748033024

In terms of touchups:
- add sequenced/thread task runner handles to
  the third_party/blink/public/DEPS, because multiple files were using
  it transitively anyways.
- rewrite certain parts of the codebase which used
  ThreadTaskRunnerHandles instead of CurrentDefaultHandles.
- fix a compile issue with forward-declaration in
  extensions/browser/extension_file_task_runner.h.

AX-Relnotes: n/a.
Bug: 1026641
Change-Id: I737ef32aee4e77c21eaa3a2bdc403a28322cf1b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4133323
Owners-Override: Gabriel Charette <gab@chromium.org>
Commit-Queue: Sean Maher <spvw@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1090532}
2023-01-09 21:42:28 +00:00
Joe Downing
6f596e50a7 Fix VideoChannelStateObserver uniqueness issue
WebRTC creates encoders as needed using a factory we provide but it does
not provide any sort of mapping or context to those encoders to indicate
which video stream the encoder is producing frames for.  Interestingly,
the SDP params are provided to the encoders but not the video streams so
in the past we have provided a WeakPtr to the VideoStream so the encoder
factory can pass a copy to each encoder as it is created so the encoder
can provide stats and pass on interesting SDP format info to the stream.

This worked fine when in single stream mode as no mapping was nneded,
however with the introduction of multi-stream mode, there are now N
video streams and no way to map an encoder to a stream (also we have a
problem where the 'last' video stream becomes the registered event sink
and all N encoders end up sending stats and SDP param info to the same
stream which messes the stats up and prevents framerate selection from
working).

This CL adds an event router class which uses the screen_id of the
display being captured to route events to the proper video stream. The
screen id should be stable as it is used in ClientSession to track each
video stream's lifetime.

Bug: b:24933048
Change-Id: I33285c47c61d6a744be8bb720f38b53d114c0ae1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4103904
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1083176}
2022-12-14 17:46:47 +00:00
Joe Downing
55c9ad866b Clean up VideoChannelStateObserver interface
This interface has a few methods which are no longer required. I am
removing them as that simplifies the multi-threaded dance for the
classes involved.  It also means less work in a follow-up CL where I add
a map for screen_id <-> VideoStream so that framerates and video stats
are delivered to the correct video stream instance.

Bug: b:249330480
Change-Id: I1a22a7510af8552877925ca3712a25f4cbcca8cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4098491
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1082217}
2022-12-13 03:14:38 +00:00
Sean Maher
52fa5a7f26 task posting v3: moving away from SequencedTaskRunnerHandle
To continue the migration away from TaskRunnerHandles, the codebase
was refactored using the following scripts:
shell script:
https://paste.googleplex.com/4673967729147904
python:
https://paste.googleplex.com/5302682490241024

This will do a few sed-like modifications, changing calls to methods of
SequencedTaskRunnerHandle to calls to methods of
SequencedTaskRunner::CurrentDefaultHandle, and swapping includes.

Bug: 1026641
AX-Relnotes: n/a.
Change-Id: I49e50a2bd1e78b00e7c067219fff96d2e0bc0b46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3983373
Commit-Queue: Gabriel Charette <gab@chromium.org>
Owners-Override: Gabriel Charette <gab@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1071032}
2022-11-14 15:53:25 +00:00
Florent Castelli
a9696751d0 remoting: Fix deprecated WebRTC API usage
Bug: None
Change-Id: Ib970cb7b3f1e4575a125b9eff9058a8c67a7a979
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004618
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Auto-Submit: Florent Castelli <orphis@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1067562}
2022-11-04 17:14:32 +00:00
Avi Drissman
d6cdf9b877 Update copyright headers in rlz/, remoting/
The methodology used to generate this CL is documented in
https://crbug.com/1098010#c95.

No-Try: true
No-Presubmit: true
Bug: 1098010
Change-Id: Ia865422057bdda8d25f347a651636f6f7f4d4278
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3900694
Owners-Override: Avi Drissman <avi@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1047637}
2022-09-15 19:52:53 +00:00
Joe Downing
55dba9c578 Add AV1 I444 support to CRD host
This CL adds the code needed to the CRD host to allow it provide frames
to the AV1 encoder using the I444 format.

Bug: 1329660
Change-Id: I3795a1aab734fdfe26bfc135ffc8b723a9eb6c41
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3867799
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041829}
2022-09-01 00:32:55 +00:00
Joe Downing
387c325502 Allow client to configure host framerate using SDP format params
The current method of using session-options to set the target framerate
works OK but it does not allow for changing the framerate within a
session w/o restarting it. Instead we should use the SDP format params
(the 'a=fmtp: ...' line in the SDP) to define the framerate as then
the client can request a renegotiation and allow the value to be
changed dynamically.

Bug: b:217468304
Change-Id: I327c18f5563ce6f015114b1acaadd8c25a7f8413
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3861027
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1041220}
2022-08-30 22:39:50 +00:00
Joe Downing
913ba6ec40 Use EncodedImageBuffer for encoded images
This removes a memory copy when passing the encoded frame data to
WebRTC.

Bug: b:217468304
Change-Id: Iac82ac9bc389402c869b2b9a31c237be79a71762
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3714883
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017315}
2022-06-23 20:50:34 +00:00
Joe Downing
895260c783 Run video encoder on a dedicated thread
This is a follow-up CL to crrev.com/c/3706198 where I started running
the PostEncodeTask synchronously to prevent large gaps between encoded
frames. One issue I noticed with the synchronous approach was that
we could accumulate a large queue of tasks on slower machines since
the capture thread would keep queueing new frames and the encoder
wrapper wouldn't drop them.

In this CL I take the same idea as before but instead of running the
encoder on the same sequence as the encoder wrapper, I create a
dedicated encoder thread. Now the encoder thread can be saturated
(I've only noticed small gaps (50-100us) between the encode tasks at
high frame rates) and the encoder wrapper will accumulate changes in a
|pending_frame_| which is then scheduled as soon as the previous encode
task has completed.

Bug: b:217468304
Change-Id: I75bc81a321b21396177a31417e6b29773fc9f213
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3714797
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1017270}
2022-06-23 18:34:57 +00:00
Joe Downing
54212909c9 Allow VP9 encoder speed to be set via SessionOptions
Per the thread with the WebRTC folks, it looks like there isn't a
preferred way to set the encoder speed from the client using an existing
mechanism.  While the discussion continues on whether to update an
existing standard to allow this, I'm adding a SessionOption to configure
it for testing/performance tuning purposes.

Bug: b:217468304
Change-Id: If7a950b56853734f49c091532b7b56d865002ff5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3712242
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1015581}
2022-06-17 23:05:23 +00:00
Joe Downing
7b0a989dd1 Plumb SessionOptions through to the webrtc video encoder wrapper
This CL adds on to a previous CL which added a SessionOption for
controlling the target framerate. I missed a usage of the constant
target value so I need to provide SessionOptions to that class.

Along they way I realized that the SessionOptions class has a GetInt
method which already does what I did in the WebrtcTransport CL I landed
so I cleaned up that callsite.

Bug: b:217468304
Change-Id: Ia81c43c8cb2a9f3dcbda3ccc15bd889ab07b79ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3712148
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1015577}
2022-06-17 22:56:49 +00:00
Joe Downing
4482f5a9fa Run PostEncode Tasks Synchronously
While digging into our capture/encode implementation, I noticed that
after each Encode() call, WebRTC would call the encoder wrapper again
which would drop the frame because encode_pending_ was true, then the
PostEncode task would run and the next encode request would occur after
a successful capture. In the graphs, this was represented by gaps in
between each encode call.

Existing code (w/ gaps): http://shortn/_obGooZhdR8
My change (no gaps): http://shortn/_zlaL1UHxYe

I tracked this down to some code which wraps the PostEncode callback
in a wrapper which ensures the task in executed on the current sequence.
This was added last year as there was some concern that the underlying
encoder could run on a different sequence. AFAICT, this is not the case
as |encoder_| is a remoting::WebrtcVideoEncoder which our team owns. The
VPX and AV1 encoders both execute synchronously and the H.264 encoder
already wraps the callback in a sequence bound PostTask a second time.

Bug: b:217468304
Change-Id: I5efd5f3362f4d91b01008d4bde56d018ecf6f8f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3706198
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1014604}
2022-06-15 20:08:48 +00:00
Joe Downing
e89ec62c59 AV1 encoder wrapper tweaks for Chrome Remote Desktop
This CL sets a number of encoder parameters tht we suggested by the WebM
folks. Initial testing with these values indicates a ~2X increase in
framerate and makes the codec usable (though it still seems a tad slower
than VP9 in my initial testing). I've also removed a NOTIMPL log from the
encoding path as it is way too chatty when using AV1.

Change-Id: Ieccc2a2e2f96e94f6dd5137c5ccb22492da3c878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3687980
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1010661}
2022-06-03 18:14:34 +00:00
Joe Downing
7d8c6d12a0 Updating some WebRTC classes to use webrtc constants/functions
This is a minor cleanup CL which replaces some hard-coded strings with
webrtc equivalents. I'm looking to land a webrtc change with some AV1
variations of the VP9 profile constants/function so I wanted to land
this first while I'm waiting for that CL to land.

Change-Id: I5a6032ba01534f1614ed0c4fefeaa2104b7766e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3671381
Auto-Submit: Joe Downing <joedow@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1008060}
2022-05-27 00:19:40 +00:00
Joe Downing
47eb936008 AV1 encoder implementation for CRD
This CL includes a working AV1 encoder implementation which will be
serve as a baseline for future tuning and performance testing.

Bug: b:217468304
Change-Id: I754d4d4d66a9beea54bff4a518bd669eff680de5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3648475
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1005101}
2022-05-19 03:49:28 +00:00
Joe Downing
98f24799e5 Adding an initial AV1 encoder implementation for CRD
This is a simple CL which adds a do-nothing impl and the integration
points needed for instantiating an AV1 encoder instance from the
client. I'm splitting this up as there is a fair amount of overlap
in the logic needed for VPX and AV1 (perhaps not surprising) so I
wanted to get this piece in first and then refactor the VPX encoder
so more of it can be shared between these two SW encoders.

Bug: b:217468304
Change-Id: I51f0cf6248f091332389cc9ddc3d0faa84f59a68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3633114
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: James Zern <jzern@google.com>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002876}
2022-05-12 22:11:15 +00:00
Lambros Lambrou
1e3e82742f [remoting] Don't drop frames if a key-frame was requested.
This CL fixes WebrtcVideoEncoderWrapper to never drop empty frames if
WebRTC requested encoding of a key frame.

Additionally, if a frame is dropped due to the encoder being busy with
a previous frame, any key-frame request will be remembered and applied
to the next (non-dropped) frame.

Bug: 1310116
Change-Id: I6a01b4c8735c5e636ed978a31368f814d059be8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3556668
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986235}
2022-03-29 00:12:24 +00:00
Daniel Cheng
9327fd3117 Migrate base::{size,empty,data} to STL equivalents in //remoting.
Bug: 1299695
Change-Id: I54825c7dcabd830716a0a0a8db62ed2e13b65b93
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3491923
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Owners-Override: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#975448}
2022-02-26 09:26:52 +00:00
Lambros Lambrou
8d587ec875 [remoting] Remove WebrtcFrameScheduler::GetSchedulerStats().
This simplifies handling of some per-frame stats:
* rtt_estimate
* bandwidth_estimate
* send_pending_delay

Instead of the encoder-wrapper passing these stats to the scheduler
(via the WebrtcVideoStream), it writes them directly into the encoded
frame (EncodedFrame::stats).

WebrtcVideoStream has code to generate and send the per-frame stats
(via video_stats_dispatcher_). It got the stats above from the
scheduler, but after this CL, it no longer needs the scheduler - it
gets everything it needs from the EncodedFrame.

This means that the stats-sending code can be decoupled from the
WebrtcVideoStream, which gives more flexibility for sending the
per-frame stats for multiple video-streams.

Change-Id: I4374de49ec024b7b17d9896a473513501d9993fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3263654
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#939677}
2021-11-09 02:37:18 +00:00
Lambros Lambrou
ba224494a2 [remoting host] Clean up video-scheduler.
This removes unneeded code from the scheduler:

* Encoding parameters are no longer needed (WebrtcVideoStream no
  longer uses them as it no longer creates encoders).
* Top-off encoding for empty frames is no longer handled by the
  scheduler. The scheduler sends all empty frames to the output sink,
  then WebRTC sends them to the encoder-wrapper which decides whether
  to encode or drop them (according to whether top-off is active).
* Large-frame detection for VP8 is handled by the encoder-wrapper.

Bug: 1192865
Change-Id: I9e45b4efb80e26bbe6e6010a7a2d09ec412f11db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3251888
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#936119}
2021-10-28 21:46:24 +00:00
Patrick Monette
643cdf6190 Replace base/task/ temporary forward headers with their final locations
Note to QA: This merely changes includes and should not be blamed
for files it touched.

Bug: 1255932
Change-Id: I1ce4e31efd5792ebf2080812e665cae838a54972
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3226943
Reviewed-by: Gabriel Charette <gab@chromium.org>
Owners-Override: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#932153}
2021-10-15 19:13:42 +00:00
Patrick Monette
3d7d70920a Replace task-related headers to their equivalent in base/task/
This CL was generated by using tools/git/move_source_file.py to change
the includes for those files:
base/bind_post_task.h
base/deferred_sequenced_task_runner.h
base/post_task_and_reply_with_result_internal.h
base/sequenced_task_runner.h
base/sequenced_task_runner_helpers.h
base/single_thread_task_runner.h
base/task_runner.h
base/task_runner_util.h
base/updateable_sequenced_task_runner.h

Then formatted using "git cl format". DEPS files were fixed with a
simple search and replace script.

Bug: 1255932
Change-Id: I0d9b5ddd9260fde5e4581e6c6e0080bdb0ed2c44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3209175
Reviewed-by: Gabriel Charette <gab@chromium.org>
Owners-Override: Gabriel Charette <gab@chromium.org>
Commit-Queue: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/main@{#929867}
2021-10-08 20:27:23 +00:00
Peter Kasting
e5a38eddbd Migrate "base::TimeDelta::FromX" to "base:X".
All changes were done automatically with git grep, sed, xargs, etc.

No-Presubmit: true
No-Try: true
Bug: 1243777
Change-Id: I7cc197e9027f7837cd36afc67a209079f85ec364
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3198824
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Owners-Override: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#927512}
2021-10-02 03:06:35 +00:00
Johannes Kron
700375362a Use new VP9 and H264 profile level include files
webrtc/media/base/h264_profile_level_id.* and vp9_profile.h
was recently moved to webrtc/api/video_codecs, see
https://webrtc.googlesource.com/src.git/+/c3fcee7c3a7714afc3e37d4753b40f4fdbc3653e

This CL cleans up the chromium code so that the deprecated
files can be removed in WebRTC.

Bug: chromium:1187565
Change-Id: I012fb577b81e944de8e0f441635c7d38bbb054be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3107107
Reviewed-by: Mirko Bonadei <mbonadei@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Johannes Kron <kron@chromium.org>
Cr-Commit-Position: refs/heads/main@{#914251}
2021-08-23 10:31:50 +00:00
Lambros Lambrou
4b5a22dea9 [remoting host] Move empty-frame logic into the encoder-wrapper.
The scheduler's OnCapturedFrame() method returns a boolean to signal
whether the captured frame should be sent to the encoder or dropped.
This turns out to be incompatible with the standard encoding pipeline,
because WebRTC (or the encoder-wrapper) may have dropped the prior
non-empty frame. So this CL moves the empty-frame-dropping logic into
the encoder-wrapper, and the scheduler never drops frames.

This also removes some logging of dropped frames, which can become
very spammy because the encoder-wrapper can now receive frames at 33ms
intervals (30FPS).

Bug: 1226184
Change-Id: I7c006e54dbb1c888a9fd437083b37ea9c79389b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3018058
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900616}
2021-07-12 20:27:26 +00:00
Lei Zhang
d4a2f8e1ed Swap base/stl_util.h to base/cxx17_backports.h in many files.
Files that use base::size() and base::data() should use
cxx17_backports.h directly, instead of getting it transitively through
stl_util.h.

Bug: 1210983
Change-Id: Icc5f425c23ef4e69283293e0d0d6d733fe3b4ba1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2974072
Reviewed-by: Nico Weber <thakis@chromium.org>
Owners-Override: Nico Weber <thakis@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#899418}
2021-07-08 03:43:28 +00:00
Lambros Lambrou
cff902ca51 [remoting host] Detect dropped frames and optimize the update-region.
This adds an incrementing id to each DesktopFrame, and updates the
encoder-wrapper to detect if WebRTC dropped any frames. If the
encoder-wrapper receives consecutive frames (without any drops in
between), it will use the DesktopFrame's original updated-region
instead of the VideoFrame's update-rectangle that WebRTC provided to
Encode().

This means that, most of the time, the encoder will receive an
accurate updated-region instead of a bounding rectangle, which may
result in faster encode times. And the large-frame-detection logic
should be more accurate for these frames.

Bug: 1192865
Change-Id: If53e0df50c39615258ed620312fb25131b6c8fc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3002682
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898216}
2021-07-02 16:47:48 +00:00
Lambros Lambrou
213f185cd4 [remoting] Detect large frames in WebrtcVideoEncoderWrapper.
This copies the large-frame-detection logic from the scheduler into
the new encoder. After the switch to standard-encoding-pipeline, the
scheduler's large-frame-detection no longer had any effect.

This is only used for VP8 (though in future, similar logic may be
used for H264). The VP9 encoder already has this feature: it
re-encodes the frame at lower quality if it detects overshoot of
target FPS.

Bug: 1192865
Change-Id: I6f7d1143182e2bd984a4bdda32de9da4bdf0b00e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2987254
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#897210}
2021-06-30 01:42:56 +00:00
Lambros Lambrou
5e82e69589 [remoting host] Track frame update-regions in encoding pipeline.
This sets each VideoFrame's update-rectangle to match the wrapped
DesktopFrame's update-region. This ensures WebRTC knows about each
frame's update-rect so it can combine these rectangles in case it
drops a frame.

The encoder-wrapper may also drop frames itself, so this CL also
ensures that any dropped frames' update-rects are accumulated into the
next frame sent to the encoder.

Bug: 1192865
Change-Id: I3eab5119588b042730a9e93d9e7f3a01aad38c95
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2941965
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#891438}
2021-06-10 23:20:07 +00:00
Lambros Lambrou
60da0f0e68 [remoting host] Track frame statistics per frame.
With the standard encoding pipeline, the scheduler may start capturing
a new frame before the current frame has been encoded and sent. This
means that storing the stats in WebrtcVideoStream::current_frame_stats_
is inadequate. This CL enables the frame stats to be attached to each
frame, before sending it to WebRTC's output sink. The stats will be
extracted from the frame by the new encoder, then re-attached to the
corresponding EncodedFrame after encoding.

In the current legacy code, the stats are attached manually in
WebrtcVideoStream::OnFrameEncoded(). In the new code, the stats
will be attached in WebrtcVideoStream::OnCapturedFrame().

Because the stats are attached via std::unique_ptr, this CL makes
EncodedFrame into a movable-but-not-copyable type, which makes
sense anyway since the frame data can be very large.

Bug: 1192865
Change-Id: Ie8eaa03e0912009d407eafca2f90de92a1f52845
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2911526
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#886533}
2021-05-26 01:21:26 +00:00
Lambros Lambrou
202f99608e [remoting host] Guard against encoding multiple frames in parallel.
With the standard encoding pipeline, WebRTC will occasionally try to
encode another frame while a frame is being encoded (before the
registered encode-complete callback is run).

The remoting/codec encoders have not been developed or tested for
multiple parallel encodes. The frame-scheduler currently waits for
encode-complete before scheduling a new capture, but this will
change with standard-encoding-pipeline. So this CL protects this by
dropping any Encode() requests while a frame is still pending.

This will also ensure that the stored RTP timestamp, and per-frame
stats (when this is implemented) are valid for the frame, as there
can now be only 1 frame encoded at a time.

Bug: 1192865
Change-Id: I31cff4aea119d32e23aa7a130f43e759cf5510a6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2911514
Auto-Submit: Lambros Lambrou <lambroslambrou@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#885796}
2021-05-22 19:49:13 +00:00
Lambros Lambrou
83e723ced0 [remoting host] Add new webrtc::VideoEncoder implementation.
This is a replacement for the current WebrtcDummyVideoEncoder, to be
used for the standard encoding pipeline. This class will be
instantiated by WebRTC via the VideoEncoderFactory implementation, and
it lives on whatever thread WebRTC uses internally for video-encoding.

Status notifications are passed to the VideoStream and frame-scheduler
via a VideoChannelStateObserver on the main thread.

This class handles the logic for setting encode parameters which is
currently managed by the video-scheduler (such as clear_active_map).

Bug: 1192865
Change-Id: I33b1decafdcba23870a69dbd68e835fbbac5f53f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2878558
Reviewed-by: Ilya Nikolaevskiy <ilnik@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#883623}
2021-05-17 21:23:39 +00:00