ScreenCapturerMac::RegisterRefreshAndMoveHandlers crashes on 10.7 #1

blueboxd · 2022-11-01T13:18:09Z

As mentioned in blueboxd/chromium-legacy#85.
CGDisplayStream* is not available on 10.7, so need to revive old implementation(440b4be) for 10.7.

The text was updated successfully, but these errors were encountered:

blueboxd · 2022-11-03T08:37:21Z

fixed as 64cec11

The Chromium RTCVideoEncoder unfortunately doesn't set if the result is at target quality, and the definition of the threshold is buried in libvpx_vp8_encoder.h. This change * Updates VideoStreamEncoder to postprocess an incoming EncodedImage by interpreting the incoming QP information instead. * Updates the related VideoStreamEncoder test to simulate an encoder producing images around the QP threshold. * Updates the steady state VP8 screencast QP threshold to a central include file. * Moves this and previously existing EncodedImage post-processing to a new method AugmentEncodedImage. (cherry picked from commit e1a198b) Bug: b/245029833, chromium:1364573 Change-Id: I69ae29ffe501e84f28908f7d9a8cfd066ba82b43 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275380 Reviewed-by: Erik Språng <sprang@webrtc.org> Commit-Queue: Markus Handell <handellm@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#38091} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275775 Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Commit-Position: refs/branch-heads/5304@{#1} Cr-Branched-From: 024bd84-refs/heads/main@{#38083}

This is exposing something that is already exposed in the legacy getStats() API and is only available if the "video-timing" header extension is used. Adding this metric here should unblock legacy getStats() API deprecation. The follow-up to unship or standardize this metric is tracked by https://crbug.com/webrtc/14586. (cherry picked from commit c5f8f80) Bug: webrtc:14587, chromium:1376604 Change-Id: Ic3d45b0558d7caf4be2856a4cd95b88db312f85e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279860 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Henrik Boström <hbos@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#38444} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279900 Cr-Commit-Position: refs/branch-heads/5359@{#1} Cr-Branched-From: fb3bd4a-refs/heads/main@{#38387}

…ve on dtor In the experiment WebRTC-SendPacketsOnWorkerThread ensure the safety flag is set not alive even if Start/Stop has never been called. (cherry picked from commit 12046bf) Bug: webrtc:14502, chromium:1382602 Change-Id: I01c1e663762c8bb848e9bc31b2dcb22d38d0d1e8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283380 Commit-Queue: Per Kjellander <perkj@webrtc.org> Reviewed-by: Erik Språng <sprang@webrtc.org> Reviewed-by: Evan Shrubsole <eshr@webrtc.org> Auto-Submit: Per Kjellander <perkj@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#38624} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/285381 Reviewed-by: Philip Eliasson <philipel@webrtc.org> Commit-Queue: Evan Shrubsole <eshr@webrtc.org> Cr-Commit-Position: refs/branch-heads/5414@{#1} Cr-Branched-From: a3e51df-refs/heads/main@{#38608}

When disabling a spatial layer, reconfiguration of the encoder is not necessary (bitrate will never be assigned to the inactive layer anway). This CL however makes sure we reconfigure the encoder when a spatial layer is activated. Some encoder implementations may encoder the wrong number of spatial layers if the active layers have not beens set correctly. (cherry picked from commit 17043b8) Bug: webrtc:14809, b/261097903, chromium:1310794 Change-Id: I8d34aaec95eb50a9717c06ea38f25088e5a96429 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290560 Commit-Queue: Philip Eliasson <philipel@webrtc.org> Auto-Submit: Erik Språng <sprang@webrtc.org> Reviewed-by: Philip Eliasson <philipel@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#38999} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290780 Commit-Queue: Erik Språng <sprang@webrtc.org> Cr-Commit-Position: refs/branch-heads/5481@{#1} Cr-Branched-From: 2e1a9a4-refs/heads/main@{#38901}

Cherry picked from commits: b311f6a fd29662 (cherry picked from commit b311f6a) No-Try: True Bug: chromium:1348011 Change-Id: I3219e74c49ff77e00b2224c8cf82f78d1e0fd9cf Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291708 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Johannes Kron <kron@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#39254} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/292460 Cr-Commit-Position: refs/branch-heads/5563@{#1} Cr-Branched-From: 6c032cb-refs/heads/main@{#39207}

