-
Notifications
You must be signed in to change notification settings - Fork 622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds enable_frame_num
to the experimental video reader
#5628
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -20,7 +20,8 @@ namespace dali { | |||||||||||||||||
|
||||||||||||||||||
VideoReaderDecoderCpu::VideoReaderDecoderCpu(const OpSpec &spec) | ||||||||||||||||||
: DataReader<CPUBackend, VideoSampleCpu, VideoSampleCpu, true>(spec), | ||||||||||||||||||
has_labels_(spec.HasArgument("labels")) { | ||||||||||||||||||
has_labels_(spec.HasArgument("labels")), | ||||||||||||||||||
has_frame_no_(spec.GetArgument<bool>("enable_frame_num")) { | ||||||||||||||||||
loader_ = InitLoader<VideoLoaderDecoderCpu>(spec); | ||||||||||||||||||
this->SetInitialSnapshot(); | ||||||||||||||||||
} | ||||||||||||||||||
|
@@ -32,16 +33,28 @@ void VideoReaderDecoderCpu::RunImpl(SampleWorkspace &ws) { | |||||||||||||||||
video_output.Copy(sample.data_); | ||||||||||||||||||
video_output.SetSourceInfo(sample.data_.GetSourceInfo()); | ||||||||||||||||||
|
||||||||||||||||||
int out_index = 1; | ||||||||||||||||||
if (has_labels_) { | ||||||||||||||||||
auto &label_output = ws.Output<CPUBackend>(1); | ||||||||||||||||||
auto &label_output = ws.Output<CPUBackend>(out_index); | ||||||||||||||||||
label_output.Resize({}, DALIDataType::DALI_INT32); | ||||||||||||||||||
label_output.mutable_data<int>()[0] = sample.label_; | ||||||||||||||||||
out_index++; | ||||||||||||||||||
} | ||||||||||||||||||
if (has_frame_no_) { | ||||||||||||||||||
auto &frame_no_output = ws.Output<CPUBackend>(out_index); | ||||||||||||||||||
frame_no_output.Resize({}, DALIDataType::DALI_INT32); | ||||||||||||||||||
frame_no_output.mutable_data<int>()[0] = sample.first_frame_; | ||||||||||||||||||
out_index++; | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
namespace detail { | ||||||||||||||||||
inline int VideoReaderDecoderOutputFn(const OpSpec &spec) { | ||||||||||||||||||
return spec.HasArgument("labels") ? 2 : 1; | ||||||||||||||||||
int num_outputs = 1; | ||||||||||||||||||
if (spec.HasArgument("labels")) num_outputs++; | ||||||||||||||||||
bool enable_frame_num = spec.GetArgument<bool>("enable_frame_num"); | ||||||||||||||||||
if (enable_frame_num) num_outputs++; | ||||||||||||||||||
return num_outputs; | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps clearer (?) Your call.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||||||||||||||
} | ||||||||||||||||||
} // namespace detail | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -68,6 +81,10 @@ even in the variable frame rate scenario.)code") | |||||||||||||||||
.AddArg("sequence_length", | ||||||||||||||||||
R"code(Frames to load per sequence.)code", | ||||||||||||||||||
DALI_INT32) | ||||||||||||||||||
.AddOptionalArg("enable_frame_num", | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to keep it consistent with the video reader . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand and agree - nonetheless, it's unfortunate. |
||||||||||||||||||
R"code(If set, returns the first frame number in the decoded sequence | ||||||||||||||||||
as a separate output.)code", | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||||||||||||||
false) | ||||||||||||||||||
.AddOptionalArg("step", | ||||||||||||||||||
R"code(Frame interval between each sequence. | ||||||||||||||||||
|
||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -35,6 +35,7 @@ class VideoReaderDecoderGpu : public DataReader<GPUBackend, VideoSampleGpu, Vide | |||||
|
||||||
private: | ||||||
bool has_labels_ = false; | ||||||
bool has_frame_no_ = false; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
}; | ||||||
|
||||||
} // namespace dali | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -40,6 +40,9 @@ class VideoReaderDecoderBaseTest : public VideoTestBase { | |||||
virtual void AssertFrame( | ||||||
int frame_id, const uint8_t *frame, TestVideo &ground_truth) = 0; | ||||||
|
||||||
template<typename Backend> | ||||||
int GetFrameNo(dali::TensorList<Backend> &device_frame_no); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
|
||||||
private: | ||||||
template<typename Backend> | ||||||
void RunTestImpl( | ||||||
|
@@ -129,15 +132,15 @@ class VideoReaderDecoderBaseTest : public VideoTestBase { | |||||
.AddArg("device", backend) | ||||||
.AddArg("sequence_length", sequence_length) | ||||||
.AddArg("random_shuffle", true) | ||||||
.AddArg("enable_frame_num", true) | ||||||
.AddArg("initial_fill", cfr_videos_[0].NumFrames()) | ||||||
.AddArg( | ||||||
"filenames", | ||||||
std::vector<std::string>{cfr_videos_paths_[0]}) | ||||||
.AddOutput("frames", backend)); | ||||||
|
||||||
pipe.Build({{"frames", backend}}); | ||||||
.AddOutput("frames", backend) | ||||||
.AddOutput("frame_no", backend)); | ||||||
|
||||||
std::vector<int> expected_order = {29, 46, 33, 6, 37}; | ||||||
pipe.Build({{"frames", backend}, {"frame_no", backend}}); | ||||||
|
||||||
int num_sequences = 5; | ||||||
|
||||||
|
@@ -148,9 +151,10 @@ class VideoReaderDecoderBaseTest : public VideoTestBase { | |||||
|
||||||
auto &frame_video_output = ws.Output<Backend>(0); | ||||||
const auto sample = frame_video_output.template tensor<uint8_t>(0); | ||||||
int frame_no = GetFrameNo(ws.Output<Backend>(1)); | ||||||
|
||||||
// We want to access correct order, so we comapre only the first frame of the sequence | ||||||
AssertFrame(expected_order[sequence_id], sample, ground_truth_video); | ||||||
// We want to access correct order, so we compare only the first frame of the sequence | ||||||
AssertFrame(frame_no, sample, ground_truth_video); | ||||||
} | ||||||
} | ||||||
}; | ||||||
|
@@ -168,6 +172,15 @@ void VideoReaderDecoderBaseTest::RunShuffleTest<dali::CPUBackend>() { | |||||
RunShuffleTestImpl<dali::CPUBackend>("cpu", dali::CPU_ONLY_DEVICE_ID); | ||||||
} | ||||||
|
||||||
template<> | ||||||
int VideoReaderDecoderBaseTest::GetFrameNo( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
...if going with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
dali::TensorList<dali::CPUBackend> &device_frame_no) { | ||||||
const auto frame_no = device_frame_no.template tensor<int>(0); | ||||||
int frame_no_buffer = -1; | ||||||
std::copy_n(frame_no, 1, &frame_no_buffer); | ||||||
return frame_no_buffer; | ||||||
} | ||||||
|
||||||
template<> | ||||||
void VideoReaderDecoderBaseTest::RunTest<dali::GPUBackend>( | ||||||
std::vector<std::string> &videos_paths, | ||||||
|
@@ -181,6 +194,15 @@ void VideoReaderDecoderBaseTest::RunShuffleTest<dali::GPUBackend>() { | |||||
RunShuffleTestImpl<dali::GPUBackend>("gpu", 0); | ||||||
} | ||||||
|
||||||
template<> | ||||||
int VideoReaderDecoderBaseTest::GetFrameNo( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
dali::TensorList<dali::GPUBackend> &device_frame_no) { | ||||||
const auto frame_no = device_frame_no.template tensor<int>(0); | ||||||
int frame_no_buffer = -1; | ||||||
MemCopy(&frame_no_buffer, frame_no, sizeof(int)); | ||||||
return frame_no_buffer; | ||||||
} | ||||||
|
||||||
class VideoReaderDecoderCpuTest : public VideoReaderDecoderBaseTest { | ||||||
public: | ||||||
void AssertLabel(const int *label, int ground_truth_label) override { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
VideoSample
already has a non-trivial constructor, some initialization (e.g. to invalid values, like -1) would be nice.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed