| Summary: | [GTK] Add WebP image support | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Sergio Villar Senin <svillar> | ||||||
| Component: | WebKitGTK | Assignee: | Sergio Villar Senin <svillar> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | changseok, gustavo, mathias, mrobinson, philn, plaes, s.choi, svillar, waldyrious, webkit.review.bot, xan.lopez, zan | ||||||
| Priority: | P2 | Keywords: | Gtk, Soup | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| URL: | http://www.html5rocks.com/en/tutorials/filters/understanding-css/ | ||||||||
| Bug Depends on: | 47512 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
Sergio Villar Senin
2013-01-02 03:51:24 PST
OK, so all of them are webp images, maybe it's just some issue with my setup... We don't have support for WebP yet. Perhaps the site is sending them because of our Chromium-esque user agent. (In reply to comment #2) > We don't have support for WebP yet. Perhaps the site is sending them because of our Chromium-esque user agent. Isn't there a generic WebP decoder in platform/image-decoders? What else do we need? There's a platform-independent decoder, but it has a dependency on some external library. Take a look in Source/WebCore/platform/image-decoders/webp for more details. It'd be great to add WebP support. It might be as simple as compiling this library into ours. It should be a matter of correctly detecting and linking against the libwebp library Created attachment 181040 [details]
Patch
Comment on attachment 181040 [details]
Patch
Instead of a configuration option, we should always turn this on. I don't think it should be configurable.
Created attachment 181160 [details]
Patch
The bots require libwebp. Should it be included in the Jhbuild moduleset? (In reply to comment #9) > The bots require libwebp. Should it be included in the Jhbuild moduleset? As we don't need any special version, I'd say that we could simply install the distro version in the bots... (In reply to comment #9) > The bots require libwebp. Should it be included in the Jhbuild moduleset? Right now we only use the JHBuild for dependencies where you need the correct version to have correct test results. I think it would be nice to have an alternate JHBuild moduleset with many hard-to-get dependencies, but probably not in this patch. Comment on attachment 181160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181160&action=review Thanks! Perhaps you could investigate a small change before landing: > configure.ac:278 > +# Check for WEBP Image support > +have_webp=no > +AC_CHECK_HEADERS([webp/decode.h], [have_webp="yes"], [have_webp="no"]) > +if test "$have_webp" = "yes"; then > + WEBP_LIBS='-lwebp' > +else > + AC_MSG_ERROR([WebP library (libwebp) not found]) > +fi > +AC_SUBST([WEBP_LIBS]) I don't mean to be nitpicky, but I think you can probably simplify this a bit by getting rid of the have_webp variable. That would be more useful if you were actually using it later. Instead I think you could just keep what you need here inline in the macro. For instance, something like this should work: AC_CHECK_HEADERS([webp/decode.h], [WEBP_LIBS="-lwebp'], AC_MSG_ERROR([WebP library (libwebp) not found]) AC_SUBST([WEBP_LIBS]) (In reply to comment #12) > (From update of attachment 181160 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181160&action=review > > Thanks! Perhaps you could investigate a small change before landing: > > > configure.ac:278 > > +# Check for WEBP Image support > > +have_webp=no > > +AC_CHECK_HEADERS([webp/decode.h], [have_webp="yes"], [have_webp="no"]) > > +if test "$have_webp" = "yes"; then > > + WEBP_LIBS='-lwebp' > > +else > > + AC_MSG_ERROR([WebP library (libwebp) not found]) > > +fi > > +AC_SUBST([WEBP_LIBS]) > > I don't mean to be nitpicky, but I think you can probably simplify this a bit by getting rid of the have_webp variable. That would be more useful if you were actually using it later. > > Instead I think you could just keep what you need here inline in the macro. For instance, something like this should work: > > AC_CHECK_HEADERS([webp/decode.h], [WEBP_LIBS="-lwebp'], AC_MSG_ERROR([WebP library (libwebp) not found]) > AC_SUBST([WEBP_LIBS]) Absolutely true, I should have removed it because I only defined it for using it later in the AM_CONDITIONAL. I had updated Igalia bots, kov, philn, could you please install libwebp-dev in your machines? The patch to land will unskip also these two tests: http/tests/images/webp-partial-load.html http/tests/images/webp-progressive-load.html We cannot unskip the WebP decoding test (fast/images/webp-image-decoding.html) just because it requires version 0.2 of the library which is still not available for bots distros. Committed r139046: <http://trac.webkit.org/changeset/139046> Is there a reason of not using pkg-config for lookup? (In reply to comment #17) > Is there a reason of not using pkg-config for lookup? Yes, the library does not provide a .pc (In reply to comment #18) > (In reply to comment #17) > > Is there a reason of not using pkg-config for lookup? > > Yes, the library does not provide a .pc Seems to be 0.2.x+ feature :( (In reply to comment #19) > (In reply to comment #18) > > (In reply to comment #17) > > > Is there a reason of not using pkg-config for lookup? > > > > Yes, the library does not provide a .pc > > Seems to be 0.2.x+ feature :( Yeah, apart from that, 0.2 will allow us to unskip the webp-decode test which requires some features not present in 0.1.x *** Bug 98939 has been marked as a duplicate of this bug. *** |