…back Since AddTrack now has an implicit init_encodings value, it will also have a StableState saved when associating a transceiver. That state may not have a saved mid and mline_index, and so on a rollback, it could blindly reset the mid and mline_index of an associated transceiver. This is wrong, the mid and mline_index of associated transceivers should only be updated when the StableState objects actually have one saved. (cherry picked from commit b3d424c) Bug: chromium:1424238 Change-Id: I8e80a04cd072d90200ca7643de892c0ef29b1f1a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297920 Commit-Queue: Florent Castelli <orphis@webrtc.org> Reviewed-by: Henrik Boström <hbos@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#39577} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/297983 Reviewed-by: Per Kjellander <perkj@webrtc.org> Commit-Queue: Henrik Boström <hbos@webrtc.org> Cr-Commit-Position: refs/branch-heads/5615@{#1} Cr-Branched-From: cdfeb4f-refs/heads/main@{#39376}

… spatial drop. The EncoderStreamFactory triggers different code paths depending on `number_of_streams`: one for simulcast and one for non-simulcast. The non-simulcast path is desired for both normal streams and SVC streams. The simulcast path gives sensible max bitrates for 4:2:1 scenarios, but when encodings like {active,inactive,inactive} is specified in order to do standard SVC, the max bps of the first encoding is so low that an SVC stream will never send more than its first spatial layer (even when scaleResolutionDownBy is 1). Because of this, standard SVC is broken. This CL fixes this problem by using the CreateDefaultVideoStreams() code path instead, which is the same one that legacy SVC uses. With this fix, legacy and standard SVC produce the same behavior regarding bitrate. An added benefit of this is that numberOfSimulcastStreams == 1 in the standard SVC path as well. {active,inactive,inactive} tests are updated to verify the full resolution is achieved after ramp-up. I've also confirmed that this fixes the bug in Canary, see https://crbug.com/1428098#c2. (cherry picked from commit c99753a) Bug: chromium:1428098, webrtc:15041, webrtc:15034 Change-Id: Ia1eb4ff59c4e2a56af833f7ac907a66bca8ea054 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299147 Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Commit-Queue: Henrik Boström <hbos@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#39697} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299460 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Commit-Position: refs/branch-heads/5672@{#1} Cr-Branched-From: d3e765e-refs/heads/main@{#39661}

If the caller calls RegisterObserver() on the network thread while the state is not kOpen but there are queued received data, those received data will be immediately delivered to the observer before the state is transitioned to kOpen, which may break the observer's assertions and cause problems. The problem turns out to be that, when SctpDataChannel::RegisterObserver calls DeliverQueuedReceivedData(), the data will be passed to the observer without checking the |state_| first, meanwhile SctpDataChannel::UpdateState does effectively check the state and null-check |observer_| before delivering the received data. This CL fixes this by simply making DeliverQueuedReceivedData() also check `state_ == kOpen`. In case the state transitions to kOpen after RegisterObserver() is called, the first DeliverQueuedReceivedData() call will be no-op, while the second DeliverQueuedReceivedData() call will do the work. (cherry picked from commit 2083894) No-Try: True Bug: chromium:1442696 Change-Id: If25ce6a038d704939b1a8ae73d7ced110448b050 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304687 Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#40036} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/305380 Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Cr-Commit-Position: refs/branch-heads/5735@{#1} Cr-Branched-From: df7df19-refs/heads/main@{#39949}

This reverts commit 8856410. Reason for revert: chromium:1447540 Original change's description: > pipewire capturer: Reduce the amount of copying > > Improves the capture latency by reducing the amount of > copying needed from the frame. We keep track of the > damaged region of previous frame and union it with > the damaged region of this frame and only copy this > union of the frame over. X11 capturer already has > such synchronization in place. > > The change is beneficial especially when there are > small changes on the screen (e.g. clock ticking). > For a 4k screen with 128 cores, I observed the > capture latencies drop from 5 - 8 ms to 0 ms when the > system is left idle. This is in line with the X11 > capturer. > > Bug: chromium:1291247 > Change-Id: Iffb441f9e1902d2658031f5f35b5372ee8e94073 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299720 > Reviewed-by: Alexander Cooper <alcooper@chromium.org> > Commit-Queue: Salman Malik <salmanmalik@chromium.org> > Cr-Commit-Position: refs/heads/main@{#39968} (cherry picked from commit a7d1081) Bug: chromium:1447540 Change-Id: Id1bfd3fc39fea2bb1f232cad5218f90e144920e7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/306263 Commit-Queue: Mark Foltz <mfoltz@chromium.org> Commit-Queue: Alexander Cooper <alcooper@chromium.org> Auto-Submit: Alexander Cooper <alcooper@chromium.org> Reviewed-by: Mark Foltz <mfoltz@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#40123} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/306582 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/branch-heads/5790@{#1} Cr-Branched-From: 2eacbbc-refs/heads/main@{#40122}

