[PCLF] Include video resolution into video dump file name

Bug: b/240540204
Change-Id: Idad6a5c67c2dcedb07cfa915ac986590c1e29275
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280383
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Artem Titov <titovartem@webrtc.org>
Reviewed-by: Andrey Logvin <landrey@google.com>
Cr-Commit-Position: refs/heads/main@{#38470}

api/BUILD.gn

@@ -1325,6 +1325,7 @@ if (rtc_include_tests) {
       "../test:rtc_expect_death",
       "../test:test_support",
       "task_queue:task_queue_default_factory_unittests",
+      "test/video:video_frame_writer",
       "transport:field_trial_based_config",
       "units:time_delta",
       "units:timestamp",

api/test/peerconnection_quality_test_fixture.cc

@@ -29,6 +29,8 @@ using VideoCodecConfig = ::webrtc::webrtc_pc_e2e::
     PeerConnectionE2EQualityTestFixture::VideoCodecConfig;
 using VideoSubscription = ::webrtc::webrtc_pc_e2e::
     PeerConnectionE2EQualityTestFixture::VideoSubscription;
+using VideoResolution = ::webrtc::webrtc_pc_e2e::
+    PeerConnectionE2EQualityTestFixture::VideoResolution;
 
 std::string SpecToString(
     PeerConnectionE2EQualityTestFixture::VideoResolution::VideoResolution::Spec
@@ -42,6 +44,12 @@ std::string SpecToString(
   }
 }
 
+void AppendResolution(const VideoResolution& resolution,
+                      rtc::StringBuilder& builder) {
+  builder << "_" << resolution.width() << "x" << resolution.height() << "_"
+          << resolution.fps();
+}
+
 }  // namespace
 
 PeerConnectionE2EQualityTestFixture::VideoResolution::VideoResolution(
@@ -152,9 +160,9 @@ std::unique_ptr<test::VideoFrameWriter> PeerConnectionE2EQualityTestFixture::
         absl::string_view stream_label,
         const VideoResolution& resolution) const {
   std::unique_ptr<test::VideoFrameWriter> writer = video_frame_writer_factory_(
-      GetInputDumpFileName(stream_label), resolution);
+      GetInputDumpFileName(stream_label, resolution), resolution);
   absl::optional<std::string> frame_ids_file =
-      GetInputFrameIdsDumpFileName(stream_label);
+      GetInputFrameIdsDumpFileName(stream_label, resolution);
   if (frame_ids_file.has_value()) {
     writer = CreateVideoFrameWithIdsWriter(std::move(writer), *frame_ids_file);
   }
@@ -167,9 +175,9 @@ std::unique_ptr<test::VideoFrameWriter> PeerConnectionE2EQualityTestFixture::
         absl::string_view receiver,
         const VideoResolution& resolution) const {
   std::unique_ptr<test::VideoFrameWriter> writer = video_frame_writer_factory_(
-      GetOutputDumpFileName(stream_label, receiver), resolution);
+      GetOutputDumpFileName(stream_label, receiver, resolution), resolution);
   absl::optional<std::string> frame_ids_file =
-      GetOutputFrameIdsDumpFileName(stream_label, receiver);
+      GetOutputFrameIdsDumpFileName(stream_label, receiver, resolution);
   if (frame_ids_file.has_value()) {
     writer = CreateVideoFrameWithIdsWriter(std::move(writer), *frame_ids_file);
   }
@@ -187,36 +195,45 @@ std::unique_ptr<test::VideoFrameWriter> PeerConnectionE2EQualityTestFixture::
 
 std::string
 PeerConnectionE2EQualityTestFixture::VideoDumpOptions::GetInputDumpFileName(
-    absl::string_view stream_label) const {
+    absl::string_view stream_label,
-  return test::JoinFilename(output_directory_, stream_label);
+    const VideoResolution& resolution) const {
+  rtc::StringBuilder file_name;
+  file_name << stream_label;
+  AppendResolution(resolution, file_name);
+  return test::JoinFilename(output_directory_, file_name.Release());
 }
 
 absl::optional<std::string> PeerConnectionE2EQualityTestFixture::
     VideoDumpOptions::GetInputFrameIdsDumpFileName(
-        absl::string_view stream_label) const {
+        absl::string_view stream_label,
+        const VideoResolution& resolution) const {
   if (!export_frame_ids_) {
     return absl::nullopt;
   }
-  return GetInputDumpFileName(stream_label) + ".frame_ids.txt";
+  return GetInputDumpFileName(stream_label, resolution) + ".frame_ids.txt";
 }
 
 std::string
 PeerConnectionE2EQualityTestFixture::VideoDumpOptions::GetOutputDumpFileName(
     absl::string_view stream_label,
-    absl::string_view receiver) const {
+    absl::string_view receiver,
+    const VideoResolution& resolution) const {
   rtc::StringBuilder file_name;
   file_name << stream_label << "_" << receiver;
+  AppendResolution(resolution, file_name);
   return test::JoinFilename(output_directory_, file_name.Release());
 }
 
 absl::optional<std::string> PeerConnectionE2EQualityTestFixture::
     VideoDumpOptions::GetOutputFrameIdsDumpFileName(
         absl::string_view stream_label,
-        absl::string_view receiver) const {
+        absl::string_view receiver,
+        const VideoResolution& resolution) const {
   if (!export_frame_ids_) {
     return absl::nullopt;
   }
-  return GetOutputDumpFileName(stream_label, receiver) + ".frame_ids.txt";
+  return GetOutputDumpFileName(stream_label, receiver, resolution) +
+         ".frame_ids.txt";
 }
 
 std::string PeerConnectionE2EQualityTestFixture::VideoDumpOptions::ToString()

api/test/peerconnection_quality_test_fixture.h

@@ -251,9 +251,10 @@ class PeerConnectionE2EQualityTestFixture {
 
     // output_directory - the output directory where stream will be dumped. The
     // output files' names will be constructed as
-    // <stream_name>_<receiver_name>.<extension> for output dumps and
+    // <stream_name>_<receiver_name>_<resolution>.<extension> for output dumps
-    // <stream_name>.<extension> for input dumps. By default <extension> is
+    // and <stream_name>_<resolution>.<extension> for input dumps.
-    // "y4m".
+    // By default <extension> is "y4m". Resolution is in the format
+    // <width>x<height>_<fps>.
     // sampling_modulo - 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. (Default: 1)
@@ -300,18 +301,22 @@ class PeerConnectionE2EQualityTestFixture {
     static std::unique_ptr<test::VideoFrameWriter> Y4mVideoFrameWriterFactory(
         absl::string_view file_name_prefix,
         const VideoResolution& resolution);
-    std::string GetInputDumpFileName(absl::string_view stream_label) const;
+    std::string GetInputDumpFileName(absl::string_view stream_label,
+                                     const VideoResolution& resolution) const;
     // Returns file name for input frame ids dump if `export_frame_ids()` is
     // true, absl::nullopt otherwise.
     absl::optional<std::string> GetInputFrameIdsDumpFileName(
-        absl::string_view stream_label) const;
+        absl::string_view stream_label,
+        const VideoResolution& resolution) const;
     std::string GetOutputDumpFileName(absl::string_view stream_label,
-                                      absl::string_view receiver) const;
+                                      absl::string_view receiver,
+                                      const VideoResolution& resolution) const;
     // Returns file name for output frame ids dump if `export_frame_ids()` is
     // true, absl::nullopt otherwise.
     absl::optional<std::string> GetOutputFrameIdsDumpFileName(
         absl::string_view stream_label,
-        absl::string_view receiver) const;
+        absl::string_view receiver,
+        const VideoResolution& resolution) const;
 
     std::string output_directory_;
     int sampling_modulo_ = 1;
