-
-
Notifications
You must be signed in to change notification settings - Fork 649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Losing transparency in WEBP to WEBP conversion #1351
Comments
Looks like some images are able to have |
Oh interesting, could you upload the original webp somewhere? |
You can download it directly, I just renamed it to PNG to make Github happy 😉 |
I tried with DEBUG on and I see:
So libwebp says your image has no alpha. Perhaps it means that this 200x25 block of pixels (within the 200x200 canvas) has no alpha, but obviously the unwritten bits outside the 200x25 area should be transparent? |
We used to try to spot webp images with no alpha and load them as plain RGB, but it turns out this is difficult to do reliably, especially for animated images. This patch simply removes support, so all webp images now load as RGBA. See #1351
It looks like the best solution is to simply remove RGB (ie. no alpha) support from the webp loader, so everything now loads as RGBA. This will be in 8.9. Thanks! |
@jcupitt Should the new logic to always load WebP as RGBA apply only to animated (multi-frame) images? |
Even for non-animated images it's awkward. You can have a 200x200 pixel non-alpha image in a 400x400 canvas, for example, so there should be a 100 pixel transparent border. How annoying! |
You're right we could turn off alpha write for webp where all frames are RGB, and all frames fill the canvas exactly. Do you think this is worthwhile? |
Perhaps we could use // Features gathered from the bitstream
struct WebPBitstreamFeatures {
int width; // Width in pixels, as read from the bitstream.
int height; // Height in pixels, as read from the bitstream.
int has_alpha; // True if the bitstream contains an alpha channel.
int has_animation; // True if the bitstream is an animation.
int format; // 0 = undefined (/mixed), 1 = lossy, 2 = lossless
uint32_t pad[5]; // padding for later use
};
VP8StatusCode WebPGetFeatures(const uint8_t* data,
size_t data_size,
WebPBitstreamFeatures* features); |
Oh, that's a new call, or I'd not seen it. I tried with the sample image above and I see:
It seems it's reporting the alpha of the area of pixels within the canvas, but not whether or not the canvas is transparent (ie. has pixels not covered in all frames), so I don't think it'll help here. |
You're right:
Is the original GIF available to see if it's reproducible with |
disable webp alpha output if all frame fill the canvas and are solid see #1351
This time I made a branch :) Now it outputs alpha for a webp animation if any frames don't completely fill the canvas. |
Here's @deftomat 's test image as a webp: |
I found an interesting note in the WebP documentation:
Perhaps we could force WebP alpha output if the background color isn't opaque? This seems to work for me: printf( "webp2vips: alpha = %d\n", (WebPDemuxGetI( read->demux, WEBP_FF_BACKGROUND_COLOR ) & 0x7F000000) >> 24); The
Which indicates that the background color is completely transparent. |
Oh hmm this is very confusing. That page is actually out of date: I came across a google groups post from the devs saying that BACKGROUND should not be used with DISPOSE and that it was just for display. We had to move away from using BACKGROUND a few months ago: 4a9db0e#diff-17a3ebd6f63f54acf230809cd70e2136 I think this means that a webp animation effectively has three layers:
libvips webpload is outputting layers 1 + 2, and stripping the alpha in the special case that we know for sure that the alpha is solid. We currently ignore the BACKGROUND setting. |
Ah, I see. After testing, the BACKGROUND hint seems to be rather confusing, as you said. For example, it even reports a transparent background when I flatten the alpha out of the image. The WebP RGBA handling can be slightly improved; see: $ vips flatten x.webp[n=-1] x2.webp
$ webpmux -info x2.webp
Canvas size: 200 x 200
Features present: animation EXIF metadata
Background color : 0xFFFFFFFF Loop Count : 0
Number of frames: 4
No.: width height alpha x_offset y_offset duration dispose blend image_size compression
1: 200 200 no 0 0 500 none no 1104 lossy
2: 150 29 no 0 86 500 none yes 1338 lossy
3: 150 28 no 0 86 500 none yes 1164 lossy
4: 150 25 no 0 88 500 none yes 1102 lossy
Size of the EXIF metadata: 186
$ vipsheader x2.webp
x2.webp: 200x200 uchar, 4 bands, srgb, webpload It still reports 4 bands if the alpha channel is removed. This can be resolved with the following patch: diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c
index 1111111..2222222 100644
--- a/libvips/foreign/webp2vips.c
+++ b/libvips/foreign/webp2vips.c
@@ -493,10 +493,21 @@ read_header( Read *read, VipsImage *out )
vips_image_set_int( out, "gif-delay",
VIPS_RINT( read->delay / 10.0 ) );
- /* If we find a frame which doesn't fill the canvas,
- * we'll need to save as RGBA.
- */
do {
+ /* If we find a frame that fills the whole canvas,
+ * and doesn't contain an alpha channel, skip.
+ */
+ if( !iter.has_alpha &&
+ iter.x_offset == 0 &&
+ iter.y_offset == 0 &&
+ iter.width == read->canvas_width &&
+ iter.height == read->canvas_height ) {
+ break;
+ }
+
+ /* Otherwise, if we find a frame which doesn't
+ * fill the canvas, we'll need to save as RGBA.
+ */
if( iter.x_offset != 0 ||
iter.y_offset != 0 ||
iter.width != read->canvas_width || $ vipsheader x2.webp
x2.webp: 200x200 uchar, 3 bands, srgb, webpload |
Nice idea, but is it 100% safe? You will drop the alpha if there's at least one 100% solid frame, is that right? But how about an animation where the first frame is transparent and the second frame is solid? When it displays, we should see the background on the transparent frame, so we'll need the alpha even though it's not always necessary. The only time we can not have an alpha in an animation is something like a video clip where each frame is a new and completely solid rect of pixels. |
You're certainly right that it should check the rect alpha too, good point! I've updated the branch. |
Indeed, the patch that I mentioned isn't 100% safe. I think it's better to add an alpha channel in such situations instead of removing it, just to be sure (+ it's only for animated WebP images, which I don't see often). fwiw, I quickly looked at other implementations. Skia does approximately the same than us, but just for the first frame only. https://github.com/google/skia/blob/161f47dfbf6ae5641263d8c95a421d2e82ae4a36/src/codec/SkWebpCodec.cpp#L134-L135 |
Oh interesting, yes I guess we could skip testing x_offset / y_offset. Shall I merge to master? |
It can be merged safely, I think. Should it also be included in the 8.8 branch? |
I'm not sure about patching 8.8 as well. It's not really a bugfix and it's change in behaviour, though a small one. |
OK, merged to 8.9, nice job everyone! |
Oh argh, I forgot the rgba -> rgb output stage. Try now, git master seems to work for me. |
AWESOME AS ALWAYS! Thanks! Works as expected. libvips is the best OS experience I ever had. You guys are on 🔥 |
Running
vips rot input.webp[n=-1] output.webp d0
whereinput.webp
is an animated image transforms transparent pixels into black.Works OK with non-animated image. Also, works for GIF -> WEBP.
EDIT:
Actually, it looks like
webploader
loads the file incorrectly.For example, the following animated image has a transparency and
bands=3
.(Disclaimer: Image was generated by VIPS itself by converting GIF to WEBP)
The text was updated successfully, but these errors were encountered: