Skip to content
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

jpeg: add support for crop and partial decode #721

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

sathishm8
Copy link

add interface to support region-of-interest jpeg decode

Copy link
Contributor

@XinfengZhang XinfengZhang left a comment

Choose a reason for hiding this comment

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

dumb question:
why it was needed in decode side?
from my understanding, we could give more bit budget in the ROI areas during encoding
but how to use it for decoding? do some extra post processing?

@sathishm8
Copy link
Author

dumb question: why it was needed in decode side? from my understanding, we could give more bit budget in the ROI areas during encoding but how to use it for decoding? do some extra post processing?

Certain hardware support this feature to decode only the region of interest specified, which helps to finish the decode in lesser time and with lower memory usage, to support this we need vaapi interface to propogate the region of interest window to hardware decoder. Else the image has to be fully decoded into an appropriate render target (full size, higher decode time and larger memory) and later crop the required window (into another surface that is crop window size) and then pass it to further post processing stages.

va/va.h Outdated Show resolved Hide resolved
@sathishm8
Copy link
Author

Hi @XinfengZhang , pushed the updated version with the changes suggested. please review the update.

va/va_dec_jpeg.h Outdated
@@ -75,8 +75,10 @@ typedef struct _VAPictureParameterBufferJPEGBaseline {
uint8_t color_space;
/** \brief Set to VA_ROTATION_* for a single rotation angle reported by VAConfigAttribDecJPEG. */
uint32_t rotation;
/** \brief region-of-interest boundary in pixels */
VARectangle roi_rectangle;
Copy link
Contributor

Choose a reason for hiding this comment

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

block boundary? or pixel boundary? also, zero means no such feature enabled?

Copy link
Author

Choose a reason for hiding this comment

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

it is the ROI rectangle boundary in pixels , zero is to be ignored and considered as no ROI decode request and process complete image decode. And it is single rectangle ROI support, no support for multiple ROI rectangles.

Copy link
Author

Choose a reason for hiding this comment

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

to check if feature is enabled or not VAConfigAttribValDecJPEG roi bit field is to be used from applications.

when roi_rectangle.width and roi_rectangle.height are non zero then checks are are to be performed to make sure the rectangle is within the full image resolution and the rectangle itself is aligned to macroblock size (in mesa frontend va etc).

if roi_rectangle.width and roi_rectangle.height are 0 (in ROI supported HW only), then it means full image decode is requested,

on HW that dont support ROI if non zero roi_rectangle values are passed then can return not supported error code.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, but still have some questions:

  1. sound like it is something like "crop" ?, or jpeg spec support decode from any block?
  2. how about the output surfaces? if I create a full size surface, it means only part data in rectangle is filled at the left top corner?

btw: which platform support this feature?

Copy link
Author

Choose a reason for hiding this comment

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

  1. Yes you are right it is like crop or decode of any specific block defined by roi_rectangle.
  2. Application is supposed to allocate output surface with resolution that exactly match the roi_rectangle it requested for decode. Output surface properties (resolution) will be validated across the roi_rectangle definition to make sure the output surface is the right size for the requested roi_rectangle.

Upcoming AMD platform supports this feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Application is supposed to allocate output surface with resolution that exactly match the roi_rectangle it requested for decode. Output surface properties (resolution) will be validated across the roi_rectangle definition to make sure the output surface is the right size for the requested roi_rectangle.

it is hard request or recommended one? my question is: any resolution bigger than roi_rectangle is ok to be output?

Copy link
Author

Choose a reason for hiding this comment

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

it is a hard request that surface size has to match with actual roi_rectangle request size , bigger resolution of surface will create surface pitch related issues.

Copy link
Author

Choose a reason for hiding this comment

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

Also hard request because, exact surface size (matching roi_rectangle) is the render target which can be submitted to renderer/display or submit to post process on gpu, bigger size surface will not be right choice for this pupose as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

from the libva perspective, there are 3 ways to achieve similar goal.

  1. decode call + vpp(crop) call
  2. decode call + VAConfigAttribDecProcessing in one call
  3. this interface for jpeg only, rectangle should be aligned with block size (8x8), but the call itself is simple.

this PR LGTM, maybe using different name "crop_xxx" is better?

Copy link
Author

Choose a reason for hiding this comment

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

okay, thank you for explaining and summarizing.

Sure, will change the bitfield variable as "crop" and the VARectangle variable as "crop_rectangle" ?
and will appropriately change the commit message also to

jpeg: add support for crop and partial decode

add interface to support jpeg crop and partial decode.

v2: remove redundant macro for bit position and use crop bit-field (Carl Zhang)
v3: rename variables to crop instead of roi (Carl Zhang)

Signed-off-by: Sathishkumar S <[email protected]>
@sathishm8 sathishm8 changed the title jpeg: add region of interest decode support jpeg: add support for crop and partial decode Aug 21, 2023
@XinfengZhang XinfengZhang merged commit b4870fd into intel:master Aug 24, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants