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

Remove redundant JPEG decoder initialization from peeking shape function #2610

Merged
merged 1 commit into from
Jan 14, 2021

Conversation

JanuszL
Copy link
Contributor

@JanuszL JanuszL commented Jan 12, 2021

  • in the case of progressive JPEG decoder initialization is very expensive and not needed for image shape inference

Signed-off-by: Janusz Lisiecki jlisiecki@nvidia.com

Why we need this PR?

Pick one, remove the rest

  • It improves PeekImageShape operator performance for progressive JPEGs

What happened in this PR?

Fill relevant points, put NA otherwise. Replace anything inside []

  • What solution was applied:
    Removes redundant JPEG decoder initialization from peeking shape function as in the case of progressive JPEG decoder initialization is very expensive and not needed for image shape inference
  • Affected modules and functionalities:
    PeekImageShape operator
  • Key points relevant for the review:
    NA
  • Validation and testing:
    present test applies
  • Documentation (including examples):
    NA

JIRA TASK: [NA]

- in the case of progressive JPEG decoder initialization is very expensive
  and not needed for image shape inference

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL
Copy link
Contributor Author

JanuszL commented Jan 12, 2021

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1975574]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1975574]: BUILD PASSED

@@ -593,10 +593,9 @@ bool GetImageInfo(const void* srcdata, int datasize, int* width, int* height,
SetSrc(&cinfo, srcdata, datasize, false);

DALI_ENFORCE(jpeg_read_header(&cinfo, TRUE) == JPEG_HEADER_OK);
DALI_ENFORCE(jpeg_start_decompress(&cinfo)); // required to transfer image size to cinfo
Copy link
Contributor

Choose a reason for hiding this comment

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

the original code says that this is required to transfer the image size to cinfo. Are we sure it is not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was required to populate output_width, output_height and output_components, but is not needed for image_width, image_height and num_components. (Operator tests still passes).

@JanuszL JanuszL merged commit 85f38a5 into NVIDIA:master Jan 14, 2021
@JanuszL JanuszL deleted the accelerate_peek_shape branch January 14, 2021 08:24
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.

None yet

4 participants