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

Add JpegCompressionDistortion CPU and GPU operators #2823

Merged
merged 7 commits into from
Apr 13, 2021

Conversation

jantonguirao
Copy link
Contributor

@jantonguirao jantonguirao commented Mar 29, 2021

Signed-off-by: Joaquin Anton [email protected]

Why we need this PR?

Pick one, remove the rest

  • It adds a new operator needed to generate JPEG-like distortion

What happened in this PR?

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

  • What solution was applied:
    Added a JpegCompressionDistortion operator, both GPU and CPU implementations.
    The CPU version uses OpenCV imencode/imdecode
    The GPU version uses a custom CUDA kernel
  • Affected modules and functionalities:
    New operator
  • Key points relevant for the review:
    All
  • Validation and testing:
    Python tests added
  • Documentation (including examples):
    Docstr

JIRA TASK: [DALI-1932] [DALI-1941]

.AddOptionalArg("quality",
R"code(JPEG compression quality from 1 (lowest quality) to 95 (highest quality).

Any values outside the range 1-99 will be clamped.)code",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Any values outside the range 1-99 will be clamped.)code",
Any values outside the range 1-95 will be clamped.)code",

?

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 actually the other way around. I fixed it now

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that you can set quality 100 in many graphics editors. Also, I predict that having this parameter as float would be more convenient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both libjpeg and OpenCV use integer for the quality factor. I will check if 100 makes sense in the current formula.

@jantonguirao jantonguirao changed the title [WIP] Add JpegCompressionDistortion operator Add JpegCompressionDistortion operator Mar 29, 2021
Comment on lines 70 to 67
cv::Mat in_mat(sh[0], sh[1], CV_8UC3, (void*) in_view[sample_idx].data); // NOLINT
cv::Mat out_mat(sh[0], sh[1], CV_8UC3, (void*) out_view[sample_idx].data); // NOLINT
Copy link
Contributor

@mzient mzient Mar 29, 2021

Choose a reason for hiding this comment

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

Wouldn't that work?

Suggested change
cv::Mat in_mat(sh[0], sh[1], CV_8UC3, (void*) in_view[sample_idx].data); // NOLINT
cv::Mat out_mat(sh[0], sh[1], CV_8UC3, (void*) out_view[sample_idx].data); // NOLINT
cv::Mat in_mat(sh[0], sh[1], CV_8UC3, in_view[sample_idx].data);
cv::Mat out_mat(sh[0], sh[1], CV_8UC3, out_view[sample_idx].data);

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 doesn't work (there are other overloads)

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't there a problem with the fact that our shape has int64 extents? What about this?

Suggested change
cv::Mat in_mat(sh[0], sh[1], CV_8UC3, (void*) in_view[sample_idx].data); // NOLINT
cv::Mat out_mat(sh[0], sh[1], CV_8UC3, (void*) out_view[sample_idx].data); // NOLINT
int h = sh[0], w = sh[1];
cv::Mat in_mat(h, w, CV_8UC3, in_view[sample_idx].data);
cv::Mat out_mat(h, w, CV_8UC3, out_view[sample_idx].data);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main problem seems to be:

jpeg_compression_distortion_op_cpu.cc:69:59: error: invalid conversion from ‘const void*’ to ‘void*’

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is const - use const_cast<void*> then (or const_cast<uint8_t*> or whatever our format is).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@klecki klecki marked this pull request as draft March 30, 2021 09:42
@jantonguirao jantonguirao changed the title Add JpegCompressionDistortion operator Add JpegCompressionDistortion CPU and GPU operators Mar 30, 2021
@jantonguirao jantonguirao force-pushed the jpeg_distortion_op_gpu branch 2 times, most recently from 4b1e530 to 32b82b0 Compare April 2, 2021 12:44
@jantonguirao jantonguirao marked this pull request as ready for review April 6, 2021 10:00
@jantonguirao jantonguirao assigned mzient and unassigned awolant Apr 9, 2021
Signed-off-by: Joaquin Anton <[email protected]>
Signed-off-by: Joaquin Anton <[email protected]>
Signed-off-by: Joaquin Anton <[email protected]>
Comment on lines 22 to 25
.DocStr(R"code(Produces JPEG-like distortion in RGB images.

The level of degradation of the image can be controlled with the ``quality`` argument,
)code")
Copy link
Member

Choose a reason for hiding this comment

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

IMHO, there should be a clear explanation, what is a "JPEG-like distortion"

Copy link
Contributor

@mzient mzient Apr 12, 2021

Choose a reason for hiding this comment

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

Suggested change
.DocStr(R"code(Produces JPEG-like distortion in RGB images.
The level of degradation of the image can be controlled with the ``quality`` argument,
)code")
.DocStr(R"code(Introduces JPEG compression artifacts to RGB images.
JPEG is a lossy compression format which exploits characteristics of natural images and human visual system to achieve high compression ratios. The information loss originates from sampling the color information at a lower spatial resolution than the brightness and from representing high frequency components of the image with a lower effective bit depth. The conversion to frequency domain and quantization is applied independently to 8x8 pixel blocks, which introduces additional artifacts at block boundaries.
This operation produces images by subjecting the input to a transformation that mimics JPEG compression with given ``quality`` factor followed by decompression .
)code")

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Signed-off-by: Joaquin Anton <[email protected]>
@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2260239]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2260239]: BUILD PASSED

R"code(JPEG compression quality from 1 (lowest quality) to 100 (highest quality).

Any values outside the range 1-100 will be clamped.)code",
95, true);
Copy link
Contributor

@mzient mzient Apr 12, 2021

Choose a reason for hiding this comment

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

It's a nitpick, really, but I think that a more realistic value would be 90. 80 is usually considered quite poor, 95 typically requires considerable magnification to see any distortion. Perhaps there shouldn't be any default value?

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's the default quality used in OpenCV and libjpeg-turbo, I believe.

Signed-off-by: Joaquin Anton <[email protected]>
Signed-off-by: Joaquin Anton <[email protected]>
@jantonguirao
Copy link
Contributor Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2263323]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [2263323]: BUILD PASSED

@jantonguirao jantonguirao merged commit 9158265 into NVIDIA:master Apr 13, 2021
@JanuszL JanuszL mentioned this pull request May 19, 2021
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.

6 participants