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

JP2Grok: new Grok JPEG 2000 driver based on Grok JPEG 2000 Library #10294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boxerab
Copy link
Contributor

@boxerab boxerab commented Jun 25, 2024

Let's try this again !

What does this PR do?

This PR adds a new driver for the JPEG 2000 format using Grok library.

Design Strategy

In a previous merged PR, common features supporting both OpenJPEG and Grok libraries were refactored into a template driver named JP2DatasetBase. This PR creates a new Grok driver inheriting from JP2DatasetBase.

Performance

Basic performance testing of gdal_translate with default settings on a large HiRISE image shows this driver outperforming the existing open source driver by 6 times.

What are related issues/pull requests?

#3449
#5117
#8133

Tasks

  • Support encode (currently disabled)
  • Address issues raised by @rouault in previous Grok PRs
  • Add test case(s)
  • Add documentation
  • Updated Python API documentation (swig/include/python/docs/)
  • Review
  • Adjust for comments
  • All CI builds and checks have passed
@@ -0,0 +1,552 @@

.. _raster.JP2Grok:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. _raster.JP2Grok:
.. _raster.jp2grok:


This driver is an implementation of a JPEG2000 reader/writer based on
Grok library **v2**.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Grok library **v2**.
Grok library.
@rouault
Copy link
Member

rouault commented Jun 25, 2024

@boxerab boxerab changed the title JPGrok: new Grok JPEG 2000 driver based on Grok JPEG 2000 Library Jun 25, 2024
@rouault
Copy link
Member

rouault commented Jun 26, 2024

@boxerab Just looking at https://github.com/GrokImageCompression/grok/commits/master/ , I'm puzzled to see that Grok git history has been completely reset, and that the released packages are only the latest one. This isn't super re-assuring for audit purposes, especially after the XZ story.

@boxerab
Copy link
Contributor Author

boxerab commented Jun 27, 2024

@boxerab Just looking at https://github.com/GrokImageCompression/grok/commits/master/ , I'm puzzled to see that Grok git history has been completely reset, and that the released packages are only the latest one. This isn't super re-assuring for audit purposes, especially after the XZ story.

@rouault the situation is not really analogous - no majour tools such as openssh rely on Grok, and the XZ Utils backdoor was not found via audit or code review, but rather by sheer luck.

The library has entered its maintenance phase - feature complete, stable and performant. The only changes I anticipate are bug fixes, but we haven't had many bugs for the past couple of years. The history has been reset, issues tracker has been cleared and only the latest release will be hosted on github.

The code continues to pass all unit tests and the OSS-Fuzz fuzzers continue to run. Anyone is free to audit the code,
and to report issues.

@rouault
Copy link
Member

rouault commented Jun 27, 2024

Anyone is free to audit the code

good luck when you have zero history... Was there something in the history that wasn't appropriate and had to be removed? But in that case there are less drastic ways of removing a commit than just erasing the whole history, which is an important asset of open source projects. I'm not the only one to find that there's something fishy here: https://mastodon.social/@EvenRouault/112687807835976568 . I can't name a project that has reset their history like that. This is an important asset to understand the rationale of parts of the code and having an audit trail. My 2 cents if there's nothing to hide and you've kept a copy of your initial git repo before the big reset is that you re-push it, and re apply the later modifications on top of if

@boxerab
Copy link
Contributor Author

boxerab commented Jun 28, 2024

Yes, a few people weren't happy about the changes. Others are not happy with the AGPL license.
They are free to use other open source or commercial offerings.
As many continue to benefit from the project, it continues on its current path.

@jratike80
Copy link
Collaborator

jratike80 commented Jun 28, 2024

As many continue to benefit from the project, it continues on its current path.

That applies to GDAL as well. We need to consider if and how our paths will meet.

@boxerab
Copy link
Contributor Author

boxerab commented Jun 28, 2024

That applies to GDAL as well. We need to consider if and how our paths will meet.

Indeed. I will note that GDAL has drivers for completely opaque black box software such as ERDAS ECW/JP2 SDK,
while every line of a Grok release can be scrutinized by anyone, although I doubt anyone outside of the project will ever do so.

@rouault
Copy link
Member

rouault commented Jun 28, 2024

I will note that GDAL has drivers for completely opaque black box software such as ERDAS ECW/JP2 SDK

indeed, that's what we mentioned during yesterday's monthly GDAL maintainer meeting. Given that we won't ship Grok itself as part of deliverables of the GDAL project, this isn't a blocking point for us to consider including the source code of the GDAL driver part. This is the responsibility of the end user to decide whether it fits their criteria

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants