Extend decoding support #4480

awolant · 2022-11-30T17:58:57Z

Category:

New feature

Description:

Adds ability for FramesDecoderGpu to decode raw H264 i H265 streams from memory.

Additional information:

Affected modules and functionalities:

FramesDecoderGpu and its tests. Some changes to its base class FramesDecoder were also necessary.

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: N/A

JIRA TASK: DALI-3147

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

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

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

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

dali-automaton · 2022-12-06T20:28:16Z

CI MESSAGE: [6696799]: BUILD FAILED

dali-automaton · 2022-12-06T20:30:31Z

CI MESSAGE: [6696815]: BUILD FAILED

dali-automaton · 2022-12-07T09:42:24Z

CI MESSAGE: [6703268]: BUILD STARTED

dali-automaton · 2022-12-07T10:21:17Z

CI MESSAGE: [6703268]: BUILD FAILED

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

dali-automaton · 2022-12-07T10:39:18Z

CI MESSAGE: [6703703]: BUILD STARTED

dali-automaton · 2022-12-07T11:24:28Z

CI MESSAGE: [6703703]: BUILD FAILED

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

dali-automaton · 2022-12-07T13:15:39Z

CI MESSAGE: [6704652]: BUILD STARTED

dali-automaton · 2022-12-07T14:57:38Z

CI MESSAGE: [6704652]: BUILD PASSED

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

dali-automaton · 2022-12-09T13:57:39Z

CI MESSAGE: [6727772]: BUILD STARTED

JanuszL · 2022-12-09T14:25:50Z

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