@@ -384,7 +389,7 @@ class PeerConnectionE2EQualityTestFixture {
     // stream on receiver side per each receiver.
     absl::optional<VideoDumpOptions> output_dump_options;
     // If set to true uses fixed frame rate while dumping output video to the
-    // file. `fps` will be used as frame rate.
+    // file. Requested `VideoSubscription::fps()` will be used as frame rate.
     bool output_dump_use_fixed_framerate = false;
     // If true will display input and output video on the user's screen.
     bool show_on_screen = false;

api/test/peerconnection_quality_test_fixture_unittest.cc

@@ -13,19 +13,25 @@
 #include <vector>
 
 #include "absl/types/optional.h"
+#include "api/test/video/video_frame_writer.h"
 #include "rtc_base/gunit.h"
 #include "test/gmock.h"
+#include "test/testsupport/file_utils.h"
 
 namespace webrtc {
 namespace webrtc_pc_e2e {
 namespace {
 
+using ::testing::Eq;
+
 using VideoResolution = ::webrtc::webrtc_pc_e2e::
     PeerConnectionE2EQualityTestFixture::VideoResolution;
 using VideoConfig =
     ::webrtc::webrtc_pc_e2e::PeerConnectionE2EQualityTestFixture::VideoConfig;
 using VideoSubscription = ::webrtc::webrtc_pc_e2e::
     PeerConnectionE2EQualityTestFixture::VideoSubscription;
+using VideoDumpOptions = ::webrtc::webrtc_pc_e2e::
+    PeerConnectionE2EQualityTestFixture::VideoDumpOptions;
 
 TEST(PclfVideoSubscriptionTest,
      MaxFromSenderSpecEqualIndependentOfOtherFields) {
@@ -78,6 +84,67 @@ TEST(PclfVideoSubscriptionTest, GetMaxResolutionSelectMaxForEachDimention) {
   EXPECT_EQ(resolution->fps(), 10);
 }
 
+struct TestVideoFrameWriter : public test::VideoFrameWriter {
+ public:
+  TestVideoFrameWriter(absl::string_view file_name_prefix,
+                       const VideoResolution& resolution)
+      : file_name_prefix(file_name_prefix), resolution(resolution) {}
+
+  bool WriteFrame(const VideoFrame& frame) override { return true; }
+
+  void Close() override {}
+
+  std::string file_name_prefix;
+  VideoResolution resolution;
+};
+
+TEST(VideoDumpOptionsTest, InputVideoWriterHasCorrectFileName) {
+  VideoResolution resolution(/*width=*/1280, /*height=*/720, /*fps=*/30);
+
+  TestVideoFrameWriter* writer = nullptr;
+  VideoDumpOptions options("foo", /*sampling_modulo=*/1,
+                           /*export_frame_ids=*/false,
+                           /*video_frame_writer_factory=*/
+                           [&](absl::string_view file_name_prefix,
+                               const VideoResolution& resolution) {
+                             auto out = std::make_unique<TestVideoFrameWriter>(
+                                 file_name_prefix, resolution);
+                             writer = out.get();
+                             return out;
+                           });
+  std::unique_ptr<test::VideoFrameWriter> created_writer =
+      options.CreateInputDumpVideoFrameWriter("alice-video", resolution);
+
+  ASSERT_TRUE(writer != nullptr);
+  ASSERT_THAT(writer->file_name_prefix,
+              Eq(test::JoinFilename("foo", "alice-video_1280x720_30")));
+  ASSERT_THAT(writer->resolution, Eq(resolution));
+}
+
+TEST(VideoDumpOptionsTest, OutputVideoWriterHasCorrectFileName) {
+  VideoResolution resolution(/*width=*/1280, /*height=*/720, /*fps=*/30);
+
+  TestVideoFrameWriter* writer = nullptr;
+  VideoDumpOptions options("foo", /*sampling_modulo=*/1,
+                           /*export_frame_ids=*/false,
+                           /*video_frame_writer_factory=*/
+                           [&](absl::string_view file_name_prefix,
+                               const VideoResolution& resolution) {
+                             auto out = std::make_unique<TestVideoFrameWriter>(
+                                 file_name_prefix, resolution);
+                             writer = out.get();
+                             return out;
+                           });
+  std::unique_ptr<test::VideoFrameWriter> created_writer =
+      options.CreateOutputDumpVideoFrameWriter("alice-video", "bob",
+                                               resolution);
+
+  ASSERT_TRUE(writer != nullptr);
+  ASSERT_THAT(writer->file_name_prefix,
+              Eq(test::JoinFilename("foo", "alice-video_bob_1280x720_30")));
+  ASSERT_THAT(writer->resolution, Eq(resolution));
+}
+
 }  // namespace
 }  // namespace webrtc_pc_e2e
 }  // namespace webrtc