[PCLF] Cleanup old video dumping API
Bug: b/240540206
Change-Id: I1184f3f73a6de430e7103783b8959d8ff222e31e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/270485
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38163}
diff --git a/api/test/peerconnection_quality_test_fixture.h b/api/test/peerconnection_quality_test_fixture.h
index b60c3b4..028848b 100644
--- a/api/test/peerconnection_quality_test_fixture.h
+++ b/api/test/peerconnection_quality_test_fixture.h
@@ -320,54 +320,11 @@
// each RtpEncodingParameters of RtpParameters of corresponding
// RtpSenderInterface for this video stream.
absl::optional<int> temporal_layers_count;
- // DEPRECATED: use input_dump_options instead. If specified the input stream
- // will be also copied to specified file. It is actually one of the test's
- // output file, which contains copy of what was captured during the test for
- // this video stream on sender side. It is useful when generator is used as
- // input.
- absl::optional<std::string> input_dump_file_name;
- // DEPRECATED: use input_dump_options instead. Used only if
- // `input_dump_file_name` is set. Specifies the module for the video frames
- // to be dumped. Modulo equals X means every Xth frame will be written to
- // the dump file. The value must be greater than 0.
- int input_dump_sampling_modulo = 1;
// If specified defines how input should be dumped. It is actually one of
// the test's output file, which contains copy of what was captured during
// the test for this video stream on sender side. It is useful when
// generator is used as input.
absl::optional<VideoDumpOptions> input_dump_options;
- // DEPRECATED: use output_dump_options instead. If specified this file will
- // be used as output on the receiver side for this stream.
- //
- // If multiple output streams will be produced by this stream (e.g. when the
- // stream represented by this `VideoConfig` is received by more than one
- // peer), output files will be appended with receiver names. If the second
- // and other receivers will be added in the middle of the call after the
- // first frame for this stream has been already written to the output file,
- // then only dumps for newly added peers will be appended with receiver
- // name, the dump for the first receiver will have name equal to the
- // specified one. For example:
- // * If we have peers A and B and A has `VideoConfig` V_a with
- // V_a.output_dump_file_name = "/foo/a_output.yuv", then the stream
- // related to V_a will be written into "/foo/a_output.yuv".
- // * If we have peers A, B and C and A has `VideoConfig` V_a with
- // V_a.output_dump_file_name = "/foo/a_output.yuv", then the stream
- // related to V_a will be written for peer B into "/foo/a_output.yuv.B"
- // and for peer C into "/foo/a_output.yuv.C"
- // * If we have peers A and B and A has `VideoConfig` V_a with
- // V_a.output_dump_file_name = "/foo/a_output.yuv", then if after B
- // received the first frame related to V_a peer C joined the call, then
- // the stream related to V_a will be written for peer B into
- // "/foo/a_output.yuv" and for peer C into "/foo/a_output.yuv.C"
- //
- // The produced files contains what was rendered for this video stream on
- // receiver side.
- absl::optional<std::string> output_dump_file_name;
- // DEPRECATED: use output_dump_options instead. Used only if
- // `output_dump_file_name` is set. Specifies the module for the video frames
- // to be dumped. Modulo equals X means every Xth frame will be written to
- // the dump file. The value must be greater than 0.
- int output_dump_sampling_modulo = 1;
// If specified defines how output should be dumped on the receiver side for
// this stream. The produced files contain what was rendered for this video
// stream on receiver side per each receiver.
diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc
index 661208e..8cc64fd 100644
--- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc
+++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.cc
@@ -28,30 +28,8 @@
namespace webrtc {
namespace webrtc_pc_e2e {
-
namespace {
-class VideoWriter final : public rtc::VideoSinkInterface<VideoFrame> {
- public:
- VideoWriter(test::VideoFrameWriter* video_writer, int sampling_modulo)
- : video_writer_(video_writer), sampling_modulo_(sampling_modulo) {}
- ~VideoWriter() override = default;
-
- void OnFrame(const VideoFrame& frame) override {
- if (frames_counter_++ % sampling_modulo_ != 0) {
- return;
- }
- bool result = video_writer_->WriteFrame(frame);
- RTC_CHECK(result) << "Failed to write frame";
- }
-
- private:
- test::VideoFrameWriter* const video_writer_;
- const int sampling_modulo_;
-
- int64_t frames_counter_ = 0;
-};
-
class AnalyzingFramePreprocessor
: public test::TestVideoCapturer::FramePreprocessor {
public:
@@ -109,7 +87,7 @@
<< "Failed to write frame id to the output file: " << file_name_;
}
-VideoQualityAnalyzerInjectionHelper::VideoWriter2::VideoWriter2(
+VideoQualityAnalyzerInjectionHelper::VideoWriter::VideoWriter(
test::VideoFrameWriter* video_writer,
VideoFrameIdsWriter* frame_ids_writer,
int sampling_modulo)
@@ -117,7 +95,7 @@
frame_ids_writer_(frame_ids_writer),
sampling_modulo_(sampling_modulo) {}
-void VideoQualityAnalyzerInjectionHelper::VideoWriter2::OnFrame(
+void VideoQualityAnalyzerInjectionHelper::VideoWriter::OnFrame(
const VideoFrame& frame) {
if (frames_counter_++ % sampling_modulo_ != 0) {
return;
@@ -172,28 +150,15 @@
std::vector<std::unique_ptr<rtc::VideoSinkInterface<VideoFrame>>> sinks;
test::VideoFrameWriter* writer = nullptr;
if (config.input_dump_options.has_value()) {
- // Using new API for video dumping.
- writer = MaybeCreateVideoWriter(
+ writer = CreateVideoWriter(
config.input_dump_options->GetInputDumpFileName(*config.stream_label),
config);
- RTC_CHECK(writer);
- VideoFrameIdsWriter* frame_ids_writer = nullptr;
- if (config.input_dump_options->export_frame_ids()) {
- frame_ids_writers_.push_back(std::make_unique<VideoFrameIdsWriter>(
- *config.input_dump_options->GetInputFrameIdsDumpFileName(
- *config.stream_label)));
- frame_ids_writer = frame_ids_writers_.back().get();
- }
- sinks.push_back(std::make_unique<VideoWriter2>(
+ VideoFrameIdsWriter* frame_ids_writer = MaybeCreateVideoFrameIdsWriter(
+ config.input_dump_options->GetInputFrameIdsDumpFileName(
+ *config.stream_label));
+ sinks.push_back(std::make_unique<VideoWriter>(
writer, frame_ids_writer,
config.input_dump_options->sampling_modulo()));
- } else {
- // Using old API. To be removed.
- writer = MaybeCreateVideoWriter(config.input_dump_file_name, config);
- if (writer) {
- sinks.push_back(std::make_unique<VideoWriter>(
- writer, config.input_dump_sampling_modulo));
- }
}
if (config.show_on_screen) {
sinks.push_back(absl::WrapUnique(
@@ -256,19 +221,15 @@
frame_ids_writers_.clear();
}
-test::VideoFrameWriter*
-VideoQualityAnalyzerInjectionHelper::MaybeCreateVideoWriter(
- absl::optional<std::string> file_name,
+test::VideoFrameWriter* VideoQualityAnalyzerInjectionHelper::CreateVideoWriter(
+ absl::string_view file_name,
const PeerConnectionE2EQualityTestFixture::VideoConfig& config) {
- if (!file_name.has_value()) {
- return nullptr;
- }
// TODO(titovartem) create only one file writer for simulcast video track.
// For now this code will be invoked for each simulcast stream separately, but
// only one file will be used.
std::unique_ptr<test::VideoFrameWriter> video_writer =
std::make_unique<test::Y4mVideoFrameWriterImpl>(
- *file_name, config.width, config.height, config.fps);
+ std::string(file_name), config.width, config.height, config.fps);
if (config.output_dump_use_fixed_framerate) {
video_writer = std::make_unique<test::FixedFpsVideoFrameWriterAdapter>(
config.fps, clock_, std::move(video_writer));
@@ -278,6 +239,17 @@
return out;
}
+VideoQualityAnalyzerInjectionHelper::VideoFrameIdsWriter*
+VideoQualityAnalyzerInjectionHelper::MaybeCreateVideoFrameIdsWriter(
+ absl::optional<std::string> frame_ids_dump_file_name) {
+ if (!frame_ids_dump_file_name.has_value()) {
+ return nullptr;
+ }
+ frame_ids_writers_.push_back(
+ std::make_unique<VideoFrameIdsWriter>(*frame_ids_dump_file_name));
+ return frame_ids_writers_.back().get();
+}
+
void VideoQualityAnalyzerInjectionHelper::OnFrame(absl::string_view peer_name,
const VideoFrame& frame) {
rtc::scoped_refptr<I420BufferInterface> i420_buffer =
@@ -321,37 +293,16 @@
std::vector<std::unique_ptr<rtc::VideoSinkInterface<VideoFrame>>> sinks;
test::VideoFrameWriter* writer = nullptr;
if (config.output_dump_options.has_value()) {
- // Using new API with output directory.
- writer = MaybeCreateVideoWriter(
+ writer = CreateVideoWriter(
config.output_dump_options->GetOutputDumpFileName(
receiver_stream.stream_label, receiver_stream.peer_name),
config);
- RTC_CHECK(writer);
- VideoFrameIdsWriter* frame_ids_writer = nullptr;
- if (config.output_dump_options->export_frame_ids()) {
- frame_ids_writers_.push_back(std::make_unique<VideoFrameIdsWriter>(
- *config.output_dump_options->GetOutputFrameIdsDumpFileName(
- receiver_stream.stream_label, receiver_stream.peer_name)));
- frame_ids_writer = frame_ids_writers_.back().get();
- }
- sinks.push_back(std::make_unique<VideoWriter2>(
+ VideoFrameIdsWriter* frame_ids_writer = MaybeCreateVideoFrameIdsWriter(
+ config.output_dump_options->GetOutputFrameIdsDumpFileName(
+ receiver_stream.stream_label, receiver_stream.peer_name));
+ sinks.push_back(std::make_unique<VideoWriter>(
writer, frame_ids_writer,
config.output_dump_options->sampling_modulo()));
- } else {
- // Using old API. To be removed.
- absl::optional<std::string> output_dump_file_name =
- config.output_dump_file_name;
- if (output_dump_file_name.has_value() && peers_count_ > 2) {
- // TODO(titovartem): make this default behavior for any amount of peers.
- rtc::StringBuilder builder(*output_dump_file_name);
- builder << "." << receiver_stream.peer_name;
- output_dump_file_name = builder.str();
- }
- writer = MaybeCreateVideoWriter(output_dump_file_name, config);
- if (writer) {
- sinks.push_back(std::make_unique<VideoWriter>(
- writer, config.output_dump_sampling_modulo));
- }
}
if (config.show_on_screen) {
sinks.push_back(absl::WrapUnique(
diff --git a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h
index 76b8b44..9108488 100644
--- a/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h
+++ b/test/pc/e2e/analyzer/video/video_quality_analyzer_injection_helper.h
@@ -145,12 +145,12 @@
FILE* output_file_;
};
- class VideoWriter2 final : public rtc::VideoSinkInterface<VideoFrame> {
+ class VideoWriter final : public rtc::VideoSinkInterface<VideoFrame> {
public:
- VideoWriter2(test::VideoFrameWriter* video_writer,
- VideoFrameIdsWriter* frame_ids_writer,
- int sampling_modulo);
- ~VideoWriter2() override = default;
+ VideoWriter(test::VideoFrameWriter* video_writer,
+ VideoFrameIdsWriter* frame_ids_writer,
+ int sampling_modulo);
+ ~VideoWriter() override = default;
void OnFrame(const VideoFrame& frame) override;
@@ -162,9 +162,11 @@
int64_t frames_counter_ = 0;
};
- test::VideoFrameWriter* MaybeCreateVideoWriter(
- absl::optional<std::string> file_name,
+ test::VideoFrameWriter* CreateVideoWriter(
+ absl::string_view file_name,
const PeerConnectionE2EQualityTestFixture::VideoConfig& config);
+ VideoFrameIdsWriter* MaybeCreateVideoFrameIdsWriter(
+ absl::optional<std::string> frame_ids_dump_file_name);
// Creates a deep copy of the frame and passes it to the video analyzer, while
// passing real frame to the sinks
void OnFrame(absl::string_view peer_name, const VideoFrame& frame);
diff --git a/test/pc/e2e/peer_configurer.cc b/test/pc/e2e/peer_configurer.cc
index 998f789..4571d35 100644
--- a/test/pc/e2e/peer_configurer.cc
+++ b/test/pc/e2e/peer_configurer.cc
@@ -109,15 +109,6 @@
RTC_CHECK(inserted) << "Duplicate video_config.stream_label="
<< video_config.stream_label.value();
- if (video_config.input_dump_file_name.has_value()) {
- RTC_CHECK_GT(video_config.input_dump_sampling_modulo, 0)
- << "video_config.input_dump_sampling_modulo must be greater than 0";
- }
- if (video_config.output_dump_file_name.has_value()) {
- RTC_CHECK_GT(video_config.output_dump_sampling_modulo, 0)
- << "video_config.input_dump_sampling_modulo must be greater than 0";
- }
-
// TODO(bugs.webrtc.org/4762): remove this check after synchronization of
// more than two streams is supported.
if (video_config.sync_group.has_value()) {
