[FFmpeg-devel] [PATCH] webp: fix transforms after a palette with pixel packing.
James Zern
jzern at google.com
Tue Sep 21 03:44:11 EEST 2021
On Wed, Sep 8, 2021 at 6:46 PM James Zern <jzern at google.com> wrote:
>
> On Mon, Aug 30, 2021 at 5:11 AM Maryla <maryla-at-google.com at ffmpeg.org> wrote:
> >
> > When a color indexing transform with 16 or fewer colors is used,
> > WebP uses "pixel packing", i.e. storing several pixels in one byte,
> > which virtually reduces the width of the image (see WebPContext's
> > reduced_width field). This reduced_width should always be used when
> > reading and applying subsequent transforms.
> >
> > Updated patch with added fate test.
> > The source image dual_transform.webp can be downloaded by cloning
> > https://chromium.googlesource.com/webm/libwebp-test-data/
> >
> > Fixes: 9368
> > ---
> > libavcodec/webp.c | 34 ++++++++++---------
> > tests/fate/image.mak | 3 ++
> > .../fate/webp-rgb-lossless-palette-predictor | 6 ++++
> > 3 files changed, 27 insertions(+), 16 deletions(-)
> > create mode 100644 tests/ref/fate/webp-rgb-lossless-palette-predictor
> >
>
> This works locally and matches the output from libwebp. I sent a
> request to samples-request@ to add the file. This should have a micro
> version bump for libavcodec/version.h; I've added that locally.
>
The sample addition is still pending. I've verified the pam output of
this file and lossless* from the webp test set match libwebp. I'll
submit this soon with the fate test disabled if there aren't any
comments.
> > diff --git a/libavcodec/webp.c b/libavcodec/webp.c
> > index 3efd4438d9..e4c67adc3a 100644
> > --- a/libavcodec/webp.c
> > +++ b/libavcodec/webp.c
> > @@ -181,7 +181,10 @@ typedef struct ImageContext {
> > uint32_t *color_cache; /* color cache data */
> > int nb_huffman_groups; /* number of huffman groups */
> > HuffReader *huffman_groups; /* reader for each huffman group */
> > - int size_reduction; /* relative size compared to primary image, log2 */
> > + /* relative size compared to primary image, log2.
> > + * for IMAGE_ROLE_COLOR_INDEXING with <= 16 colors, this is log2 of the
> > + * number of pixels per byte in the primary image (pixel packing) */
> > + int size_reduction;
> > int is_alpha_primary;
> > } ImageContext;
> >
> > @@ -205,7 +208,9 @@ typedef struct WebPContext {
> >
> > int nb_transforms; /* number of transforms */
> > enum TransformType transforms[4]; /* transformations used in the image, in order */
> > - int reduced_width; /* reduced width for index image, if applicable */
> > + /* reduced width when using a color indexing transform with <= 16 colors (pixel packing)
> > + * before pixels are unpacked, or same as width otherwise. */
> > + int reduced_width;
> > int nb_huffman_groups; /* number of huffman groups in the primary image */
> > ImageContext image[IMAGE_ROLE_NB]; /* image context for each role */
> > } WebPContext;
> > @@ -425,13 +430,9 @@ static int decode_entropy_coded_image(WebPContext *s, enum ImageRole role,
> > static int decode_entropy_image(WebPContext *s)
> > {
> > ImageContext *img;
> > - int ret, block_bits, width, blocks_w, blocks_h, x, y, max;
> > + int ret, block_bits, blocks_w, blocks_h, x, y, max;
> >
> > - width = s->width;
> > - if (s->reduced_width > 0)
> > - width = s->reduced_width;
> > -
> > - PARSE_BLOCK_SIZE(width, s->height);
> > + PARSE_BLOCK_SIZE(s->reduced_width, s->height);
> >
> > ret = decode_entropy_coded_image(s, IMAGE_ROLE_ENTROPY, blocks_w, blocks_h);
> > if (ret < 0)
> > @@ -460,7 +461,7 @@ static int parse_transform_predictor(WebPContext *s)
> > {
> > int block_bits, blocks_w, blocks_h, ret;
> >
> > - PARSE_BLOCK_SIZE(s->width, s->height);
> > + PARSE_BLOCK_SIZE(s->reduced_width, s->height);
> >
> > ret = decode_entropy_coded_image(s, IMAGE_ROLE_PREDICTOR, blocks_w,
> > blocks_h);
> > @@ -476,7 +477,7 @@ static int parse_transform_color(WebPContext *s)
> > {
> > int block_bits, blocks_w, blocks_h, ret;
> >
> > - PARSE_BLOCK_SIZE(s->width, s->height);
> > + PARSE_BLOCK_SIZE(s->reduced_width, s->height);
> >
> > ret = decode_entropy_coded_image(s, IMAGE_ROLE_COLOR_TRANSFORM, blocks_w,
> > blocks_h);
> > @@ -620,7 +621,7 @@ static int decode_entropy_coded_image(WebPContext *s, enum ImageRole role,
> > }
> >
> > width = img->frame->width;
> > - if (role == IMAGE_ROLE_ARGB && s->reduced_width > 0)
> > + if (role == IMAGE_ROLE_ARGB)
> > width = s->reduced_width;
> >
> > x = 0; y = 0;
> > @@ -925,7 +926,7 @@ static int apply_predictor_transform(WebPContext *s)
> > int x, y;
> >
> > for (y = 0; y < img->frame->height; y++) {
> > - for (x = 0; x < img->frame->width; x++) {
> > + for (x = 0; x < s->reduced_width; x++) {
> > int tx = x >> pimg->size_reduction;
> > int ty = y >> pimg->size_reduction;
> > enum PredictionMode m = GET_PIXEL_COMP(pimg->frame, tx, ty, 2);
> > @@ -965,7 +966,7 @@ static int apply_color_transform(WebPContext *s)
> > cimg = &s->image[IMAGE_ROLE_COLOR_TRANSFORM];
> >
> > for (y = 0; y < img->frame->height; y++) {
> > - for (x = 0; x < img->frame->width; x++) {
> > + for (x = 0; x < s->reduced_width; x++) {
> > cx = x >> cimg->size_reduction;
> > cy = y >> cimg->size_reduction;
> > cp = GET_PIXEL(cimg->frame, cx, cy);
> > @@ -985,7 +986,7 @@ static int apply_subtract_green_transform(WebPContext *s)
> > ImageContext *img = &s->image[IMAGE_ROLE_ARGB];
> >
> > for (y = 0; y < img->frame->height; y++) {
> > - for (x = 0; x < img->frame->width; x++) {
> > + for (x = 0; x < s->reduced_width; x++) {
> > uint8_t *p = GET_PIXEL(img->frame, x, y);
> > p[1] += p[2];
> > p[3] += p[2];
> > @@ -1004,7 +1005,7 @@ static int apply_color_indexing_transform(WebPContext *s)
> > img = &s->image[IMAGE_ROLE_ARGB];
> > pal = &s->image[IMAGE_ROLE_COLOR_INDEXING];
> >
> > - if (pal->size_reduction > 0) {
> > + if (pal->size_reduction > 0) { // undo pixel packing
> > GetBitContext gb_g;
> > uint8_t *line;
> > int pixel_bits = 8 >> pal->size_reduction;
> > @@ -1030,6 +1031,7 @@ static int apply_color_indexing_transform(WebPContext *s)
> > }
> > }
> > av_free(line);
> > + s->reduced_width = s->width; // we are back to full size
> > }
> >
> > // switch to local palette if it's worth initializing it
> > @@ -1126,7 +1128,7 @@ static int vp8_lossless_decode_frame(AVCodecContext *avctx, AVFrame *p,
> >
> > /* parse transformations */
> > s->nb_transforms = 0;
> > - s->reduced_width = 0;
> > + s->reduced_width = s->width;
> > used = 0;
> > while (get_bits1(&s->gb)) {
> > enum TransformType transform = get_bits(&s->gb, 2);
> > diff --git a/tests/fate/image.mak b/tests/fate/image.mak
> > index 3b58972a53..ea9c801990 100644
> > --- a/tests/fate/image.mak
> > +++ b/tests/fate/image.mak
> > @@ -517,6 +517,9 @@ fate-webp-rgb-lena-lossless-rgb24: CMD = framecrc -i $(TARGET_SAMPLES)/webp/rgb_
> > FATE_WEBP += fate-webp-rgba-lossless
> > fate-webp-rgba-lossless: CMD = framecrc -i $(TARGET_SAMPLES)/webp/rgba_lossless.webp
> >
> > +FATE_WEBP += fate-webp-rgb-lossless-palette-predictor
> > +fate-webp-rgb-lossless-palette-predictor: CMD = framecrc -i $(TARGET_SAMPLES)/webp/dual_transform.webp
> > +
> > FATE_WEBP += fate-webp-rgb-lossy-q80
> > fate-webp-rgb-lossy-q80: CMD = framecrc -i $(TARGET_SAMPLES)/webp/rgb_q80.webp
> >
> > diff --git a/tests/ref/fate/webp-rgb-lossless-palette-predictor b/tests/ref/fate/webp-rgb-lossless-palette-predictor
> > new file mode 100644
> > index 0000000000..92a4ad9810
> > --- /dev/null
> > +++ b/tests/ref/fate/webp-rgb-lossless-palette-predictor
> > @@ -0,0 +1,6 @@
> > +#tb 0: 1/25
> > +#media_type 0: video
> > +#codec_id 0: rawvideo
> > +#dimensions 0: 100x30
> > +#sar 0: 0/1
> > +0, 0, 0, 1, 12000, 0xb200d843
> > --
> > 2.33.0.259.gc128427fd7-goog
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list