Commit c7ed71be authored by Brian White's avatar Brian White Committed by Commit Bot

Revert "Migrate blink::MediaStreamVideoSource to WTF::Bind"

This reverts commit 81046e57.

Reason for revert: 

Mac ASAN tests are failing:
https://ci.chromium.org/p/chromium/builders/ci/Mac%20ASan%2064%20Tests%20%281%29?limit=200

This seems the most likely culprit between the last good and when the (very frequent) failures began.

https://chromium.googlesource.com/chromium/src.git/+log/17f09d5d92fc142fae6863b6c0d8bae17355ab95..8dde0a59b1362fc7a19186ca686cd5c6960003f5

[33545:775:0516/215851.235197:ERROR:vt_video_encode_accelerator_mac.cc(517)]  VTCompressionSessionCreate failed: -12908
[90mtracing_service_impl.cc:[0m [39mConfigured tracing, #sources:2, duration:0 ms, #buffers:1, total buffer size:12 KB, total sessions:1[0m
[90mshared_memory_arbiter_im[0m [31mShared memory buffer overrun! Stalling[0m
...


Original change's description:
> Migrate blink::MediaStreamVideoSource to WTF::Bind
> 
> This CL is the 3rd out of n CLs to convert modules/mediastream/
> away from base::Bind{Once,Repeating} to their respective Blink/WTF
> counterparts.
> 
> In this particular CL, a weak reference of MediaStreamVideoSource is
> passed to VideoTrackAdapter ctor to avoid double-bind methods with
> CrossThreadBind -> ConvertToBaseCallback -> CrossThreadBind.
> 
> R=​dgozman@chromium.org, guidou@chromium.org
> CC=​​​blink-reviews-vendor@chromium.org
> 
> BUG=923394
> 
> Change-Id: Ibd75c5fef80bef1a4328c4abb007fb07efbcc095
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1594114
> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#660725}

TBR=haraken@chromium.org,hiroshige@chromium.org,guidou@chromium.org,tonikitoo@igalia.com,tzik@chromium.org

Change-Id: I082573534da71adbb7b56a70d3b34ccd15272ba2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 923394
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1616857Reviewed-by: default avatarBrian White <bcwhite@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660986}
parent 4602841f
......@@ -16,13 +16,11 @@
#include "base/macros.h"
#include "base/stl_util.h"
#include "base/strings/string_number_conversions.h"
#include "base/threading/sequenced_task_runner_handle.h"
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/public/web/modules/mediastream/media_stream_constraints_util_video_device.h"
#include "third_party/blink/public/web/modules/mediastream/media_stream_video_track.h"
#include "third_party/blink/renderer/modules/mediastream/video_track_adapter.h"
#include "third_party/blink/renderer/platform/cross_thread_functional.h"
#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
#include "third_party/blink/renderer/platform/scheduler/public/thread.h"
namespace blink {
......@@ -38,7 +36,9 @@ MediaStreamVideoSource* MediaStreamVideoSource::GetVideoSource(
MediaStreamVideoSource::MediaStreamVideoSource()
: state_(NEW), weak_factory_(this) {
track_adapter_ = base::MakeRefCounted<VideoTrackAdapter>(
Platform::Current()->GetIOTaskRunner(), GetWeakPtr());
Platform::Current()->GetIOTaskRunner(),
base::BindRepeating(&MediaStreamVideoSource::OnFrameDropped,
weak_factory_.GetWeakPtr()));
}
MediaStreamVideoSource::~MediaStreamVideoSource() {
......@@ -65,8 +65,8 @@ void MediaStreamVideoSource::AddTrack(
switch (state_) {
case NEW: {
state_ = STARTING;
StartSourceImpl(ConvertToBaseCallback(CrossThreadBind(
&VideoTrackAdapter::DeliverFrameOnIO, track_adapter_)));
StartSourceImpl(
base::Bind(&VideoTrackAdapter::DeliverFrameOnIO, track_adapter_));
break;
}
case STARTING:
......@@ -128,9 +128,9 @@ void MediaStreamVideoSource::RemoveTrack(MediaStreamVideoTrack* video_track,
// stopping a source with StopSource() can have side effects that affect
// sources created after that StopSource() call, but before the actual
// stop takes place. See https://crbug.com/778039.
StopForRestart(WTF::Bind(&MediaStreamVideoSource::DidStopSource,
weak_factory_.GetWeakPtr(),
std::move(callback)));
StopForRestart(base::BindOnce(&MediaStreamVideoSource::DidStopSource,
weak_factory_.GetWeakPtr(),
std::move(callback)));
if (state_ == STOPPING_FOR_RESTART || state_ == STOPPED_FOR_RESTART) {
// If the source supports restarting, it is necessary to call
// FinalizeStopSource() to ensure the same behavior as StopSource(),
......@@ -186,9 +186,9 @@ void MediaStreamVideoSource::ReconfigureTrack(
void MediaStreamVideoSource::StopForRestart(RestartCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (state_ != STARTED) {
Thread::Current()->GetTaskRunner()->PostTask(
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
WTF::Bind(std::move(callback), RestartResult::INVALID_STATE));
base::BindOnce(std::move(callback), RestartResult::INVALID_STATE));
return;
}
......@@ -223,8 +223,8 @@ void MediaStreamVideoSource::OnStopForRestartDone(bool did_stop_for_restart) {
RestartResult result = did_stop_for_restart ? RestartResult::IS_STOPPED
: RestartResult::IS_RUNNING;
Thread::Current()->GetTaskRunner()->PostTask(
FROM_HERE, WTF::Bind(std::move(restart_callback_), result));
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(restart_callback_), result));
}
void MediaStreamVideoSource::Restart(
......@@ -232,9 +232,9 @@ void MediaStreamVideoSource::Restart(
RestartCallback callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
if (state_ != STOPPED_FOR_RESTART) {
Thread::Current()->GetTaskRunner()->PostTask(
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
WTF::Bind(std::move(callback), RestartResult::INVALID_STATE));
base::BindOnce(std::move(callback), RestartResult::INVALID_STATE));
return;
}
DCHECK(!restart_callback_);
......@@ -264,8 +264,8 @@ void MediaStreamVideoSource::OnRestartDone(bool did_restart) {
RestartResult result =
did_restart ? RestartResult::IS_RUNNING : RestartResult::IS_STOPPED;
Thread::Current()->GetTaskRunner()->PostTask(
FROM_HERE, WTF::Bind(std::move(restart_callback_), result));
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(std::move(restart_callback_), result));
}
void MediaStreamVideoSource::UpdateHasConsumers(MediaStreamVideoTrack* track,
......@@ -400,8 +400,8 @@ void MediaStreamVideoSource::StartFrameMonitoring() {
track_adapter_->SetSourceFrameSize(current_format->frame_size);
}
track_adapter_->StartFrameMonitoring(
frame_rate, WTF::BindRepeating(&MediaStreamVideoSource::SetMutedState,
weak_factory_.GetWeakPtr()));
frame_rate, base::Bind(&MediaStreamVideoSource::SetMutedState,
weak_factory_.GetWeakPtr()));
}
void MediaStreamVideoSource::SetReadyState(
......
......@@ -149,7 +149,8 @@ class VideoTrackAdapter::VideoFrameResolutionAdapter
VideoFrameResolutionAdapter(
scoped_refptr<base::SingleThreadTaskRunner> render_message_loop,
const VideoTrackAdapterSettings& settings,
base::WeakPtr<MediaStreamVideoSource> media_stream_video_source);
base::RepeatingCallback<void(media::VideoCaptureFrameDropReason)>
frame_dropped_cb);
// Add |frame_callback| to receive video frames on the IO-thread and
// |settings_callback| to set track settings on the main thread.
......@@ -215,7 +216,8 @@ class VideoTrackAdapter::VideoFrameResolutionAdapter
// registered in AddCallbacks.
const scoped_refptr<base::SingleThreadTaskRunner> renderer_task_runner_;
base::WeakPtr<MediaStreamVideoSource> media_stream_video_source_;
base::RepeatingCallback<void(media::VideoCaptureFrameDropReason)>
frame_dropped_cb_;
VideoTrackAdapterSettings settings_;
double frame_rate_;
......@@ -233,9 +235,10 @@ class VideoTrackAdapter::VideoFrameResolutionAdapter
VideoTrackAdapter::VideoFrameResolutionAdapter::VideoFrameResolutionAdapter(
scoped_refptr<base::SingleThreadTaskRunner> render_message_loop,
const VideoTrackAdapterSettings& settings,
base::WeakPtr<MediaStreamVideoSource> media_stream_video_source)
base::RepeatingCallback<void(media::VideoCaptureFrameDropReason)>
frame_dropped_cb)
: renderer_task_runner_(render_message_loop),
media_stream_video_source_(media_stream_video_source),
frame_dropped_cb_(std::move(frame_dropped_cb)),
settings_(settings),
frame_rate_(MediaStreamVideoSource::kDefaultFrameRate),
last_time_stamp_(base::TimeDelta::Max()),
......@@ -485,18 +488,17 @@ void VideoTrackAdapter::VideoFrameResolutionAdapter::ResetFrameRate() {
void VideoTrackAdapter::VideoFrameResolutionAdapter::
PostFrameDroppedToMainTaskRunner(
media::VideoCaptureFrameDropReason reason) {
PostCrossThreadTask(
*renderer_task_runner_, FROM_HERE,
CrossThreadBindOnce(&MediaStreamVideoSource::OnFrameDropped,
media_stream_video_source_, reason));
PostCrossThreadTask(*renderer_task_runner_, FROM_HERE,
CrossThreadBind(frame_dropped_cb_, reason));
}
VideoTrackAdapter::VideoTrackAdapter(
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
base::WeakPtr<MediaStreamVideoSource> media_stream_video_source)
base::RepeatingCallback<void(media::VideoCaptureFrameDropReason)>
frame_dropped_cb)
: io_task_runner_(io_task_runner),
media_stream_video_source_(media_stream_video_source),
renderer_task_runner_(base::ThreadTaskRunnerHandle::Get()),
frame_dropped_cb_(std::move(frame_dropped_cb)),
monitoring_frame_rate_(false),
muted_state_(false),
frame_counter_(0),
......@@ -541,7 +543,7 @@ void VideoTrackAdapter::AddTrackOnIO(
}
if (!adapter.get()) {
adapter = base::MakeRefCounted<VideoFrameResolutionAdapter>(
renderer_task_runner_, settings, media_stream_video_source_);
renderer_task_runner_, settings, frame_dropped_cb_);
adapters_.push_back(adapter);
}
......@@ -752,10 +754,9 @@ void VideoTrackAdapter::DeliverFrameOnIO(
if (adapters_.IsEmpty()) {
PostCrossThreadTask(
*renderer_task_runner_, FROM_HERE,
CrossThreadBindOnce(&MediaStreamVideoSource::OnFrameDropped,
media_stream_video_source_,
media::VideoCaptureFrameDropReason::
kVideoTrackAdapterHasNoResolutionAdapters));
CrossThreadBind(frame_dropped_cb_,
media::VideoCaptureFrameDropReason::
kVideoTrackAdapterHasNoResolutionAdapters));
}
for (const auto& adapter : adapters_)
adapter->DeliverFrame(frame, estimated_capture_time, is_device_rotated);
......
......@@ -9,7 +9,6 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/single_thread_task_runner.h"
#include "base/time/time.h"
......@@ -41,7 +40,8 @@ class BLINK_EXPORT VideoTrackAdapter
VideoTrackAdapter(
scoped_refptr<base::SingleThreadTaskRunner> io_task_runner,
base::WeakPtr<MediaStreamVideoSource> media_stream_video_source);
base::RepeatingCallback<void(media::VideoCaptureFrameDropReason)>
frame_dropped_cb);
// Register |track| to receive video frames in |frame_callback| with
// a resolution within the boundaries of the arguments, and settings
......@@ -129,12 +129,13 @@ class BLINK_EXPORT VideoTrackAdapter
const scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
base::WeakPtr<MediaStreamVideoSource> media_stream_video_source_;
// |renderer_task_runner_| is used to ensure that
// VideoCaptureDeliverFrameCB is released on the main render thread.
const scoped_refptr<base::SingleThreadTaskRunner> renderer_task_runner_;
const base::RepeatingCallback<void(media::VideoCaptureFrameDropReason)>
frame_dropped_cb_;
// VideoFrameResolutionAdapter is an inner class that lives on the IO-thread.
// It does the resolution adaptation and delivers frames to all registered
// tracks.
......
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