This could potentially lead to unnecessary restarts since it is also
started after the encoder is created. However, it is needed since the
hardware acceleration support can change even though the encoder has
not been recreated.
Bug: b/145730598
Change-Id: Iad1330e7c7bdf769a68c4ecf7abb6abbf3a4fe71
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/203140
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Jakob Ivarsson <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33060}
This CL implements a Resource that aggressively reports overuse or
underuse until the encoded stream has the max pixels specified. The
pixel limit is controlled with a field trial, e.g:
--force-fieldtrials="WebRTC-PixelLimitResource/Enabled-307200/"
This caps the resolution to 307200 (=640x480). This can be used by the
TestBed to simulate being CPU limited. Note that the resource doesn't
care about degradation preference at the moment, so if the degradation
preference would be set to "maintain-resolution" the PixelLimitResource
would never stop reporting overuse and we would quickly get a low-FPS
stream.
PixelLimitResource runs a repeating task and reports overuse, underuse
or neither every 5 seconds. This ensures we quickly reach the desired
resolution.
Unit tests are added. I did not add any integration tests (I think
that's overkill for a testing-only resource) but I have manually
verified that this works as intended.
This CL also moves the FakeVideoStreamInputStateProvider into a test/
folder and exposes video_stream_adapter.cc's GetLowerResolutionThan().
Bug: webrtc:12261
Change-Id: Ifbf7c4c05e9dd2843543589bebef3f49b18c38c0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195600
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32771}
External reasons here are simulcast configuration and
source resolution change.
Initial frame dropper should be enabled in these cases because the
client can request way too big resolution for available bitrate and
usual quality scaling would take too long.
Bug: none
Change-Id: I02fbbd3c15b53b39672c083c2a1f9a780256c507
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/195004
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32707}
This is a reland of f5e261aaf65cdf2eb903cdf40d651846be44f447
This CL disables RTC_NO_UNIQUE_ADDRESS on MSan builds since
there have been some issues.
Original change's description:
> Introduce RTC_NO_UNIQUE_ADDRESS.
>
> This macro introduces the possibility to suggest the compiler that a
> data member doesn't need an address different from other non static
> data members.
>
> The usage of a macro is to maintain portability since at the moment
> the attribute [[no_unique_address]] is only supported by clang
> with at least -std=c++11 but it should be supported by all the
> compilers starting from C++20.
>
> Bug: webrtc:11495
> Change-Id: I9f12b67b4422a2749649eaa6b004a67d5fd572d8
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173331
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32246}
Bug: webrtc:11495, webrtc:12218
Change-Id: I4e6c7cc37d3daffad2407c9a2acfa897fa5b426a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189968
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32668}
And use it in a few places that were using RTC_CHECK(false) or FATAL()
to do the exact same job. There should be no change in behavior.
Bug: none
Change-Id: I36d5e6bcf35fd41534e08a8c879fa0811b4f1967
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/191963
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32567}
"warning: control reaches end of non-void function [-Wreturn-type]"
Reported by gcc (8.3)
In all the reported cases, the end of function is never actually
reached. Add RTC_CHECK(false) to ensure the compiler is aware that
this path is a dead-end.
Bug: webrtc:12008
Change-Id: I7f816fde3d1897ed2774057c7e05da66e1895e60
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189784
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Fabien VALLÉE <fabien.vallee@netgem.com>
Cr-Commit-Position: refs/heads/master@{#32503}
(Reland with no changes after the fix to the downstream project)
This can be overriden for kNative frame types to perform scaling efficiently.
Default implementations for existing buffer types require actual
buffer implementation, thus this CL also merges "video_frame"
with "video_frame_I420" build targets.
Originally Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186303
(Landing with TBR as it's unchaged reland of already approved CL)
TBR=nisse@webrtc.org,sakal@webrtc.org
Bug: webrtc:11976, chromium:1132299
Change-Id: Ia23f7d3e474bd9cdc177104cc5c6d772f04b210f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/187345
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32362}
This reverts commit f5e261aaf65cdf2eb903cdf40d651846be44f447.
Reason for revert: Breaks downstream projects.
Original change's description:
> Introduce RTC_NO_UNIQUE_ADDRESS.
>
> This macro introduces the possibility to suggest the compiler that a
> data member doesn't need an address different from other non static
> data members.
>
> The usage of a macro is to maintain portability since at the moment
> the attribute [[no_unique_address]] is only supported by clang
> with at least -std=c++11 but it should be supported by all the
> compilers starting from C++20.
>
> Bug: webrtc:11495
> Change-Id: I9f12b67b4422a2749649eaa6b004a67d5fd572d8
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173331
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32246}
TBR=mbonadei@webrtc.org,kwiberg@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:11495
Change-Id: Ice318d1b11ca3dff09c190187a0b0a32ca945fe3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/186944
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32335}
This macro introduces the possibility to suggest the compiler that a
data member doesn't need an address different from other non static
data members.
The usage of a macro is to maintain portability since at the moment
the attribute [[no_unique_address]] is only supported by clang
with at least -std=c++11 but it should be supported by all the
compilers starting from C++20.
Bug: webrtc:11495
Change-Id: I9f12b67b4422a2749649eaa6b004a67d5fd572d8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/173331
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32246}
The former was unused, the latter is replaced with the explicit C++11
deletions. The related RTC_DISALLOW_COPY_AND_ASSIGN is left for now,
it is used in a lot more places.
Bug: None
Change-Id: I49503e7f2b9ff43c6285f8695833479bbc18c380
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185500
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32224}
This is a reland of ba8abbb630cdd9d05e22c830d0845e920762850d
This can be relanded as the queuing issues that were causing a
crash in the WebRTC roll in Chromium have been resolved. I have
added the Chromium failing targets to the CQ for this commit and
they have succeeded.
Original change's description:
> [Adaptation] Remove QualityScalerResource when disabled.
>
> Bug: webrtc:11843
> Change-Id: I2d3e40356c266f189db0242f3c7590e6d83e4456
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181369
> Commit-Queue: Evan Shrubsole <eshr@google.com>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#31924}
Bug: webrtc:11843
Change-Id: I228331293060ef996f1dd7f8e18d52b0818f526b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/182080
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31996}
Resource adaptation needs refactoring for async adaptations. For now
the resource adaptation processor can work on the encoder thread, until
it is refactored to support async adaptation.
Bug: webrtc:11867
Change-Id: I9c46da356db19c0fd52748c999ccb216f2ca923b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/182040
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31991}
There was a small miss in https://webrtc-review.googlesource.com/c/src/+/181369
which allowed frame dropping to re-enable, and there was no test to catch this.
This case has been fixed along with a test to ensure this isn't missed in the future.
Bug: webrtc:11843
Change-Id: I201aa451d4751586c780a07dc72a7401aed78088
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181661
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31936}
Some restrictions previously were preventing adaptation up caused by the quality resource. However, it makes sense to use the same restrictions in the case of other resources. This CL removes now unneeded wire-up of reason/resource causing adaptation.
Bug: webrtc:11771
Change-Id: Iec301a59d2a41d32d23b6be340f3b5637d697e52
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181580
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31926}
Instead it should be ensured that it is started with the correct config.
This removes confusion regarding a resource state. If the resource
check is stopped then the adaptations for that resource should be
removed, and there is no way to determine that if we have one method for
stop for both reconfigure and shutdown.
Bug: webrtc:11843
Change-Id: I491f2fd1f4f803a4610124c7b0026ad75ab4a9cc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181368
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31913}
Since the degradation preference can be fetch from the thread safe
provider, this removed the need to have 2 queues.
Bug: None
Change-Id: I94cb8b7d3d2950acfe0ad0a6d46edc038ed8e286
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177523
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31898}
It was not used by any class and all future uses can use the
VideoSourceRestrictionsListener.
Bug: webrtc:11834
Change-Id: I5c71b93cc503f458dce0ccdd78b91b5a1debc56d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181062
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31896}
This way can double adapt right away instead of relying
on the qp scaler checking soon into the future.
Bug: webrtc:11830
Change-Id: I8e878168303cf6a4c3edcf3997dd8ac2413a4479
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/181060
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31895}
This is one less dependency on the task queue, and will make
things like removing resources and cleanup much easier in the future.
Bug: webrtc:11754
Change-Id: I732f1935d1b58ffe09ca2a2bf59beebc1930214d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178869
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31686}
It has no relevance in ResourceAdaptationProcessorInterface.
The logic moves to an interaction directly on the VideoStreamAdapter.
Bug: webrtc:11700
Change-Id: I4c7b3e1e1361722a69b71e8f9bde33f5909c011a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/178380
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31634}
This patch creates a new class which provides the DegradationPreference
thread safe to all classes that need if (BalancedConstraint and
QpScalerResource). It also broadcasts to all listeners when the
preferences are updated, so the ResourceAdaptationProcessor can update
the video if needed.
In future work, this could be used to remove the need for two task
queues for the VideoStreamEncoder resources.
Bug: webrtc:11700
Change-Id: I05480db8b7157b5643f6f86ec9c64850839b3e76
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/177522
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31623}
This moves this responsibility from the ResourceAdaptaitonProcessor
to the VideoStreamAdapter. A new interface for listening to adaptation
changes was added, and the ResourceAdaptationProcessor now listens on
the VideoStreamAdapter for those changes.
This means that going forward,
1. We can do adaptations outside of resource limitations, like setting
prior adaptations on a resource like initial frame dropper is designed
to.
2. Adaptations can be on a different queue than the
ResourceAdaptaitonProcessor's queue since updates are pushed through
the listener.
Bug: webrtc:11700
Change-Id: I6de0dec748dba095e702f0b9893c5afa50b51aa9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176859
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31615}
This introduces a new class for encapsulating the QualityRampupExperiment
R=hbos@webrtc.org
Bug: webrtc:11553
Change-Id: If2f2347cdcbd0c79821355f90e2d7ad3171143b5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176363
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31531}
A more detailed explaination is in the bug, but this changes
the way that adaptation happens when multiple resources are
limited. Only the one that is most limited can trigger an
adaptation up. If multiple resources are most limited both
need to underuse to adapt up.
Some of the changes in this patch to make it all work:
* VideoStreamEncoder unittests that did not reflect this
new behaviour have been changed.
* PeekNextRestrictions returns the adaptation counters as
well as the restrictions.
* Adaptation statstics have changed so that when adapting
up all resources are tagged as triggering the adaptation.
Additionally the statistics for the current adaptation is
now the total number of adaptations per reason, rather then
the number of adaptations due to that reason.
* PreventAdaptUpDueToActiveCounts is removed as most limited
resource is a strong implementation of that.
Bug: webrtc:11553
Change-Id: If1545a201c8e019598edf82657a1befde8b05268
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176128
Commit-Queue: Evan Shrubsole <eshr@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31497}
This is a prerequisite to implementing it externally.
Bug: webrtc:11525
Change-Id: I9cb3b4418396485d3eb9f25cafa51cbff6db7817
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176401
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31418}
ResourceListener::OnResourceUsageStateMeasured() now takes
ResourceUsageState as argument, making Resource::UsageState()
superfluous.
With the existing "fire-and-forget" behavior of always clearing usage
state on reacting to a signal, there is no longer a need to call
ClearUsageState() so this too is removed. (We may want to have a
callback in the future to hint to the Resource that it is a good idea
to clear internal measurement samples, i.e. because the load of the
system is about to change, but we can revisit that when we need it.)
Moving the usage state to the callback has the benefit of getting rid
of the assumption that UsageState() has to return the same value every
time it is called in the same task.
This CL is also the final nail in the coffin for Resource needing to
know about the adaptation task queue: ResourceAdaptationProcessor's
ResourceListener now takes care of posting to the adaptation task
queue. To support this, the processor's SequenceChecker is replaced
by a TaskQueueBase pointer.
Bug: webrtc:11525, webrtc:11618
Change-Id: I2277e71cc3759c85b62465020935603f03792c94
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176376
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31416}
IsAdaptationUpAllowed is moved from Resource to AdaptationConstraint.
OnAdaptationApplied is moved from Resource to AdaptationListener.
In a future CL, Resource will be moved to api/, but
AdaptationConstraint and AdaptationListener will stay in call/.
The processor, encode stream and manager are updated to keep track of
both resources, constraints and listeners. Fakes and tests are updated.
After this CL, the manager's inner classes that prevent adaptation
implement AdaptationConstraint instead of Resource.
Bug: webrtc:11525
Change-Id: Ie9cd5b1ba7d8e161951e131ab8f6bd9d5cf765bf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176368
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31409}
This CL is in preparation for moving Resource to the api/ folder. It
does not move it, but makes it such that the moving CL can be a pure
move.
In order to do this, we must stop depending on rtc_base/rtc::TaskQueue
in favor of api/webrtc::TaskQueueBase.
There are also other rtc_base/ dependencies that we do not want to
expose to the api/ folder, like critical sections and thread
annotations which are not publically exposed. To get around this, we
make Resource an abstract interface and move all of the base class
functionality into a new non-api/ class: VideoStreamEncoderResource.
The Resource now has Register/UnregisterAdaptationTaskQueue() methods.
By explicitly unregistering, we can ensure validity of the pointer even
if the Resource outlives the PeerConnection. While public interface
methods are only to be called on the adaptation task queue, posting to
the task queue happens off-queue, so a |lock_| is introduced to guard
it.
Bug: webrtc:11525
Change-Id: I50b3a30960cdec9032016c779b47001c01dad32f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176320
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31402}
This CL adds a 1 second cooldown period for QualityScalerResource to
signal kUnderuse due to being disabled.
If underuse is signaled every frame, any RTC_LOGging performed by the
ResourceAdaptationProcessor would become very spammy.
Plus we don't need to adapt every single frame.
Bug: webrtc:11616
Change-Id: Id76e5ca39a5e5dac9b71fdab79fb4f3dd5aeab1f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/176228
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31374}
This CL unblocks future Call-Level Mitigation strategies by moving the
ResourceAdaptationProcessor to a separate task queue. This signifies a
major milestone in the new resource adaptation architecture because
with this CL the threading model is in place and moving the Processor
to the Call and increasing its responsibilities is made possible.
In this CL, we still have one Processor per VideoStreamEncoder and the
VideoStreamEncoder is responsible for the creation and the destruction
of its Processor and that Processor's task queue. But the PostTasks are
in place and the decision-making is executed on a separate queue.
This CL:
- Moves ResourceAdaptationProcessor to an adaptation task queue.
It continues to be entirely single-threaded, but now operates on a
separate task queue.
- Makes Resources thread-safe: Interaction with the Processor, i.e.
OnResourceUsageStateMeasured() and IsAdaptationUpAllowed(), happens
on the adaptation task queue. State updates are pushed from the
encoder task queue with PostTasks.
- QualityScalerResource operates on both task queues; the QP usage
callbacks are invoked asynchronously.
- The VideoStreamEncoderResourceManager operates on the encoder task
queue with the following exceptions:
1) Its resources are accessible on any thread (using a mutex). This
is OK because resources are reference counted and thread safe.
This aids adding and removing resources to the Processor on the
adaptation task queue.
2) |active_counts_| is moved to the adaptation task queue. This makes
it possible for PreventAdaptUpDueToActiveCounts to run
IsAdaptationUpAllowed() on the adaptation task queue.
A side-effect of this is that some stats reporting now happen on
the adaptation task queue, but that is OK because
VideoStreamEncoderObserver is thread-safe.
The Manager is updated to take the new threading model into account:
- OnFrameDroppedDueToSize() posts to the adaptation task queue to
invoke the Processor.
- OnVideoSourceRestrictionsUpdated(), now invoked on the adaptation
task queue, updates |active_counts_| synchronously but posts to the
encoder task queue to update video source restrictions (which it
only uses to calculate target frame rate).
- MaybePerformQualityRampupExperiment() posts to the adaptation task
queue to maybe reset video source restrictions on the Processor.
|quality_rampup_done_| is made std::atomic.
Bug: webrtc:11542, webrtc:11520
Change-Id: I1cfd76e0cd42f006a6d2527f5aa2aeb5266ba6d6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174441
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31231}
This CL makes the VideoStreamEncoderResourceManager's inner Resources
(PreventAdaptUpDueToActiveCounts,
PreventIncreaseResolutionDueToBitrateResource and
PreventAdaptUpInBalancedResource) not directly depend on any of the
manager's states that will continue to live on the encoder task queue
when the adaptation task queue is introduced in the next CL.
PreventAdaptUpDueToActiveCounts depends on effective degradation
preference, which it can get from the Processor, and the active counts,
which will move to the adaptation queue and is safe to use.
PreventIncreaseResolutionDueToBitrateResource depends on encoder
settings and target bitrate. This Resource now listens to these states
being updated, which may be implemented with a PostTask when the
adaptation queue is added.
PreventAdaptUpInBalancedResource depends on the effective degradation
preference, which it can get from the Processor; balanced settings,
which is a const readonly struct (thread-safe); and encoder target
bitrate, which it listens for being updated (to be PostTask'ed).
All resources depends on GetReasonFromResource() which will be callable
from the adaptation queue.
Bug: webrtc:11542, webrtc:11520
Change-Id: Ifa7bd87d9d8729988073f78f6a37c6f3b8aa4db1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174807
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/master@{#31220}
This CL adds a queue for pending QualityScalerQpUsageHandlerCallbacks
and private methods for "Queueing", "Handling" and "Aborting" them,
using a sequence number as an ID to ensure we don't accidentally invoke
the same callback twice.
Because we don't have the adaptation task queue yet, callbacks are still
synchronously handled, which means the "pending callbacks" queue would
never have more than 1 element. However, when the adaptation task queue
is added and this is made asynchronous, it will be possible for multiple
callbacks to be pending simultaneously. This design is future-proof.
This CL is split out to aid reviewability. The CL that adds the
adaptation task queue will affect a lot of code. By landing this
separately, the adaptation queue CL will be easier to review.
This CL adds quality_scaler_resource_unittest.cc.
Bug: webrtc:11542, webrtc:11520
Change-Id: I00e7f6bfda9f8e8e82ec25916aa48e9349c8d70c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174802
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31219}
In a future CL, adaptation processing and stream encoder resource
management will happen on different task queues. When this is the case,
asynchronous tasks will be posted in both directions and some resources
will have internal states used on multiple threads.
This CL makes the Resource class reference counted in order to support
posting tasks to a different threads without risk of use-after-free
when a posted task is executed with a delay. This is preferred over
WeakPtr strategies because WeakPtrs are single-threaded and preferred
over raw pointer usage because the reference counted approach enables
more compile-time and run-time assurance. This is also "future proof";
when resources can be injected through public APIs, ownership needs to
be shared between libwebrtc and the application (e.g. Chrome).
To reduce the risk of making mistakes in the future CL, sequence
checkers and task queue DCHECKs are added as well as other DCHECKs to
make sure things have been cleaned up before destruction, e.g:
- Processor gets a sequence checker. It is entirely single-threaded.
- Processor must not have any attached listeners or resources on
destruction.
- Resources must not have any listeners on destruction.
- The Manager, EncodeUsageResource and QualityScalerResource DCHECKs
they are running on the encoder queue.
- TODOs are added illustrating where we want to add PostTasks in the
future CL.
Lastly, upon VideoStreamEncoder::Stop() we delete the
ResourceAdaptationProcessor. Because the Processor is already used in
posted tasks, some if statements are added to ensure the Processor is
not used after destruction.
Bug: webrtc:11542, webrtc:11520
Change-Id: Ibaa8a61d86d87a71f477d1075a117c28d9d2d285
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/174760
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@google.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31217}
