Skip to content

Conversation

brianpopow
Copy link
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

This PR adds support for encoding lossy webp images with alpha channels.
The alpha data can be either compressed (with webp lossless compression) or uncompressed.

Related to #1802

{
int width = image.Width;
int height = image.Height;
IMemoryOwner<byte> alphaData = ExtractAlphaChannel(image, configuration, memoryAllocator);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see where this is getting disposed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its done in the Dispose of the Vp8Encoder

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to keep the responsibility of disposing to the AlphaEncoder instead?

Copy link
Member

@JimBobSquarePants JimBobSquarePants Feb 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that's wise. Difficult to keep track of otherwise.

private readonly int[] scratch = new int[16];
private readonly bool alphaCompression;

private readonly byte[] averageBytesPerMb = { 50, 24, 16, 9, 7, 5, 3, 2 };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private readonly byte[] averageBytesPerMb = { 50, 24, 16, 9, 7, 5, 3, 2 };
// This uses C#'s optimization to refer to the static data segment of the assembly, no allocation occurs.
private static ReadOnlySpan<byte> AverageBytesPerMb => new byte[] { 50, 24, 16, 9, 7, 5, 3, 2 };

While you're on it.

Need to update

int averageBytesPerMacroBlock = this.averageBytesPerMb[this.BaseQuant >> 4];
to new name too.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for pushing this through so quickly!

@JimBobSquarePants JimBobSquarePants merged commit b7e348f into master Feb 2, 2022
@JimBobSquarePants JimBobSquarePants deleted the bp/webpalpha branch February 2, 2022 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants