GNOME Bugzilla – Bug 743681
Add Instagram filters
Last modified: 2016-03-02 17:23:52 UTC
While reading Jon's blog, I realized that people have been reverse engineering Instagram filters. It appears to me that the ones in imgflo-server [2] are based on noflo-image. So, why don't we just add them as native GEGL operations? They would be pretty cool to have in gnome-photos, for example. [1] http://www.jonnor.com/2015/01/imgflo-0-3/ [2] https://github.com/jonnor/imgflo-server/tree/master/graphs
Created attachment 295731 [details] [review] Add Instagram filters - 1977 and Brannan I started with the simpler ones. I have a question, though. The filters in the following two places are not exact, even though the later claims to be based on the former: - http://matthewruddy.github.io/jQuery-filter.me/ - https://github.com/jonnor/imgflo-server/tree/master/graphs The adjustment curves are the same, but while Brannan in imgflo applies desaturation, the other one doesn't. Also the desaturation algorithms appear slightly different.
Created attachment 295740 [details] [review] Add Instagram filters - 1977 and Brannan
Created attachment 295811 [details] [review] Add Instagram filters - 1977 and Brannan Added linear interpolation to 1977 for higher bit depth images.
Created attachment 296300 [details] [review] Add Instagram filters Reorganized the code to make things a bit more generic and parametrized. Now we have gegl:insta-filter meta operation which takes an enum for selecting the filter. It creates a sub-graph made up of gegl:insta-curve, which applies the preset curve, and other operations. Added two more filters - Gotham and Nashville.
Review of attachment 296300 [details] [review]: For the meta-op, I was kind-of expecting it to be implemented by having one op per filter, and then swapping these when the enum changes instead of just the process() function.But I guess I don't see any particular disadvantage to this approach. And if there are, should be change-able later anyways.
Review of attachment 296300 [details] [review]: Nice that it provides both f32 and u8 processing. Wrong filename in POTFILES.in? operations/workshop/insta-brannan.c Also, I don't think Brannan, 1977, Gotham, Nashville should be marked translatable. These are after all proper-nounds, like 'GEGL' etc. Tested that the code builds,but don't have time to run/check visuals now. Fix the translation issues (can be separate commit) and its good to push to master I think.
Swapping out the process vfunc in prepare isn't re-entrant, multiple different instances in one graph - or different instances in different threads might give racy/unpredictable results.
Created attachment 320695 [details] [review] Add Instagram filters
(In reply to Jon Nordby from comment #6) > Review of attachment 296300 [details] [review] [review]: > > Nice that it provides both f32 and u8 processing. > > Wrong filename in POTFILES.in? operations/workshop/insta-brannan.c > Also, I don't think Brannan, 1977, Gotham, Nashville should be marked > translatable. > These are after all proper-nounds, like 'GEGL' etc. Fixed.
(In reply to Øyvind Kolås from comment #7) > Swapping out the process vfunc in prepare isn't re-entrant, multiple > different instances in one graph - or different instances in different > threads might give racy/unpredictable results. Of course, you are right. Setting a class-wide variable based on an instance-level parameter is obviously wrong. Absolutely stupid mistake on my part.
One thing that I added to gnome-photos' copy of the operation is a NONE preset. It is convenient to have it because often the UI would be a series of buttons, one for each filter, and one without any effect. Having a NONE preset makes the application code even more generic.
Comment on attachment 320695 [details] [review] Add Instagram filters commit aeb38236ce9a374508c76e476c180f2803f2db2a Author: Debarshi Ray <debarshir@gnome.org> Date: Thu Jan 29 08:08:29 2015 +0100 Add Instagram filters Currently 1977, Brannan, Gotham and Nashville are implemented. The filters are provided by a meta-operation called gegl:insta-filter which takes an enum specifying which one to apply. The hidden gegl:insta-curve operation has the preset curves in terms of look-up tables. Based on: http://matthewruddy.github.io/jQuery-filter.me/ https://github.com/jonnor/imgflo-server/tree/master/graphs https://bugzilla.gnome.org/show_bug.cgi?id=743681
I will now attach a patch for Hefe.
Created attachment 321655 [details] [review] insta-filter, insta-curve: Add a NONE preset
Created attachment 321656 [details] [review] perf: Test gegl:insta-filter performance
commit 17bf9eb9f6499d761ca69aeff56e65e5089cc036 Author: Debarshi Ray <debarshir@gnome.org> Date: Fri Feb 19 13:38:47 2016 +0100 insta-filter, insta-curve: Add a NONE preset ... and make it the default, instead of 1977. https://bugzilla.gnome.org/show_bug.cgi?id=743681
Created attachment 321911 [details] [review] operations/workshop/insta-curve: Fix crash while building
Comment on attachment 321911 [details] [review] operations/workshop/insta-curve: Fix crash while building From #gegl on GIMPNet: 13:03 <rishi> pippin: What do you think of https://bugzilla.gnome.org/attachment.cgi?id=321911 ? 13:06 <pippin> setting properties shouldn't make things segfault.. so yes - sure
commit 7080e5aa86d0c885a9ee307813bddc4e320b0cc5 Author: Debarshi Ray <debarshir@gnome.org> Date: Tue Feb 23 02:27:25 2016 +0100 operations/workshop/insta-curve: Fix crash while building Even though there is no reason for an application to call gegl:insta-curve with the NONE preset, the build does do that which triggers the assertion. Therefore, let's make it work. https://bugzilla.gnome.org/show_bug.cgi?id=743681
Created attachment 322838 [details] [review] Rename insta-curve and insta-filter to retro-curve and retro-filter
renaming to retro fix applied, more presets not inspired by the instagram presets might also be desirable.
Reverting inclusion due to potential legal concerns; might be reincluded if backed by affirmative legal advice. commit 6dfd40264ba9c8187ff8ef8396707aced5216b33 Author: Øyvind Kolås <pippin@gimp.org> Date: Wed Mar 2 12:27:09 2016 +0100 move retro-curve/-filter out of workshop commit fce98ae5c7415c1b4c509b509a24a808bf2a4b75 Author: Øyvind Kolås <pippin@gimp.org> Date: Wed Mar 2 13:39:30 2016 +0100 remove retro look filter presets
Has there been legal advice to not include it? What are the 'potential legal concerns' based on? Copyright? Trademark? Patent?
Concerns about duplicating behavior and possibly names for referring to that behavior raised with the GNOME board has not produced sufficiently qualified legal advice to feel confident about not provoking legal problem if the functionality gets exposed in high profile GEGL applications like GIMP. Using other community based reverse engineering/synthesis of film stock emulation and vignetting / framing / scuffing seems safer and less problematic for exposing similar one-shot retro feel presets than relying on the reverse engineering of the filters of one particular popular smart phone photo app.
@Jon Nordby: so to more directly respond - at least trademark, possibly copyright - probably not patent.