rocDecode integration in rocAL #253

fiona-gladwin · 2024-12-18T12:09:17Z

Add support for rocDecode hardware decoder in rocAL
Add support to create sequences using the seek operation in rocDecode, works seamlessly for H265 videos.

Requires VideoDemuxer to have .cpp file

…ation

Facing seg fault

Add utils folder to rocAL hip to compile with rocDecode

This reverts commit 21b0538.

Revert additional changes

…c_integration

kiritigowda · 2024-12-23T20:31:05Z

@fiona-gladwin failing to compile

/home/vsts/work/1/s/rocAL/source/decoders/video/rocdec_video_decoder.cpp:134:49: error: no member named 'selected_frame_dts_' in 'VideoSeekContext'
  134 |             required_frame_dts = video_seek_ctx.selected_frame_dts_;
      |                                  ~~~~~~~~~~~~~~ ^
/home/vsts/work/1/s/rocAL/source/decoders/video/rocdec_video_decoder.cpp:138:60: error: too many arguments to function call, expected at most 3, have 4
  138 |             _demuxer->Demux(&pvideo, &n_video_bytes, &pts, &dts);
      |             ~~~~~~~~~~~~~~~                                ^~~~
/home/vsts/work/1/rocm/share/rocdecode/utils/video_demuxer.h:156:14: note: 'Demux' declared here
  156 |         bool Demux(uint8_t **video, int *video_size, int64_t *pts = nullptr) {
      |              ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/vsts/work/1/s/rocAL/source/decoders/video/rocdec_video_decoder.cpp:23:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/iomanip:40:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/ios_base.h:41:

fiona-gladwin · 2024-12-24T04:02:13Z

@fiona-gladwin failing to compile

/home/vsts/work/1/s/rocAL/source/decoders/video/rocdec_video_decoder.cpp:134:49: error: no member named 'selected_frame_dts_' in 'VideoSeekContext'
  134 |             required_frame_dts = video_seek_ctx.selected_frame_dts_;
      |                                  ~~~~~~~~~~~~~~ ^
/home/vsts/work/1/s/rocAL/source/decoders/video/rocdec_video_decoder.cpp:138:60: error: too many arguments to function call, expected at most 3, have 4
  138 |             _demuxer->Demux(&pvideo, &n_video_bytes, &pts, &dts);
      |             ~~~~~~~~~~~~~~~                                ^~~~
/home/vsts/work/1/rocm/share/rocdecode/utils/video_demuxer.h:156:14: note: 'Demux' declared here
  156 |         bool Demux(uint8_t **video, int *video_size, int64_t *pts = nullptr) {
      |              ^     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /home/vsts/work/1/s/rocAL/source/decoders/video/rocdec_video_decoder.cpp:23:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/iomanip:40:
In file included from /usr/lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/ios_base.h:41:

@kiritigowda This PR requires PR #479 of rocDecode to be merged for successful compilation

rrawther

Please address the review comments

rrawther · 2025-01-10T17:15:34Z

cmake/FindrocDecode.cmake

+# 
+# MIT License
+# 
+# Copyright (c) 2024 Advanced Micro Devices, Inc.


rrawther · 2025-01-10T17:34:48Z

cmake/FindrocDecode.cmake

+find_path(ROCDECODE_INCLUDE_DIRS
+    NAMES rocdecode.h rocparser.h
+    HINTS
+    $ENV{rocDecode_PATH}/include/rocdecode


what is rocDecode_PATH? did you mean ROCM_PATH

rrawther · 2025-01-10T17:35:05Z

cmake/FindrocDecode.cmake

+find_library(ROCDECODE_LIBRARIES
+    NAMES rocdecode
+    HINTS
+    $ENV{rocDecode_PATH}/lib


same here. Please copy FindrocDecode from rocDecode repo instead

rrawther · 2025-01-10T18:05:17Z

rocAL/include/decoders/video/rocdec_video_decoder.h

+        RocDecVideoDecoder();
+        VideoDecoder::Status Initialize(const char *src_filename, int device_id = 0) override;
+        VideoDecoder::Status Decode(unsigned char *output_buffer, unsigned seek_frame_number, size_t sequence_length, size_t stride, int out_width, int out_height, int out_stride, AVPixelFormat out_format) override;
+        int seek_frame(AVRational avg_frame_rate, AVRational time_base, unsigned frame_number) override { return 0; }


please use same function name style for all functions here. Here snake and camel cases are mixed

rrawther · 2025-01-10T18:06:09Z

rocAL/include/decoders/video/rocdec_video_decoder.h

+#include "video_decoder.h"
+
+#ifdef ROCAL_VIDEO
+#if ENABLE_HIP


we should get rid of ENABLE_HIP and ENABLE_ROCDECODE flags

rrawther · 2025-01-10T18:33:58Z

rocAL/source/decoders/video/rocdec_video_decoder.cpp

+}
+
+// Seeks to the frame_number in the video file and decodes each frame in the sequence.
+VideoDecoder::Status RocDecVideoDecoder::Decode(unsigned char *out_buffer, unsigned seek_frame_number, size_t sequence_length, size_t stride, int out_width, int out_height, int out_stride, AVPixelFormat out_pix_format) {


rename out_buffer to output_buffer_ptr

rrawther · 2025-01-10T19:11:25Z

rocAL/source/decoders/video/rocdec_video_decoder.cpp

+            return Status::FAILED;
+        }
+        // Take the min of sequence length and num frames to avoid out of bounds memory error
+        int required_n_frames = std::min(static_cast<int>(sequence_length), n_frame_returned);


Isn't the sequence_length always <= n_frame_returned. In line#144 please check if n_frame_returned < sequence_length and return error.
Remove this line

rrawther · 2025-01-10T19:12:22Z

rocAL/source/decoders/video/rocdec_video_decoder.cpp

+        }
+        // Take the min of sequence length and num frames to avoid out of bounds memory error
+        int required_n_frames = std::min(static_cast<int>(sequence_length), n_frame_returned);
+        for (int i = 0; i < required_n_frames; i++) {


change it to sequence_length. What if there in not enough frames in the sequence when there is EOF?

rrawther · 2025-01-10T19:15:40Z

rocAL/source/decoders/video/rocdec_video_decoder.cpp

+        int required_n_frames = std::min(static_cast<int>(sequence_length), n_frame_returned);
+        for (int i = 0; i < required_n_frames; i++) {
+            uint8_t *pframe = _rocvid_decoder->GetFrame(&pts);
+            if (dts >= requested_frame_dts) {


how is the dts value updated here. We are only returning pts in the call

rrawther · 2025-01-10T19:16:22Z

rocAL/source/loaders/video/video_loader.cpp

@@ -102,6 +102,12 @@ void VideoLoader::set_output(Tensor *output_tensor) {
    _output_mem_size = ((_output_tensor->info().data_size() + 8) & ~7);  // Making output size as a multiple of 8 to support vectorized load and store in RPP
 }

+void VideoLoader::set_gpu_device_id(int device_id) {
+    if (device_id < 0)


please set to 0 here as default instead

fiona-gladwin and others added 30 commits May 8, 2024 02:52

Add CMakeList changes

b902adf

Requires VideoDemuxer to have .cpp file

Add rocDecode utils files and cmake changes

4b928e7

rocDecode integration for rocAL

ad1161a

Merge remote-tracking branch 'upstream/develop' into rr/rocdec_integr…

bcc6f5c

…ation

merge with tot

0fcb28d

Add initial support for rocDecode integration

342d9ac

Add fix to decode sequences

d26c18c

VideoDemuxer - support to return dts

bfd1ad9

Support to seek exact frame

6873f6c

Facing seg fault

Add intermediary buffers to copy

d89d3d9

Change the name of rocdec files

2e22877

Merge branch 'rocdecode_integrate_1' into rr/rocdec_integration

4bbc597

Temporary rocDecode cmake changes

21b0538

Add utils folder to rocAL hip to compile with rocDecode

Fix CMake for rocDecode

e343af0

Minor fix

9223116

Fix stride for rocdecode video decoder

526d170

Revert "Temporary rocDecode cmake changes"

b0a7b01

This reverts commit 21b0538.

Introduce CMake for rocDecode

494839e

Revert additional changes

Fix issues with CMake

1752acf

rocDecode Flush decoder after decoding each sequence

c03169a

Merge branch 'develop' of https://github.com/ROCm/rocAL into fg/rocde…

7a09b51

…c_integration

Add changes to flush decoder after seek

77041e3

Add support in rocDecode to seek to exact frame using dts

a937981

Add changes in API to pass device ID to video decoder

feed1c6

Merge branch 'develop' of https://github.com/ROCm/rocAL into fg/rocde…

b5d4a4c

…c_integration

Add stream synchronize

a6f9c04

Merge branch 'develop' of https://github.com/ROCm/rocAL into fg/rocde…

3ef5613

…c_integration

Merge branch 'develop' of https://github.com/ROCm/rocAL into fg/rocde…

8e04589

…c_integration

Minor changes

97b5a03

Minor change

0370b4d

fiona-gladwin added 6 commits December 13, 2024 11:07

Change rocDecode to all caps in CMake

5d0e2bd

Minor changes

a613ebf

Minor CMake change

c4d231f

Minor changes

0d3b80e

Introduce enable rocdecode flag in CMake

b341b31

Minor fix

9239eb4

fiona-gladwin requested review from kiritigowda and rrawther as code owners December 18, 2024 12:09

Merge branch 'develop' into fg/rocdec_integration

8507402

kiritigowda self-assigned this Dec 19, 2024

kiritigowda added enhancement New feature or request ci:precheckin labels Dec 19, 2024

Merge branch 'develop' into fg/rocdec_integration

1a23995

kiritigowda marked this pull request as draft December 23, 2024 20:31

fiona-gladwin and others added 5 commits January 2, 2025 15:00

Minor changes

d61e280

Merge branch 'develop' into fg/rocdec_integration

37f59aa

Merge branch 'develop' into fg/rocdec_integration

5ddf898

Merge branch 'develop' into fg/rocdec_integration

853353b

Merge branch 'develop' into fg/rocdec_integration

033689d

rrawther reviewed Jan 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rocDecode integration in rocAL #253

rocDecode integration in rocAL #253

fiona-gladwin commented Dec 18, 2024

kiritigowda commented Dec 23, 2024

fiona-gladwin commented Dec 24, 2024 •

edited

Loading

rrawther left a comment

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rrawther Jan 10, 2025

rocDecode integration in rocAL #253

Are you sure you want to change the base?

rocDecode integration in rocAL #253

Conversation

fiona-gladwin commented Dec 18, 2024

kiritigowda commented Dec 23, 2024

fiona-gladwin commented Dec 24, 2024 • edited Loading

rrawther left a 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

fiona-gladwin commented Dec 24, 2024 •

edited

Loading