Skip to content

rocAL Fused Crop decoding support using rocJpeg#368

Merged
kiritigowda merged 101 commits intoROCm:developfrom
SundarRajan28:fg/rocjpeg_crop_decoder
Sep 4, 2025
Merged

rocAL Fused Crop decoding support using rocJpeg#368
kiritigowda merged 101 commits intoROCm:developfrom
SundarRajan28:fg/rocjpeg_crop_decoder

Conversation

@fiona-gladwin
Copy link
Copy Markdown
Contributor

@fiona-gladwin fiona-gladwin commented Aug 13, 2025

Motivation

  • Adds fused crop decoder support using rocJpeg hardware decoder, used for Resnet50 trainings.
  • Fix minor bug wrt scaling images in the batch for rocJpeg decoder

Technical Details

  • Introduced rocJpeg fused crop decoder class with relevant members
  • Introduce rocal decoder_type variable to all the partial loader API's in C++ and python
  • Fix a bug in rocJpeg decoder when the images in the batch are scaled and modify stride accordingly

Test Plan

C++ Unit reader test case 14 - with GPU backend
Test added as part of testAllScript.sh

ROCDEC_VIDEO_DECODE = 5, //!< for video decoding using HW via rocDecode
AUDIO_SOFTWARE_DECODE = 6, //!< Uses sndfile to decode audio files
ROCJPEG_DEC = 7 //!< rocJpeg hardware decoder for decoding jpeg files
ROCJPEG_DEC = 7, //!< rocJpeg hardware decoder for decoding jpeg files
Copy link
Copy Markdown
Collaborator

@rrawther rrawther Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please rename to ROCJPEG for consistency

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will issue in a separate PR

Copy link
Copy Markdown
Collaborator

@rrawther rrawther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the review comments

@fiona-gladwin
Copy link
Copy Markdown
Contributor Author

@rrawther I have addressed other PR comments.
Changing the names for all other enums will results in more number of file changes i.e 10+
Please let me know if I can issue a separate PR for this enum name change

@fiona-gladwin fiona-gladwin requested a review from a team as a code owner August 26, 2025 09:49
@kiritigowda kiritigowda requested a review from Copilot August 27, 2025 16:59
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for fused crop decoding using the rocJpeg hardware decoder, specifically targeted for Resnet50 training workloads. The implementation introduces a new rocJpeg fused crop decoder class and integrates decoder type selection across the C++ and Python APIs.

  • Introduces FusedCropRocJpegDecoder class and ROCJPEG_CROPPED decoder type for hardware-accelerated crop decoding
  • Adds RocalDecoderType parameter to partial loader APIs to allow decoder selection between TurboJPEG and rocJpeg
  • Fixes stride calculation bug in rocJpeg decoder when images in the batch require scaling

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/cpp_api/unit_tests/unit_tests.cpp Updates test case 14 to use appropriate decoder based on GPU backend
rocAL_pybind/amd/rocal/decoders.py Adds device parameter and decoder type selection logic to image_slice function
rocAL/source/loaders/image/image_read_and_decode.cpp Extends decoder support and adds crop window handling for ROCJPEG_CROPPED type
rocAL/source/decoders/image/rocjpeg_fused_crop_decoder.cpp New implementation of fused crop decoder using rocJpeg hardware
rocAL/source/decoders/image/rocjpeg_decoder.cpp Moves shared utility functions to header and fixes stride calculation bug
rocAL/source/decoders/image/decoder_factory.cpp Adds factory support for ROCJPEG_CROPPED decoder type
rocAL/source/api/rocal_api_data_loaders.cpp Updates API functions to accept and handle decoder type parameter
rocAL/include/decoders/image/rocjpeg_fused_crop_decoder.h Header file defining the new fused crop decoder class interface
rocAL/include/decoders/image/rocjpeg_decoder.h Moves utility functions to header as inline functions and adds rescaling flag
rocAL/include/decoders/image/decoder.h Adds ROCJPEG_CROPPED enum value and removes duplicate includes
rocAL/include/api/rocal_api_data_loaders.h Updates API signatures to include decoder type parameter
docs/examples/notebooks/decoder_examples.ipynb Updates documentation to mention rocJPEG hardware decoder usage

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

_actual_decoded_width[i] = decoded_width;
_actual_decoded_height[i] = decoded_height;

if (_rocjpeg_decoder->is_partial_decoder()) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think, is_cropped_decoder is better name than is_partial_decoder (please change when changing decoder enums)

Copy link
Copy Markdown
Collaborator

@rrawther rrawther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am approving this PR assuming the review comments for name changes will be done as a separate PR

@rrawther
Copy link
Copy Markdown
Collaborator

rrawther commented Sep 2, 2025

@fiona-gladwin : Can you please resolve all conversations so we can merge this PR

@fiona-gladwin
Copy link
Copy Markdown
Contributor Author

@fiona-gladwin : Can you please resolve all conversations so we can merge this PR

@rrawther For the other comments wrt enum name change will issue a follow up PR. All comments are addressed

@kiritigowda kiritigowda merged commit 4143419 into ROCm:develop Sep 4, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants