[gdal-dev] [PR] New driver for reading/writing of JPEG XR format

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[gdal-dev] [PR] New driver for reading/writing of JPEG XR format

Mateusz Loskot
Hi,

I have just submitted GitHub pull request with new raster driver: JPEGXR
for reading/writing JPEG XR format by Microsoft.
Although the project started as an experiment, the driver is fairly usable.

https://github.com/OSGeo/gdal/pull/203

For more details, please refer to the PR linked above.

I'd highly appreciate reviews, corrections, testing results
as well as improvements to this driver.

Credits go to Frank Warmerdam who pointed me at the JPEG XR
format long time ago.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PR] New driver for reading/writing of JPEG XR format

Kurt Schwehr-2
Mateusz,

I tried to take a quick stab at some initial review comments.  It got confusing with too many comments in a small section of code.  Sorry.  I'm not used to reviewing in github.  Hopefully you can follow what I'm getting at with the comments.

-kurt

On Tue, Mar 14, 2017 at 4:37 PM, Mateusz Loskot <[hidden email]> wrote:
Hi,

I have just submitted GitHub pull request with new raster driver: JPEGXR
for reading/writing JPEG XR format by Microsoft.
Although the project started as an experiment, the driver is fairly usable.

https://github.com/OSGeo/gdal/pull/203

For more details, please refer to the PR linked above.

I'd highly appreciate reviews, corrections, testing results
as well as improvements to this driver.

Credits go to Frank Warmerdam who pointed me at the JPEG XR
format long time ago.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev



--

_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PR] New driver for reading/writing of JPEG XR format

Mateusz Loskot
On 15 March 2017 at 06:00, Kurt Schwehr <[hidden email]> wrote:
> Mateusz,
>
> I tried to take a quick stab at some initial review comments.  It got
> confusing with too many comments in a small section of code.  Sorry.  I'm
> not used to reviewing in github.  Hopefully you can follow what I'm getting
> at with the comments.

Kurt,

Although I'm familiar with GitHub PRs, comments and line comments [1],
I'm fairly new to the new (sort of) GitHub Reviews [1].
I think I will manage though.

I appreciate your detailed comments.


[1] https://help.github.com/articles/commenting-on-a-pull-request/
[2] https://github.com/blog/2256-a-whole-new-github-universe-announcing-new-tools-forums-and-features


Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net
_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PR] New driver for reading/writing of JPEG XR format

Mateusz Loskot
On 15 March 2017 at 21:21, Mateusz Loskot <[hidden email]> wrote:

> On 15 March 2017 at 06:00, Kurt Schwehr <[hidden email]> wrote:
>> Mateusz,
>>
>> I tried to take a quick stab at some initial review comments.  It got
>> confusing with too many comments in a small section of code.  Sorry.  I'm
>> not used to reviewing in github.  Hopefully you can follow what I'm getting
>> at with the comments.
>
> Kurt,
>
> Although I'm familiar with GitHub PRs, comments and line comments [1],
> I'm fairly new to the new (sort of) GitHub Reviews [1].
> I think I will manage though.
>
> I appreciate your detailed comments.
>
>
> [1] https://help.github.com/articles/commenting-on-a-pull-request/
> [2] https://github.com/blog/2256-a-whole-new-github-universe-announcing-new-tools-forums-and-features

One more thing I forgot to mention:

I usually tend to submit single-commit pull requests.
If I need to update one, I do `git push -f ...`.
That works well for small/moderate size of PRs.

In this case, I will follow GitHub recommendation [1] to update a PR
with separate/new commits, but not push-forcing.
For example, I'm about to update .travis.yml with libjxr-dev installation in
some of gdal/ci/travis/.../before_install.sh scripts, which I'm going to push
as a new commit.

Any objections to this approach?

[1] https://help.github.com/articles/about-pull-requests/

Best regardsm
--
Mateusz Loskot, http://mateusz.loskot.net
_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PR] New driver for reading/writing of JPEG XR format

Even Rouault-2

 

> In this case, I will follow GitHub recommendation [1] to update a PR

> with separate/new commits, but not push-forcing.

> For example, I'm about to update .travis.yml with libjxr-dev installation in

> some of gdal/ci/travis/.../before_install.sh scripts, which I'm going to

> push as a new commit.

>

> Any objections to this approach?

 

That's fine. Anyway, at some point, when this will go back to SVN, git PR history will be lost

 

Even

 

 

--

Spatialys - Geospatial professional services

http://www.spatialys.com


_______________________________________________
gdal-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/gdal-dev
Loading...