Commit 218b32cc authored by phoglund's avatar phoglund Committed by Commit bot

Revert of Removing silence trimming from WebRTC quality test. (patchset #5...

Revert of Removing silence trimming from WebRTC quality test. (patchset #5 id:80001 of https://codereview.chromium.org/890893004/)

Reason for revert:
This actually made the score worse by about 0.4 on all bots except one, and one bot is still not helped by this. I will try to solve this through other means, like forcing the volume to the right level on the bots where we get bad scores.

Original issue's description:
> Removing silence trimming from WebRTC quality test
>
> It turns out PESQ can deal with delays, so we don't need to
> silence-trim the files to get them to line up. This solves another
> problem where the silence-trimming becomes too aggressive on Windows
> with the new ref file.
>
> I also need to monitor if the new ref file, which is a lot lower
> in volume than the old one, unfairly punishes the score. The
> speaker volume is a bit random on the bots so the recording could
> end up being quite low. Tests on my workstation suggests that PESQ
> is actually pretty good about ignoring that too though. There will
> be a slight effect on the score (like 0.4 MOS), which I think is
> acceptable. Otherwise we need to find a way to control the speaker
> volume on the bots much more stringently.
>
> BUG=446859
>
> Committed: https://crrev.com/36d358e0cdff201a99501e9c986a8a26f8965a40
> Cr-Commit-Position: refs/heads/master@{#314797}

TBR=henrika@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=446859

Review URL: https://codereview.chromium.org/908703002

Cr-Commit-Position: refs/heads/master@{#315192}
parent 46b4983d
...@@ -54,10 +54,6 @@ static const char kWebRtcAudioTestHtmlPage[] = ...@@ -54,10 +54,6 @@ static const char kWebRtcAudioTestHtmlPage[] =
// Test we can set up a WebRTC call and play audio through it. // Test we can set up a WebRTC call and play audio through it.
// //
// WARNING: this test may mess with both output and input levels on your system
// and may cause audio to start blasting at full volume into your
// headphones.
//
// If you're not a googler and want to run this test, you need to provide a // If you're not a googler and want to run this test, you need to provide a
// pesq binary for your platform (and sox.exe on windows). Read more on how // pesq binary for your platform (and sox.exe on windows). Read more on how
// resources are managed in chrome/test/data/webrtc/resources/README. // resources are managed in chrome/test/data/webrtc/resources/README.
...@@ -110,7 +106,7 @@ static const char kWebRtcAudioTestHtmlPage[] = ...@@ -110,7 +106,7 @@ static const char kWebRtcAudioTestHtmlPage[] =
// 5. Launch chrome and try playing a video with sound. You should see // 5. Launch chrome and try playing a video with sound. You should see
// in the volume meter for the mix device. Configure the mix device to have // in the volume meter for the mix device. Configure the mix device to have
// 50 / 100 in level. Also go into the playback tab, right-click Speakers, // 50 / 100 in level. Also go into the playback tab, right-click Speakers,
// and set that level to 50 / 100. Otherwise you might get distortion in // and set that level to 50 / 100. Otherwise you will get distortion in
// the recording. // the recording.
class MAYBE_WebRtcAudioQualityBrowserTest : public WebRtcTestBase { class MAYBE_WebRtcAudioQualityBrowserTest : public WebRtcTestBase {
public: public:
...@@ -253,7 +249,7 @@ class AudioRecorder { ...@@ -253,7 +249,7 @@ class AudioRecorder {
base::Process recording_application_; base::Process recording_application_;
}; };
bool ForceMicrophoneLevelTo100Percent() { bool ForceMicrophoneVolumeTo100Percent() {
#if defined(OS_WIN) #if defined(OS_WIN)
// Note: the force binary isn't in tools since it's one of our own. // Note: the force binary isn't in tools since it's one of our own.
base::CommandLine command_line(test::GetReferenceFilesDir().Append( base::CommandLine command_line(test::GetReferenceFilesDir().Append(
...@@ -491,7 +487,6 @@ void SplitFileOnSilenceIntoDir(const base::FilePath& to_split, ...@@ -491,7 +487,6 @@ void SplitFileOnSilenceIntoDir(const base::FilePath& to_split,
// First trim beginning and end since they are tricky for the splitter. // First trim beginning and end since they are tricky for the splitter.
base::FilePath trimmed_audio = CreateTemporaryWaveFile(); base::FilePath trimmed_audio = CreateTemporaryWaveFile();
// TODO(phoglund): find smarter way to do this and get rid of RemoveSilence.
ASSERT_TRUE(RemoveSilence(to_split, trimmed_audio)); ASSERT_TRUE(RemoveSilence(to_split, trimmed_audio));
DVLOG(0) << "Trimmed silence: " << trimmed_audio.value() << std::endl; DVLOG(0) << "Trimmed silence: " << trimmed_audio.value() << std::endl;
...@@ -548,14 +543,24 @@ void AnalyzeSegmentsAndPrintResult( ...@@ -548,14 +543,24 @@ void AnalyzeSegmentsAndPrintResult(
void ComputeAndPrintPesqResults(const base::FilePath& reference_file, void ComputeAndPrintPesqResults(const base::FilePath& reference_file,
const base::FilePath& recording, const base::FilePath& recording,
const std::string& perf_modifier) { const std::string& perf_modifier) {
base::FilePath trimmed_reference = CreateTemporaryWaveFile();
base::FilePath trimmed_recording = CreateTemporaryWaveFile();
ASSERT_TRUE(RemoveSilence(reference_file, trimmed_reference));
ASSERT_TRUE(RemoveSilence(recording, trimmed_recording));
std::string raw_mos; std::string raw_mos;
std::string mos_lqo; std::string mos_lqo;
ASSERT_TRUE(RunPesq(reference_file, recording, 16000, &raw_mos, &mos_lqo)); ASSERT_TRUE(RunPesq(trimmed_reference, trimmed_recording, 16000,
&raw_mos, &mos_lqo));
perf_test::PrintResult( perf_test::PrintResult(
"audio_pesq", perf_modifier, "raw_mos", raw_mos, "score", true); "audio_pesq", perf_modifier, "raw_mos", raw_mos, "score", true);
perf_test::PrintResult( perf_test::PrintResult(
"audio_pesq", perf_modifier, "mos_lqo", mos_lqo, "score", true); "audio_pesq", perf_modifier, "mos_lqo", mos_lqo, "score", true);
EXPECT_TRUE(base::DeleteFile(trimmed_reference, false));
EXPECT_TRUE(base::DeleteFile(trimmed_recording, false));
} }
} // namespace } // namespace
...@@ -569,7 +574,8 @@ void ComputeAndPrintPesqResults(const base::FilePath& reference_file, ...@@ -569,7 +574,8 @@ void ComputeAndPrintPesqResults(const base::FilePath& reference_file,
// audio device is up following the getUserMedia call in the left tab. The time // audio device is up following the getUserMedia call in the left tab. The time
// it takes to negotiate a call isn't deterministic, but two seconds should be // it takes to negotiate a call isn't deterministic, but two seconds should be
// plenty of time. Similarly, the recording time should be enough to catch the // plenty of time. Similarly, the recording time should be enough to catch the
// whole reference file. // whole reference file. If you then silence-trim the reference file and actual
// file, you should end up with two time-synchronized files.
void MAYBE_WebRtcAudioQualityBrowserTest::SetupAndRecordAudioCall( void MAYBE_WebRtcAudioQualityBrowserTest::SetupAndRecordAudioCall(
const base::FilePath& reference_file, const base::FilePath& reference_file,
const base::FilePath& recording, const base::FilePath& recording,
...@@ -577,7 +583,7 @@ void MAYBE_WebRtcAudioQualityBrowserTest::SetupAndRecordAudioCall( ...@@ -577,7 +583,7 @@ void MAYBE_WebRtcAudioQualityBrowserTest::SetupAndRecordAudioCall(
const base::TimeDelta recording_time) { const base::TimeDelta recording_time) {
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
ASSERT_TRUE(test::HasReferenceFilesInCheckout()); ASSERT_TRUE(test::HasReferenceFilesInCheckout());
ASSERT_TRUE(ForceMicrophoneLevelTo100Percent()); ASSERT_TRUE(ForceMicrophoneVolumeTo100Percent());
ConfigureFakeDeviceToPlayFile(reference_file); ConfigureFakeDeviceToPlayFile(reference_file);
...@@ -625,8 +631,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcAudioQualityBrowserTest, ...@@ -625,8 +631,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcAudioQualityBrowserTest,
ASSERT_NO_FATAL_FAILURE(SetupAndRecordAudioCall( ASSERT_NO_FATAL_FAILURE(SetupAndRecordAudioCall(
reference_file, recording, kAudioOnlyCallConstraints, reference_file, recording, kAudioOnlyCallConstraints,
base::TimeDelta::FromSeconds(25))); base::TimeDelta::FromSeconds(25)));
ASSERT_NO_FATAL_FAILURE(ComputeAndPrintPesqResults( ComputeAndPrintPesqResults(reference_file, recording, "_getusermedia");
reference_file, recording, "_getusermedia"));
EXPECT_TRUE(base::DeleteFile(recording, false)); EXPECT_TRUE(base::DeleteFile(recording, false));
} }
...@@ -641,7 +646,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcAudioQualityBrowserTest, ...@@ -641,7 +646,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcAudioQualityBrowserTest,
ASSERT_TRUE(test::HasReferenceFilesInCheckout()); ASSERT_TRUE(test::HasReferenceFilesInCheckout());
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady()); ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
ASSERT_TRUE(ForceMicrophoneLevelTo100Percent()); ASSERT_TRUE(ForceMicrophoneVolumeTo100Percent());
content::WebContents* left_tab = content::WebContents* left_tab =
OpenPageWithoutGetUserMedia(kWebRtcAudioTestHtmlPage); OpenPageWithoutGetUserMedia(kWebRtcAudioTestHtmlPage);
...@@ -671,9 +676,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcAudioQualityBrowserTest, ...@@ -671,9 +676,7 @@ IN_PROC_BROWSER_TEST_F(MAYBE_WebRtcAudioQualityBrowserTest,
// through WebAudio earlier). // through WebAudio earlier).
base::FilePath reference_file = base::FilePath reference_file =
test::GetReferenceFilesDir().Append(kReferenceFile); test::GetReferenceFilesDir().Append(kReferenceFile);
ASSERT_NO_FATAL_FAILURE(ComputeAndPrintPesqResults( ComputeAndPrintPesqResults(reference_file, recording, "_webaudio");
reference_file, recording, "_webaudio"));
EXPECT_TRUE(base::DeleteFile(recording, false));
} }
/** /**
......
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