(cherry picked from commit eec1810) Bug: chromium:1454086 Change-Id: I39573b706c4031d091c45a182b13cb3b2dba6233 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/309920 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#40332} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/310920 Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Cr-Commit-Position: refs/branch-heads/5845@{#1} Cr-Branched-From: f80cf81-refs/heads/main@{#40319}

Instead, use BlockingCall to match with how unregistration is done. This is needed because the ThreadWrapper implementation in Chromium, overriding the Thread implementation in WebRTC, does not order sent (blocking) tasks along with posted tasks. That makes the functional difference that Thread1 posting and sending tasks to Thread2, can not assume that the tasks run in the order they were posted and sent. I.e. in this case: // Running on Thread1. thread2->PostTask([](){ Foo(); }); thread2->BlockingCall([](){ Bar(); }); Thread2 may actually execute Bar() first, and then Foo(). (cherry picked from commit 70cea9b) No-Try: true Bug: chromium:1470992 Change-Id: I1f83f12ce39c09279c0f2b3bc71c3a33e2cb16c5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317700 Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#40624} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/318360 Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> Cr-Commit-Position: refs/branch-heads/5938@{#1} Cr-Branched-From: 82e5f91-refs/heads/main@{#40524}

The FrameCadenceAdapter starts a delayed task to request a new refresh frame on receiving frame drop. However, the resulting RepeatingTaskHandle was not Stop()ed on destruction, leading to UAF. (cherry picked from commit fb98b01) Fixed: chromium:1478944 Change-Id: Iba441420953e989cfc7fcfd2f358b5b30f375786 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320200 Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#40747} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/320420 Reviewed-by: Henrik Andreassson <henrika@webrtc.org> Cr-Commit-Position: refs/branch-heads/5993@{#1} Cr-Branched-From: 5afcec0-refs/heads/main@{#40703}

…h vp8" This is a reland of commit 0d4b350 Patchset 1 is the original CL. Patchset 2 contains a small tweak of the target bitrate in the unit test, in order to make in less susceptible to flakiness on runtime environments running a slightly different libvpx. Original change's description: > Add mitigation for very long frame drop gaps with vp8 > > Bug: webrtc:15530 > Change-Id: I11f5e3f31f71301700dbff3fc9285236160bee45 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322320 > Reviewed-by: Sergey Silkin <ssilkin@webrtc.org> > Commit-Queue: Sergey Silkin <ssilkin@webrtc.org> > Auto-Submit: Erik Språng <sprang@webrtc.org> > Cr-Commit-Position: refs/heads/main@{#40866} (cherry picked from commit d7703d9) Bug: webrtc:15530 Change-Id: I096b7d952286f7f53852d1ca70aea398b2747784 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322540 Auto-Submit: Erik Språng <sprang@webrtc.org> Commit-Queue: Erik Språng <sprang@webrtc.org> Reviewed-by: Sergey Silkin <ssilkin@webrtc.org> Commit-Queue: Sergey Silkin <ssilkin@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#40874} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/322900 Cr-Commit-Position: refs/branch-heads/6045@{#1} Cr-Branched-From: bce7ce7-refs/heads/main@{#40854}

@30fps

