search

briandonahue / FluxJpeg.Core

Clone of fjcore library that seems to be stagant on google code
http://code.google.com/p/fjcore/
20 stars 10 forks source link

Two simple fixes in FluxJpeg #5

Closed Ericvf closed 10 years ago

Ericvf commented 10 years ago

Hi -

I've forked this project and committed a few fixes that I needed for our customer project. I originally forked to googlecode project and noticed that the developers using the library had the same difficulties we did.

The first problem we encountered was that the density properties can be updated but never make it into the output file when re-encoding the image. JpegEncoder never touches the JFIF or EXIF headers that contain this info.

The second problem was that hierarchical Jpeg's weren't supported. Luckily the library already knew about this problem and threw an exception.

Both of these issues have been reported in the original google repository and I was surprised that they weren't fixed 5 years later. After changing the code for our customer project I found out about the GitHub repository and decided to fork this one.

This pull request revolves around these two problems. I had to create GitHub account for this.

briandonahue commented 10 years ago

@Ericvf Thanks for working on this! To be perfectly honest, my needs for this library were pretty simplistic, and my contributions pretty minor. I don't fully understand the changes you made, though I'd assume you understand the library better than I do, to have made them! The only comments I have is that it seems you removed a lot of the ReadMe text, which describe the project, and now they just describe your fork. The Readme should describe the overall purpose and features of the library, though you are welcome to add yourself as a contributor and describe your contributions. If @anders9ustafsson want to take a look, maybe he will review and approve the merge?

Thanks again!

Brian

anders9ustafsson commented 10 years ago

Many thanks @Ericvf for sharing your fixes and improvements, it is of great value to the project.

Unfortunately I will not have time to review the pull request in detail within the next two weeks since I will be traveling, but I will look at it as soon as I am back.

The main reason why very little has happened in the last 5 years is that the original author of the code has not had the time or interest to maintain it. My main interest in this project has been to provide a Portable Class Library version of the library, and if I understand correctly @briandonahue , you primarily wanted to bring the library to NuGet, right? Thus, we are very happy to see that someone has taken the time and effort to improve and extend on the library.

By the way, @Ericvf , are your updates PCL compliant, i.e. is it still possible to build the PCL library? If not, I would be very thankful if you could spend some effort to modify your updates to be as PCL compliant as possible.

Best regards, Anders @ Cureos

Ericvf commented 10 years ago

Thanks for the response. I hope you have time to review the changes since they are very minor. I did notice that my visual studio file comparer does a much better job than GitHub. The GitHub view of my changes in JpegDecoder.cs are a good example.

My changes are quite simple, no new code has been introduced and the portable library still builds. However I have not tested it in a WinRT project.

Ericvf commented 10 years ago

ps @briandonahue;

I did a pull request with my changes and afterwards I modified the readme.md file. Now it seems that those additional commits are also being pulled. Those updates were intended for my fork alone. I wrote on the readme that the fork is made to be able to merge my fixes with the master. Clearly I am new to GitHub.

Can I exclude these commits from my pull request?

briandonahue commented 10 years ago

@Ericvf I don't think it's typical to edit the readme in a fork, but maybe I just haven't seen it often. I think the only way to do this would be to make the readme change in your fork's master, and make the pull request changes in a separate branch, created from the original master, then create a pull request from that branch to our master. We could also probably just revert your commit prior to merging, as well.

@anders9ustafsson I originally found this library when I need to resize images in Silverlight on the client before uploading to the server. I know very little about image encoding, but I forked the repository to make a few simple changes, such as adding proportional resizing method and wiring up the progress events so that I could show progress in my UI. That's why I don't really feel qualified to review any changes made to the encoding stuff!

As for the nuget package, I actually got a request as an Issue #1 so put it together then. I haven't really needed this library in years - I've long since replaced my silverlight client! :)

anders9ustafsson commented 10 years ago

@briandonahue I am about to take a closer look at this pull request now, but I don't seem to have the privileges to merge it. If I am not mistaken, you previously added "cureos" as a collaborator, but I have since then turned "cureos" into an organization and created a new individual user for myself. Do you think you can add "anders9ustafsson" as a collaborator as well?

Many thanks in advance! Anders

briandonahue commented 10 years ago

@anders9ustafsson You are added as a collaborator!

anders9ustafsson commented 10 years ago

@briandonahue @Ericvf I have now merged the pull request and published an updated release on NuGet. Many thanks, @Ericvf, for this important contribution to the library.