Refactor video tests #3864

awolant · 2022-04-28T14:33:41Z

Refactor video tests for new video readers to make it easier to add new tests for new formats in the future. Extend tests coverage.

Category:

Refactoring of the video tests.

Checklist

Tests

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: N/A

JIRA TASK: N/A

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant · 2022-04-28T14:33:48Z

!build

dali-automaton · 2022-04-28T14:35:23Z

CI MESSAGE: [4701610]: BUILD STARTED

awolant · 2022-04-28T14:38:30Z

dali/operators/reader/loader/video/frames_decoder.cc

+
+  if (flush_state_) {
+    flush_state_ = false;
+  }
+


While improving tests coverage I found this small bug in the frames decoder - flush_state_ flag was not cleared while decoder was reset.

dali-automaton · 2022-04-28T16:27:47Z

CI MESSAGE: [4701610]: BUILD PASSED

JanuszL · 2022-04-28T17:36:53Z

dali/operators/reader/loader/video/frames_decoder_test.cc


-TEST_F(FramesDecoderGpuTest, VariableFrameRateGpu) {
+class FramesDecoderGpuTest : public FramesDecoderTestBase {


Is it possible to unify this with FramesDecoderTest_CpuOnlyTests?
The main difference I see is the additional copy of the data from DeviceBuffer to frame_cpu, and the different method of validation compared to ground_truth. I think these two things could be just a specialization of a method for the GPU test, but other steps could be common (the flow of the tests itself).

Didn't want to over do it in the beginning but sure, done.

JanuszL · 2022-04-28T17:43:54Z

dali/operators/reader/loader/video/video_test_base.cc

-std::vector<std::vector<cv::Mat>> VideoTestBase::vfr_frames_;
+void CompareFrameAvgError(
+  const uint8_t *ground_truth, const uint8_t *frame, int frame_size, double eps) {
+  std::vector<double> sums(std::thread::hardware_concurrency(), 0.0);


I wonder if you can hide the fact that detail::parallel_for uses std::thread::hardware_concurrency() for the thread number it spawns?
Maybe it could be detail::parallel_for_threads_no()?

JanuszL · 2022-04-28T17:49:13Z

dali/operators/reader/loader/video/frames_decoder_test.cc

+
+
+TEST_F(FramesDecoderTest_CpuOnlyTests, ConstantFrameRate) {
+  RunTest("/db/video/cfr/test_1.mp4", cfr_videos_[0]);


I wonder if we can have the path to the video and frames in the same place. Like:

RunTest("cfr_videos_[0].video",cfr_videos_[0].ref_frames);

?

Done. Now I keep all paths in VideoTestBase in lists and match by index. I did to to be able to pass whole list to op specs in reader tests.

dali/operators/reader/loader/video/video_test_base.h

JanuszL · 2022-04-28T18:10:07Z

dali/operators/reader/video_reader_decoder_op_test.cc

-  const int sequence_length = 6;
-  const int stride = 3;
-  const int step = 10;
+class VideoReaderDecoderGpuTest : public VideoReaderDecoderBaseTest {


Can we combine common parts with VideoReaderDecoderBaseTest? Or it is too much?

Signed-off-by: Albert Wolant <awolant@nvidia.com>

JanuszL · 2022-04-29T14:28:23Z

dali/operators/reader/loader/video/frames_decoder_test.cc

-    }
-}
+ private:
+  std::vector<uint8_t> frame_buffer;


Suggested change

std::vector<uint8_t> frame_buffer;

std::vector<uint8_t> frame_buffer_;

JanuszL · 2022-04-29T14:28:34Z

dali/operators/reader/loader/video/frames_decoder_test.cc

+  std::vector<uint8_t> frame_cpu_buffer;
+  DeviceBuffer<uint8_t> frame_gpu_buffer;


Suggested change

std::vector<uint8_t> frame_cpu_buffer;

DeviceBuffer<uint8_t> frame_gpu_buffer;

std::vector<uint8_t> frame_cpu_buffer_;

DeviceBuffer<uint8_t> frame_gpu_buffer_;

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant · 2022-05-02T08:42:51Z

dali/operators/reader/loader/video/video_loader_decoder_cpu.cc

@@ -18,6 +18,7 @@ namespace dali {
 void VideoLoaderDecoderCpu::PrepareEmpty(VideoSample<CPUBackend> &sample) {
  sample = {};
  sample.data_.set_pinned(false);
+  sample.data_.SetLayout("FHWC");


Another small bug fix I found during the tests rework - layout was not set in CPU reader.

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant · 2022-05-02T10:41:32Z

!build

dali-automaton · 2022-05-02T10:45:24Z

CI MESSAGE: [4728585]: BUILD STARTED

dali-automaton · 2022-05-02T11:58:21Z

CI MESSAGE: [4728585]: BUILD FAILED

awolant · 2022-05-04T07:43:40Z

!build

dali-automaton · 2022-05-04T07:45:02Z

CI MESSAGE: [4746024]: BUILD STARTED

dali-automaton · 2022-05-04T08:54:08Z

CI MESSAGE: [4746024]: BUILD PASSED

* Refactor video tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant added 2 commits April 28, 2022 16:30

Refactor video tests

8df8799

Signed-off-by: Albert Wolant <awolant@nvidia.com>

Remove unused code

ef66376

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant commented Apr 28, 2022

View reviewed changes

JanuszL self-requested a review April 28, 2022 17:06

JanuszL self-assigned this Apr 28, 2022

JanuszL reviewed Apr 28, 2022

View reviewed changes

dali/operators/reader/loader/video/video_test_base.h Show resolved Hide resolved

JanuszL reviewed Apr 28, 2022

View reviewed changes

jantonguirao assigned prak-nv Apr 29, 2022

Review fix

7f52441

Signed-off-by: Albert Wolant <awolant@nvidia.com>

JanuszL reviewed Apr 29, 2022

View reviewed changes

awolant added 4 commits April 29, 2022 16:36

Review fix

b197d10

Signed-off-by: Albert Wolant <awolant@nvidia.com>

Review fix

021cb9f

Signed-off-by: Albert Wolant <awolant@nvidia.com>

Review fix

8982547

Signed-off-by: Albert Wolant <awolant@nvidia.com>

Fix layout in CPU reader decoder

2774e38

Signed-off-by: Albert Wolant <awolant@nvidia.com>

awolant commented May 2, 2022

View reviewed changes

awolant added 2 commits May 2, 2022 10:47

Fix dates

42dc4af

Signed-off-by: Albert Wolant <awolant@nvidia.com>

Review fix

dde7a2c

Signed-off-by: Albert Wolant <awolant@nvidia.com>

JanuszL approved these changes May 4, 2022

View reviewed changes

JanuszL unassigned prak-nv May 4, 2022

jantonguirao assigned banasraf May 4, 2022

banasraf approved these changes May 6, 2022

View reviewed changes

awolant merged commit ebd64eb into NVIDIA:main May 10, 2022

cyyever pushed a commit to cyyever/DALI that referenced this pull request May 13, 2022

Refactor video tests (NVIDIA#3864)

41c1e5e

* Refactor video tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jun 7, 2022

Refactor video tests (NVIDIA#3864)

3cd08f6

* Refactor video tests Signed-off-by: Albert Wolant <awolant@nvidia.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor video tests #3864

Refactor video tests #3864

awolant commented Apr 28, 2022 •

edited

awolant commented Apr 28, 2022

dali-automaton commented Apr 28, 2022

awolant Apr 28, 2022

dali-automaton commented Apr 28, 2022

JanuszL Apr 28, 2022

awolant Apr 29, 2022

JanuszL Apr 28, 2022

awolant Apr 29, 2022

JanuszL Apr 28, 2022

awolant May 2, 2022

JanuszL Apr 28, 2022

awolant May 2, 2022

JanuszL Apr 29, 2022

awolant Apr 29, 2022

JanuszL Apr 29, 2022

awolant Apr 29, 2022

awolant May 2, 2022

awolant commented May 2, 2022

dali-automaton commented May 2, 2022

dali-automaton commented May 2, 2022

awolant commented May 4, 2022

dali-automaton commented May 4, 2022

dali-automaton commented May 4, 2022


		TEST_F(FramesDecoderGpuTest, VariableFrameRateGpu) {
		class FramesDecoderGpuTest : public FramesDecoderTestBase {



		TEST_F(FramesDecoderTest_CpuOnlyTests, ConstantFrameRate) {
		RunTest("/db/video/cfr/test_1.mp4", cfr_videos_[0]);

	std::vector<uint8_t> frame_buffer;
	std::vector<uint8_t> frame_buffer_;

		std::vector<uint8_t> frame_cpu_buffer;
		DeviceBuffer<uint8_t> frame_gpu_buffer;

Refactor video tests #3864

Refactor video tests #3864

Conversation

awolant commented Apr 28, 2022 • edited

Category:

Checklist

Tests

Documentation

DALI team only

Requirements

awolant commented Apr 28, 2022

dali-automaton commented Apr 28, 2022

Choose a reason for hiding this comment

dali-automaton commented Apr 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awolant commented May 2, 2022

dali-automaton commented May 2, 2022

dali-automaton commented May 2, 2022

awolant commented May 4, 2022

dali-automaton commented May 4, 2022

dali-automaton commented May 4, 2022

awolant commented Apr 28, 2022 •

edited