This CL is a follow-up of work done in https://webrtc-review.googlesource.com/c/src/+/323882 where the goal was to reduce the amount of FrameDropped error logs in WebRTC.DesktopCapture.Win.WgcCaptureSessionGetFrameResult. The previous work avoids FrameDropped logs for a minimized window being captured with WGC but we still se a large amount of these error (or rather warning) logs. See [1] which comes from Canary. This CL does two different things to improve the situation: 1) It adds kFramePoolEmpty to the existing GetFrameResult::kFrameDropped enum to point out that the warning comes from the frame pool not being able to return a valid new frame. It also makes it more clear that it does not cause an outer/final error as WgcCapturerResult::kFrameDropped. We still keep the inner GetFrameResult::kFrameDropped but it is only produced when the frame pool returns NULL and our external queue is empty. Hence, a real frame-drop error. Note that, it is still easy to provoke kFramePoolEmpty simply by asking for a high resolution at a high rate. The example in [2] comes from a 4K screen @30fps. Hence, we have not fixed anything yet. 2) It also increases the size of the internal frame pool from 1 to 2. This does lead to an almost zero rate of kFramePoolEmpt warnings at the expense of a slightly reduced max capture rate. BUT, with 1 as size, we can "see" a higher max capture rate but it is not a true rate since it comes with a high rate of kFramePoolEmpty errors. Hence, we "emulate" a high capture rate by simply feeding copies of the last frame that we had stored in the external queue. Using 2 leads to a more "true" rate of what we actually can capture in terms of *new* frames and also a substantially lower rate of kFramePoolEmpty. In addition, with 1 as size, if we ask at a too high rate and provide a copy of the last frame, our CPU adaptation will not reduce its rate since we think that things are OK when it is actually not. Also, the samples in [3] and [4] both use 2 as numberOfBuffers as well. Let me also mention that with this small change, I a have not been able to provoke any kFramePoolEmpty error messages. Finally, geDisplayMedia can be called called with constraints where min and max framerate is defined. The mechanism which maintains the min rate is implemented via the RequestRefreshFrame API and it can be sent to the source (DesktopCaptureDevice) back to back with a previous timer interrupt for a capture request. Without this change, these RRFs were also a source of a large amount of kFramePoolEmpty error logs. With 2 buffers instead; this is no longer the case. [1] https://screenshot.googleplex.com/7sfv6HdGXLwyxdj [2] https://paste.googleplex.com/4795680001359872 [3] https://github.com/robmikh/Win32CaptureSample/blob/master/Win32CaptureSample/SimpleCapture.cpp [4] https://learn.microsoft.com/en-us/windows/uwp/audio-video-camera/screen-capture#add-the-screen-capture-capability (cherry picked from commit 4be5927) Bug: chromium:1314868 Change-Id: I73b823b31a993fd2cd6e007b212826dfe1a80012 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325521 Commit-Queue: Alexander Cooper <alcooper@chromium.org> Reviewed-by: Alexander Cooper <alcooper@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#41079} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/326960 Commit-Queue: Henrik Andreassson <henrika@webrtc.org> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> Reviewed-by: Stefan Holmer <stefan@webrtc.org> Cr-Commit-Position: refs/branch-heads/6099@{#1} Cr-Branched-From: 507f1cc-refs/heads/main@{#41042}

… CSRCs (cherry picked from commit 5f3ac43) Bug: chromium:1508337 Change-Id: I9f28fc0958d28bc97f9378a46fbec3e45148736f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330260 Reviewed-by: Guido Urdaneta <guidou@webrtc.org> Commit-Queue: Tony Herre <herre@google.com> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#41337} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/330564 Commit-Queue: Guido Urdaneta <guidou@webrtc.org> Cr-Commit-Position: refs/branch-heads/6167@{#1} Cr-Branched-From: ece5cb8-refs/heads/main@{#41315}

If SVC is used, the minimum bitrate would be 30kbps, instead of 49, as configured in svc_config.h, because the overall stream will get min_bitrate from the default VP8 simulcast configuration, and this 30kbps will be allocated to the stream for svc_rate_allocator to divide between layers. However, with the configuration before this change, 49kbps would be always allocated to the lowest simulcast stream. (cherry picked from commit f49a826) Bug: webrtc:15852, chromium:330672089 Change-Id: I1c77f45654af8850180a83f8e3f4428cc42d086e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343760 Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org> Reviewed-by: Sergey Silkin <ssilkin@webrtc.org> Cr-Original-Commit-Position: refs/heads/main@{#41940} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343981 Cr-Commit-Position: refs/branch-heads/6367@{#1} Cr-Branched-From: 802552a-refs/heads/main@{#41921}

blueboxd closed this as completed Nov 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ScreenCapturerMac::RegisterRefreshAndMoveHandlers crashes on 10.7 #1

ScreenCapturerMac::RegisterRefreshAndMoveHandlers crashes on 10.7 #1

blueboxd commented Nov 1, 2022

blueboxd commented Nov 3, 2022

ScreenCapturerMac::RegisterRefreshAndMoveHandlers crashes on 10.7 #1

ScreenCapturerMac::RegisterRefreshAndMoveHandlers crashes on 10.7 #1

Comments

blueboxd commented Nov 1, 2022

blueboxd commented Nov 3, 2022