+    CreateAvState(tmp_av_state, false);
+
+    while (av_read_frame(tmp_av_state->ctx_, tmp_av_state->packet_) >= 0) {
+      // We want to make sure that we call av_packet_unref in every iteration


Do you think we can extract this to a function as the same pattern in applied in L291.

Done, called CountFrames.

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

JanuszL · 2022-12-09T14:32:03Z

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

+      if (ctx_->pb != nullptr) {
+        avio_context_free(&ctx_->pb);
+      }
+      avformat_close_input(&ctx_);


Based on https://ffmpeg.org/doxygen/trunk/demux_8c_source.html#l00369, 'avformat_close_input' seems to call avformat_free_context and avio_close which calls avio_context_free. Still please double check this in the version of FFmpeg we use (asan doesn't complain so I think we are not leaking anything).

I did that based on the documentation that said that allocation of this context should be freed. Since in this scenario that we use it the wrapping object destructor takes care of it, I removed the call in our code.

I think that sometimes the examples and docs of FFmpeg are not fully accurate.

JanuszL · 2022-12-09T14:32:51Z

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

    NVDECLease GetDecoder(CUVIDEOFORMAT *video_format) {
      std::unique_lock lock(access_lock);

      auto codec_type = video_format->codec;
-      unsigned height = video_format->display_area.bottom - video_format->display_area.top;
-      unsigned width = video_format->display_area.right - video_format->display_area.left;
+      unsigned height =  video_format->coded_height;


It seems to affect the decoding speed but it was the only way to get some decoders working 🤷

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

JanuszL · 2022-12-09T14:39:03Z

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

+  // If so, we need to return frame from the buffer before sending last packet.
+  if (frame_index_if_no_pts_ != 0) {
+    if (NumEmptySpots() < (piped_pts_.size())) {
+      for (size_t i = 0; i < frame_buffer_.size(); ++i) {


Is it possible that we won't find desired frame in the frame_buffer_ as it has not been decoded yet?

I think that theoretically, this can happen. The ultimately resilient solution for this issue I have found is to allocate spaces in the buffer dynamically as much as needed.
In practice, I have never encountered this issue yet, we have buffer big enough for the files that we have seen so far.

I would like to leave it as it is now. When we do decoupling mentioned above, we can revisit it and allow for buffer dynamic extension or something like that. Is that ok?

Maybe we can error in such case instead of silently returning the wrong result so the user can provide us with a failing sample and we can adjust?

I think right now it should error out with Could not find empty slot in the frame buffer. It should try to find the next frame, fail to do it, than call for another frame, try to place it in the buffer and error out due to lack of empty spots.

dali-automaton · 2022-12-09T15:32:27Z

CI MESSAGE: [6727772]: BUILD PASSED

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

JanuszL · 2022-12-12T15:01:52Z

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

+    av_state->codec_ctx_ = avcodec_alloc_context3(av_state->codec_);
+    DALI_ENFORCE(av_state->codec_ctx_, "Could not alloc av codec context");
+
+    ret = avcodec_parameters_to_context(av_state->codec_ctx_, av_state->codec_params_);
+    DALI_ENFORCE(
+      ret >= 0,
+      make_string("Could not fill the codec based on parameters: ", detail::av_error_string(ret)));
+
+    av_state->packet_ = av_packet_alloc();
+    DALI_ENFORCE(av_state->packet_, "Could not allocate av packet");


I think you can replace it with InitAvState().

CreateAvState covers some parts of the InitAvState and some parts of other functions. I would like to decouple the av state manipulation from this code altogether, so I will refactor it during that, if that is fine?

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

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

awolant · 2022-12-12T18:23:55Z

!build

dali-automaton · 2022-12-12T18:51:42Z

CI MESSAGE: [6752073]: BUILD STARTED

dali-automaton · 2022-12-12T22:24:20Z

CI MESSAGE: [6752073]: BUILD PASSED

awolant added 8 commits November 29, 2022 17:31

Extend decoding support

7a93080

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

Add checking of the flag

f0428b5

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

Add working parsing for all files

7292b0e

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

Merge all file support

23c5092

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

Port changes to GPU decoder

a79cda5

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

Working for all files

2a827d3

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

Add support for all files

8c5e2d2

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

Remove test

29261c4

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

JanuszL reviewed Nov 30, 2022

View reviewed changes

dali/operators/reader/loader/video/frames_decoder.cc Show resolved Hide resolved

JanuszL reviewed Nov 30, 2022

View reviewed changes

dali/operators/reader/loader/video/frames_decoder.cc Show resolved Hide resolved

JanuszL reviewed Nov 30, 2022

View reviewed changes

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

JanuszL mentioned this pull request Dec 1, 2022

nvidia.dali.fn.experimental.decoders.video cannot parse raw HEVC #4479

Closed

awolant added 7 commits December 5, 2022 22:45

Sort of working version

5595867

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

Remove changes to decoding

eecbb09

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

Fix tests

6ca3745

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

Working for all files

3ee2991

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

Fixes and tests

4a8b8d9

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

Adjust tests

f07ef9d

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

Fix linter

87153e9

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

Merge remote-tracking branch 'nvidia/main' into extend_decoding_support

e0b3e69

Fix linter

7f6db46

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

Fix werror build

666c2b9

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

jantonguirao assigned stiepan Dec 9, 2022

Update to force zero latency

2939e22

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

szalpal approved these changes Dec 9, 2022

View reviewed changes

awolant added 2 commits December 9, 2022 14:01

Update raw stream tests

eccc2db

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

Update DALI_extra version

624f3c0

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

JanuszL reviewed Dec 9, 2022

View reviewed changes

dali/operators/reader/loader/video/frames_decoder.cc Show resolved Hide resolved

JanuszL reviewed Dec 9, 2022

View reviewed changes

dali/operators/reader/loader/video/frames_decoder_gpu.cc Show resolved Hide resolved

JanuszL reviewed Dec 9, 2022

View reviewed changes

awolant added 6 commits December 9, 2022 16:38

Update DALI_EXTRA_VERSION

4690624

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

Fix typo

42dffe6

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

Merge remote-tracking branch 'nvidia/main' into extend_decoding_support

77b7248

Merge remote-tracking branch 'nvidia/main' into extend_decoding_support

f92e8d5

Review fixes

2b24562

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

Review fixes

b732928

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

JanuszL assigned JanuszL and unassigned stiepan Dec 12, 2022

JanuszL reviewed Dec 12, 2022

View reviewed changes

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

JanuszL reviewed Dec 12, 2022

View reviewed changes

dali/operators/reader/loader/video/frames_decoder_gpu.cc Show resolved Hide resolved

JanuszL approved these changes Dec 12, 2022

View reviewed changes

awolant merged commit c7d6ddf into NVIDIA:main Dec 13, 2022

JanuszL mentioned this pull request Sep 6, 2023

Roadmap 2023 #4578

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend decoding support #4480

Extend decoding support #4480

awolant commented Nov 30, 2022 •

edited

Loading

dali-automaton commented Dec 6, 2022

dali-automaton commented Dec 6, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 9, 2022

JanuszL Dec 9, 2022 •

edited

Loading

awolant Dec 12, 2022

JanuszL Dec 9, 2022

awolant Dec 12, 2022

JanuszL Dec 12, 2022

JanuszL Dec 9, 2022

JanuszL Dec 9, 2022

awolant Dec 12, 2022

JanuszL Dec 12, 2022

awolant Dec 12, 2022

dali-automaton commented Dec 9, 2022

JanuszL Dec 12, 2022

awolant Dec 12, 2022

JanuszL Dec 12, 2022

awolant commented Dec 12, 2022

dali-automaton commented Dec 12, 2022

dali-automaton commented Dec 12, 2022

Extend decoding support #4480

Extend decoding support #4480

Conversation

awolant commented Nov 30, 2022 • edited Loading

Category:

Description:

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Tests:

Checklist

Documentation

DALI team only

Requirements

dali-automaton commented Dec 6, 2022

dali-automaton commented Dec 6, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 7, 2022

dali-automaton commented Dec 9, 2022

JanuszL Dec 9, 2022 • edited Loading

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

dali-automaton commented Dec 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awolant commented Dec 12, 2022

dali-automaton commented Dec 12, 2022

dali-automaton commented Dec 12, 2022

awolant commented Nov 30, 2022 •

edited

Loading

JanuszL Dec 9, 2022 •

edited

Loading