Re: Review Request 106300: add kimgio WebP image format plugin
On Sunday 16 June 2013 01:13:13 Christoph Feck wrote: On May 18, 2013, 12:44 p.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Christoph Feck wrote: Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? Albert Astals Cid wrote: If you addressed Allan's comment i'd sincerely put it where it belongs, which is obviously not kolourpaint :D This still is only in kolourpaint, which is unfortunate. Bug 267365 is a KDE general feature request. People who do not install kolourpaint will be left out. Additionally, having it installed will make it work with other Qt programs (Gwenview etc.), but not the thumbnailer. I cannot add the mime type to the thumbnailer, if the plugin is only in kolourpaint. AFAIK, it was decided to move it to kde-runtime. I created a new review request for kde-runtime: http://git.reviewboard.kde.org/r/110649/ I hopefully solved the last open comment from Fredrik Höglund today, so I'm waiting now for the final ship it and hope that this move from kolourpaint to kde-runtime isn't now too late for 4.11 -- Best regards/Schöne Grüße Martin A: Because it breaks the logical sequence of discussion Q: Why is top posting bad? () ascii ribbon campaign - against html e-mail /\ www.asciiribbon.org - against proprietary attachments Geschenkideen, Accessoires, Seifen, Kulinarisches: www.bibibest.at
Re: Review Request 106300: add kimgio WebP image format plugin
On May 18, 2013, 8:44 a.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Christoph Feck wrote: Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? Albert Astals Cid wrote: If you addressed Allan's comment i'd sincerely put it where it belongs, which is obviously not kolourpaint :D Christoph Feck wrote: This still is only in kolourpaint, which is unfortunate. Bug 267365 is a KDE general feature request. People who do not install kolourpaint will be left out. Additionally, having it installed will make it work with other Qt programs (Gwenview etc.), but not the thumbnailer. I cannot add the mime type to the thumbnailer, if the plugin is only in kolourpaint. AFAIK, it was decided to move it to kde-runtime. See https://git.reviewboard.kde.org/r/110649/ for move status. - Michael --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review32734 --- On May 26, 2013, 8:12 a.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 26, 2013, 8:12 a.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
On May 18, 2013, 12:44 p.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Christoph Feck wrote: Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? Albert Astals Cid wrote: If you addressed Allan's comment i'd sincerely put it where it belongs, which is obviously not kolourpaint :D This still is only in kolourpaint, which is unfortunate. Bug 267365 is a KDE general feature request. People who do not install kolourpaint will be left out. Additionally, having it installed will make it work with other Qt programs (Gwenview etc.), but not the thumbnailer. I cannot add the mime type to the thumbnailer, if the plugin is only in kolourpaint. AFAIK, it was decided to move it to kde-runtime. - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review32734 --- On May 26, 2013, 12:12 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 26, 2013, 12:12 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 26, 2013, 12:12 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
El Dijous, 23 de maig de 2013, a les 09:47:36, Martin Koller va escriure: On Wednesday 22 May 2013 22:15:56 Albert Astals Cid wrote: On May 18, 2013, 12:44 p.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Christoph Feck wrote: Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? If you addressed Allan's comment i'd sincerely put it where it belongs, which is obviously not kolourpaint :D Where does it belong to, as kdelibs is feature-frozen ? To kde-runtime. There's no reason a image format plugin should be in kdelibs anyway since kde- runtime is a mandatory requirement for running any kdelibs based app. Cheers, Albert
Re: Review Request 106300: add kimgio WebP image format plugin
On Wednesday 22 May 2013 22:15:56 Albert Astals Cid wrote: On May 18, 2013, 12:44 p.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Christoph Feck wrote: Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? If you addressed Allan's comment i'd sincerely put it where it belongs, which is obviously not kolourpaint :D Where does it belong to, as kdelibs is feature-frozen ? -- Best regards/Schöne Grüße Martin A: Because it breaks the logical sequence of discussion Q: Why is top posting bad? () ascii ribbon campaign - against html e-mail /\ www.asciiribbon.org - against proprietary attachments Geschenkideen, Accessoires, Seifen, Kulinarisches: www.bibibest.at
Re: Review Request 106300: add kimgio WebP image format plugin
On Tuesday 21 May 2013 21:34:23 Christoph Feck wrote: On May 18, 2013, 12:44 p.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? Probably. I sent again an email to kde-core-devel on May 11th, which was ignored, so I thought nobody cared. As the deadline for 4.11 approached, I decided to commit this to kolourpaint, which I'm the maintainer of, to at least have it in 4.11 - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review32734 --- On May 18, 2013, 12:44 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 18, 2013, 12:44 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller -- Best regards/Schöne Grüße Martin A: Because it breaks the logical sequence of discussion Q: Why is top posting bad? () ascii ribbon campaign - against html e-mail /\ www.asciiribbon.org - against proprietary attachments Geschenkideen, Accessoires, Seifen, Kulinarisches: www.bibibest.at
Re: Review Request 106300: add kimgio WebP image format plugin
On May 18, 2013, 12:44 p.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Christoph Feck wrote: Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? If you addressed Allan's comment i'd sincerely put it where it belongs, which is obviously not kolourpaint :D - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review32734 --- On May 18, 2013, 12:44 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 18, 2013, 12:44 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
On May 18, 2013, 12:44 p.m., Commit Hook wrote: This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. Just noticed this has been committed to kolourpaint instead of kdelibs. Did I miss a discussion about this change? - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review32734 --- On May 18, 2013, 12:44 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 18, 2013, 12:44 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review32734 --- This review has been submitted with commit df845b06bc440fff5a43a9cdc149e8c88c29bc67 by Martin Koller to branch master. - Commit Hook On May 9, 2013, 6:20 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 9, 2013, 6:20 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 18, 2013, 12:44 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated May 9, 2013, 6:20 p.m.) Review request for kdelibs. Changes --- added answers to comments and added Size option Description (updated) --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs (updated) - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
On Sept. 10, 2012, 10:48 p.m., Allan Sandfeld Jensen wrote: kimgio/webp.cpp, lines 67-73 http://git.reviewboard.kde.org/r/106300/diff/2/?file=82722#file82722line67 This seems needlessly explicit, why not just copy the whole block into QImage? QImage::scanLine docu says the pixel format depends on the byte order on the underlying platform. So to be sure to create correct rgba tuples, I explicitely pass the values to qRgba/qRgb On Sept. 10, 2012, 10:48 p.m., Allan Sandfeld Jensen wrote: kimgio/webp.cpp, lines 92-100 http://git.reviewboard.kde.org/r/106300/diff/2/?file=82722#file82722line92 Same as for decoding. Why copy it color by color like this, when the formats are identical? Same answer as above: scanLine data depends on the machines byte order, but WebP has always the same data representation On Sept. 10, 2012, 10:48 p.m., Allan Sandfeld Jensen wrote: kimgio/webp.cpp, line 127 http://git.reviewboard.kde.org/r/106300/diff/2/?file=82722#file82722line127 Not really required, but it would be nice to support the Size option. It is quite often used, and should be available through WebPGetInfo() ok, added On Sept. 10, 2012, 10:48 p.m., Allan Sandfeld Jensen wrote: kimgio/webp.cpp, line 159 http://git.reviewboard.kde.org/r/106300/diff/2/?file=82722#file82722line159 Should it also require VP8 in byte 12 to 16, to protect against new unsupported versions? For some reason I have code that checks the file is atleast 32 bytes long, I think that is because it is what WePGetInfo requires. not really. The spec says that the WebP header is 12 bytes and contains what I check. There is an Extended file format which starts with a ‘VP8X’ chunk (can currently contain VP8 or VP8L) but as I do not interpret that VP8 data, why restrict the reader to it? Maybe a future libwebp supports something else than VP8 after the WebP Header ... - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review18829 --- On Sept. 1, 2012, 9:23 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated Sept. 1, 2012, 9:23 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru The patch is missing a cmake rule to make it optional, though, but I sent a mail to KDE-core list in the hope someone knows... This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION mimetypes/kde.xml a82b87c Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
On April 10, 2013, 10:52 p.m., Albert Astals Cid wrote: Martin, can you clarify Allan's comments? done - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review30892 --- On Sept. 1, 2012, 9:23 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated Sept. 1, 2012, 9:23 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru The patch is missing a cmake rule to make it optional, though, but I sent a mail to KDE-core list in the hope someone knows... This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION mimetypes/kde.xml a82b87c Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
Re: Review Request 106300: add kimgio WebP image format plugin
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/#review30892 --- Martin, can you clarify Allan's comments? - Albert Astals Cid On Sept. 1, 2012, 9:23 p.m., Martin Koller wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106300/ --- (Updated Sept. 1, 2012, 9:23 p.m.) Review request for kdelibs. Description --- This patch adds support for the new Google invented WebP image format. See https://developers.google.com/speed/webp/?hl=ru The patch is missing a cmake rule to make it optional, though, but I sent a mail to KDE-core list in the hope someone knows... This addresses bug 267365. http://bugs.kde.org/show_bug.cgi?id=267365 Diffs - kimgio/CMakeLists.txt 26329c0 kimgio/webp.cpp PRE-CREATION kimgio/webp.desktop PRE-CREATION kimgio/webp.h PRE-CREATION mimetypes/kde.xml a82b87c Diff: http://git.reviewboard.kde.org/r/106300/diff/ Testing --- some KDE apps, including read/write with a modified kolourpaint (to be able to change the quality) Thanks, Martin Koller
