Skip to content

GitLab

T
tangled
Commit 0b4f2bdc authored by Kyle Milka's avatar Kyle Milka Committed by Commit Bot
Browse files
Options

Revert "Passing audio capture timestamp to WebRTC." 

This reverts commit 00134099.

Reason for revert: New test failing on several bots for use-of-uninitialized-value. see: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8886112714306274752/+/steps/blink_platform_unittests/0/stdout

TBR=waffles@chromium.org
Original change's description:
> Passing audio capture timestamp to WebRTC.
> 
> Bug: 1054403
> Change-Id: I6349687ce55535a0cdc405db67687d967a04fea1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2066777
> Commit-Queue: Minyue Li <minyue@chromium.org>
> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
> Reviewed-by: Guido Urdaneta <guidou@chromium.org>
> Reviewed-by: Henrik Andreasson <henrika@chromium.org>
> Reviewed-by: Yuri Wiitala <miu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#749126}

TBR=miu@chromium.org,wangxianzhu@chromium.org,minyue@chromium.org,guidou@chromium.org,olka@chromium.org,henrika@chromium.org,chxg@google.com

Change-Id: Iba7e0f72e06eba2f4b58a01c67f2e7c33f28cb80
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1054403
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2098402Reviewed-by: default avatarKyle Milka <kmilka@chromium.org>
Commit-Queue: Kyle Milka <kmilka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749206}
parent 7f69540a
Show whitespace changes
Inline Side-by-side
Showing with 4 additions and 179 deletions
third_party/blink/renderer/platform/BUILD.gn
View file @ 0b4f2bdc
......@@ -1886,7 +1886,6 @@ jumbo_source_set("blink_platform_unittests_sources") {
"peerconnection/task_queue_factory_test.cc",
"peerconnection/transmission_encoding_info_handler_test.cc",
"peerconnection/two_keys_adapter_map_unittest.cc",
"peerconnection/webrtc_audio_sink_test.cc",
"peerconnection/webrtc_video_track_source_test.cc",
"text/bidi_resolver_test.cc",
"text/bidi_test_harness.h",
......
third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink.cc
View file @ 0b4f2bdc
......@@ -12,7 +12,6 @@
#include "base/logging.h"
#include "base/stl_util.h"
#include "base/strings/stringprintf.h"
#include "media/base/audio_timestamp_helper.h"
#include "third_party/blink/public/platform/modules/webrtc/webrtc_logging.h"
#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
......@@ -99,16 +98,6 @@ void WebRtcAudioSink::OnData(const media::AudioBus& audio_bus,
base::TimeTicks estimated_capture_time) {
// No thread check: OnData might be called on different threads (but not
// concurrently).
// TODO(crbug.com/1054769): Better to let |fifo_| handle the estimated capture
// time and let it return a corrected interpolated capture time to
// DeliverRebufferedAudio(). Current, similar treatment is used at different
// places where |AudioPushFifo| is applied. So a update to |AudioPushFifo|
// will be a joint effort, and should be carefully carried out.
last_estimated_capture_time_ = estimated_capture_time;
adapter_->UpdateTimestampAligner(estimated_capture_time);
// The following will result in zero, one, or multiple synchronous calls to
// DeliverRebufferedAudio().
fifo_.Push(audio_bus);
......@@ -141,14 +130,9 @@ void WebRtcAudioSink::DeliverRebufferedAudio(const media::AudioBus& audio_bus,
"ToInterleaved expects 2 bytes.");
audio_bus.ToInterleaved<media::SignedInt16SampleTypeTraits>(
audio_bus.frames(), interleaved_data_.get());
const base::TimeTicks estimated_capture_time =
last_estimated_capture_time_ + media::AudioTimestampHelper::FramesToTime(
frame_delay, params_.sample_rate());
adapter_->DeliverPCMToWebRtcSinks(interleaved_data_.get(),
params_.sample_rate(), audio_bus.channels(),
audio_bus.frames(), estimated_capture_time);
audio_bus.frames());
}
namespace {
......@@ -187,20 +171,11 @@ void WebRtcAudioSink::Adapter::DeliverPCMToWebRtcSinks(
const int16_t* audio_data,
int sample_rate,
size_t number_of_channels,
size_t number_of_frames,
base::TimeTicks estimated_capture_time) {
size_t number_of_frames) {
base::AutoLock auto_lock(lock_);
// This use |timestamp_aligner_| to transform |estimated_capture_timestamp| to
// rtc::TimeMicros(). See the comment at UpdateTimestampAligner() for more
// details.
const int64_t capture_timestamp_us = timestamp_aligner_.TranslateTimestamp(
estimated_capture_time.since_origin().InMicroseconds());
for (webrtc::AudioTrackSinkInterface* sink : sinks_) {
sink->OnData(audio_data, sizeof(int16_t) * 8, sample_rate,
number_of_channels, number_of_frames,
capture_timestamp_us / rtc::kNumMicrosecsPerMillisec);
number_of_channels, number_of_frames);
}
}
......@@ -274,16 +249,4 @@ webrtc::AudioSourceInterface* WebRtcAudioSink::Adapter::GetSource() const {
return source_.get();
}
void WebRtcAudioSink::Adapter::UpdateTimestampAligner(
base::TimeTicks capture_time) {
// The |timestamp_aligner_| stamps an audio frame as if it is captured 'now',
// taking rtc::TimeMicros as the reference clock. It does not provide the time
// that the frame was originally captured, Using |timestamp_aligner_| rather
// than calling rtc::TimeMicros is to take the advantage that it aligns its
// output timestamps such that the time spacing in the |capture_time| is
// maintained.
timestamp_aligner_.TranslateTimestamp(
capture_time.since_origin().InMicroseconds(), rtc::TimeMicros());
}
} // namespace blink
third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink.h
View file @ 0b4f2bdc
......@@ -24,8 +24,6 @@
#include "third_party/blink/renderer/platform/platform_export.h"
#include "third_party/webrtc/api/media_stream_interface.h"
#include "third_party/webrtc/pc/media_stream_track.h"
#include "third_party/webrtc/rtc_base/time_utils.h"
#include "third_party/webrtc/rtc_base/timestamp_aligner.h"
namespace blink {
......@@ -102,8 +100,7 @@ class PLATFORM_EXPORT WebRtcAudioSink : public WebMediaStreamAudioSink {
void DeliverPCMToWebRtcSinks(const int16_t* audio_data,
int sample_rate,
size_t number_of_channels,
size_t number_of_frames,
base::TimeTicks estimated_capture_time);
size_t number_of_frames);
std::string label() const { return label_; }
......@@ -119,8 +116,6 @@ class PLATFORM_EXPORT WebRtcAudioSink : public WebMediaStreamAudioSink {
override;
webrtc::AudioSourceInterface* GetSource() const override;
void UpdateTimestampAligner(base::TimeTicks capture_time);
protected:
~Adapter() override;
......@@ -156,11 +151,6 @@ class PLATFORM_EXPORT WebRtcAudioSink : public WebMediaStreamAudioSink {
// receive the audio data.
std::vector<webrtc::AudioTrackSinkInterface*> sinks_;
// Used for getting capture timestamps referenced on the rtc::TimeMicros()
// clock. See the comment at the implementation of UpdateTimestampAligner()
// for more details.
rtc::TimestampAligner timestamp_aligner_;
DISALLOW_COPY_AND_ASSIGN(Adapter);
};
......@@ -192,8 +182,6 @@ class PLATFORM_EXPORT WebRtcAudioSink : public WebMediaStreamAudioSink {
// interleaved samples.
std::unique_ptr<int16_t[]> interleaved_data_;
base::TimeTicks last_estimated_capture_time_;
// In debug builds, check that WebRtcAudioSink's public methods are all being
// called on the main render thread.
THREAD_CHECKER(thread_checker_);
......
third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink_test.cc deleted 100644 → 0
View file @ 7f69540a
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/platform/peerconnection/webrtc_audio_sink.h"
#include "media/base/fake_single_thread_task_runner.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
using testing::_;
namespace blink {
namespace {
class MockAudioSink : public webrtc::AudioTrackSinkInterface {
public:
MockAudioSink() = default;
~MockAudioSink() override = default;
MOCK_METHOD6(OnData,
void(const void* audio_data,
int bits_per_sample,
int sample_rate,
size_t number_of_channels,
size_t number_of_samples,
absl::optional<int64_t> absolute_capture_timestamp_ms));
};
class ScopedFakeClock : public rtc::ClockInterface {
public:
explicit ScopedFakeClock(int64_t init_time_ms)
: prev_clock_(rtc::SetClockForTesting(this)),
time_ns_(init_time_ms * rtc::kNumNanosecsPerMillisec) {}
~ScopedFakeClock() override { rtc::SetClockForTesting(prev_clock_); }
int64_t TimeNanos() const override { return time_ns_; }
void AdvanceTimeMilliseconds(int64_t time_ms) {
time_ns_ += time_ms * rtc::kNumNanosecsPerMillisec;
}
private:
ClockInterface* const prev_clock_;
int64_t time_ns_;
};
} // namespace
TEST(WebRtcAudioSinkTest, CaptureTimestamp) {
MockAudioSink sink_1;
MockAudioSink sink_2;
base::SimpleTestTickClock dummy_clock;
std::unique_ptr<WebRtcAudioSink> webrtc_audio_sink(
new WebRtcAudioSink("test_sink", nullptr,
/*signaling_task_runner=*/
new media::FakeSingleThreadTaskRunner(&dummy_clock),
/*main_task_runner=*/
new media::FakeSingleThreadTaskRunner(&dummy_clock)));
// |web_media_stream_audio_sink| is to access methods that are privately
// inherited by WebRtcAudioSink.
WebMediaStreamAudioSink* const web_media_stream_audio_sink =
static_cast<WebMediaStreamAudioSink*>(webrtc_audio_sink.get());
webrtc_audio_sink->webrtc_audio_track()->AddSink(&sink_1);
webrtc_audio_sink->webrtc_audio_track()->AddSink(&sink_2);
constexpr int kInputChannels = 2;
constexpr int kInputFramesPerBuffer = 96;
constexpr int kSampleRateHz = 8000;
constexpr int kOutputFramesPerBuffer = kSampleRateHz / 100;
constexpr int kEnqueueFrames = kInputFramesPerBuffer - kOutputFramesPerBuffer;
constexpr int64_t kStartRtcTimestampMs = 87654321;
constexpr int64_t kStartCaptureTimestampMs = 12345678;
constexpr int64_t kCaptureIntervalMs = 567;
web_media_stream_audio_sink->OnSetFormat(media::AudioParameters(
media::AudioParameters::AUDIO_PCM_LINEAR, media::CHANNEL_LAYOUT_STEREO,
kSampleRateHz, kOutputFramesPerBuffer));
std::unique_ptr<media::AudioBus> bus =
media::AudioBus::Create(kInputChannels, kInputFramesPerBuffer);
{
ScopedFakeClock clock(kStartRtcTimestampMs);
base::TimeTicks capture_time =
base::TimeTicks() +
base::TimeDelta::FromMilliseconds(kStartCaptureTimestampMs);
// The first time to the call OnData(), the TimestampAligner should have no
// effect work. So expected capture timestamp is from fake_clock.
EXPECT_CALL(
sink_1,
OnData(_, _, kSampleRateHz, kInputChannels, kOutputFramesPerBuffer,
absl::make_optional<int64_t>(kStartRtcTimestampMs)));
EXPECT_CALL(
sink_2,
OnData(_, _, kSampleRateHz, kInputChannels, kOutputFramesPerBuffer,
absl::make_optional<int64_t>(kStartRtcTimestampMs)));
web_media_stream_audio_sink->OnData(*bus, capture_time);
capture_time += base::TimeDelta::FromMilliseconds(kCaptureIntervalMs);
clock.AdvanceTimeMilliseconds(kCaptureIntervalMs);
constexpr int64_t kExpectedTimestampMs =
kStartRtcTimestampMs + kCaptureIntervalMs -
kEnqueueFrames * 1000 / kSampleRateHz;
EXPECT_CALL(
sink_1,
OnData(_, _, kSampleRateHz, kInputChannels, kOutputFramesPerBuffer,
absl::make_optional<int64_t>(kExpectedTimestampMs)));
EXPECT_CALL(
sink_2,
OnData(_, _, kSampleRateHz, kInputChannels, kOutputFramesPerBuffer,
absl::make_optional<int64_t>(kExpectedTimestampMs)));
web_media_stream_audio_sink->OnData(*bus, capture_time);
}
}
} // namespace blink
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