WEBP: Coder enhancements/memory leak fix

Questions and postings pertaining to the development of ImageMagick, feature enhancements, and ImageMagick internals. ImageMagick source code and algorithms are discussed here. Usage questions which are too arcane for the normal user list should also be posted here.
Post Reply
tc33
Posts: 40
Joined: 2012-10-21T22:10:21-07:00
Authentication code: 67789

WEBP: Coder enhancements/memory leak fix

Post by tc33 »

I just started working with WEBP and noticed that the coder can't do WEBP lossless, and it also has a memory leak. I made some significant changes to core_coders/webp.c which allow IM users to define codec-specific optimizations and, most notably, lossless encoding. Below is my code for your consideration for inclusion, based on IM 6.8.3.0.

Sorry I don't have a DIFF tool, so here are the functions that were modified:

additional includes:
#include "magick/artifact.h"
#include "magick/string-private.h"

Code: Select all

static Image *ReadWEBPImage(const ImageInfo *image_info,
  ExceptionInfo *exception)
{
  int
    height,
    width;

  Image
    *image;

  MagickBooleanType
    status;

  register PixelPacket
    *q;

  register ssize_t
    x;

  register unsigned char
    *p;

  size_t
    length;

  ssize_t
    count,
    y;

  unsigned char
    *stream,
    *pixels;

  /*
    Open image file.
  */
  assert(image_info != (const ImageInfo *) NULL);
  assert(image_info->signature == MagickSignature);
  if (image_info->debug != MagickFalse)
    (void) LogMagickEvent(TraceEvent,GetMagickModule(),"%s",
      image_info->filename);
  assert(exception != (ExceptionInfo *) NULL);
  assert(exception->signature == MagickSignature);
  image=AcquireImage(image_info);
  status=OpenBlob(image_info,image,ReadBinaryBlobMode,exception);
  if (status == MagickFalse)
    {
      image=DestroyImageList(image);
      return((Image *) NULL);
    }
  length=(size_t) GetBlobSize(image);
  stream=(unsigned char *) AcquireQuantumMemory(length,sizeof(*stream));
  if (stream == (unsigned char *) NULL)
    ThrowReaderException(ResourceLimitError,"MemoryAllocationFailed");
  count=ReadBlob(image,length,stream);
  if (count != (ssize_t) length)
    ThrowReaderException(CorruptImageError,"InsufficientImageDataInFile");
  pixels=(unsigned char *) WebPDecodeRGBA(stream,length,&width,&height);
  if (pixels == (unsigned char *) NULL)
    ThrowReaderException(ResourceLimitError,"MemoryAllocationFailed");

  // read simple file format header to determine if lossy or lossless
  // per the spec, byte 16 is the presence of the magic chunk for lossy (space), lossless (L), or extended format (X)
  //  check for it, set image artifact for later use during save
  //  todo:  read extended format ( stream[15] == 'X'), then find VP8(?) chunk to determine lossy/lossless
  if ( ( stream[15] == ' ' ) || ( stream[15] == 'L' ) )
	  SetImageArtifact( 
		  image, 
		  "webp:lossless", 
		  ( stream[15] == 'L' ) ? "1" : "0"
		  );

  // if lossless, do we want to set quality to 100?

  image->columns=(size_t) width;
  image->rows=(size_t) height;
  p=pixels;
  for (y=0; y < (ssize_t) image->rows; y++)
  {
    q=QueueAuthenticPixels(image,0,y,image->columns,1,exception);
    if (q == (PixelPacket *) NULL)
      break;
    for (x=0; x < (ssize_t) image->columns; x++)
    {
      SetPixelRed(q,ScaleCharToQuantum(*p++));
      SetPixelGreen(q,ScaleCharToQuantum(*p++));
      SetPixelBlue(q,ScaleCharToQuantum(*p++));
      SetPixelAlpha(q,ScaleCharToQuantum(*p++));
      if (q->opacity != OpaqueOpacity)
        image->matte=MagickTrue;
      q++;
    }
    if (SyncAuthenticPixels(image,exception) == MagickFalse)
      break;
    status=SetImageProgress(image,LoadImageTag,(MagickOffsetType) y,
      image->rows);
    if (status == MagickFalse)
      break;
  }
  free(pixels);
  pixels=(unsigned char *) NULL;

  // tc:  fix memory leak
  stream = (unsigned char*) RelinquishMagickMemory( stream );

  return(image);
}

Code: Select all

static MagickBooleanType WriteWEBPImage(const ImageInfo *image_info,
  Image *image)
{
  int
    webp_status;

  MagickBooleanType
    status;

  register const PixelPacket
    *restrict p;

  register ssize_t
    x;

  //register unsigned char
  //  *restrict q;

  // tc
  register uint32_t
	*restrict q;

  ssize_t
    y;

  //unsigned char
  //  *pixels;

  uint32_t
    *pixels;

  WebPConfig
    configure;

  WebPPicture
    picture;

  WebPAuxStats
    statistics;

  // tc
  const char
    *value;

  /*
    Open output image file.
  */
  assert(image_info != (const ImageInfo *) NULL);
  assert(image_info->signature == MagickSignature);
  assert(image != (Image *) NULL);
  assert(image->signature == MagickSignature);
  if (image->debug != MagickFalse)
    (void) LogMagickEvent(TraceEvent,GetMagickModule(),"%s",image->filename);
  if ((image->columns > 16383) || (image->rows > 16383))
    ThrowWriterException(ImageError,"WidthOrHeightExceedsLimit");
  status=OpenBlob(image_info,image,WriteBinaryBlobMode,&image->exception);
  if (status == MagickFalse)
    return(status);

  if (WebPPictureInit(&picture) == 0)
    ThrowWriterException(ResourceLimitError,"MemoryAllocationFailed");
  picture.use_argb = 1;
  picture.writer=WebPWriter;
  picture.custom_ptr=(void *) image;
  picture.stats=(&statistics);
  picture.width=(int) image->columns;
  picture.argb_stride = picture.width;
  picture.height=(int) image->rows;
  if (WebPConfigInit(&configure) == 0)
    ThrowWriterException(ResourceLimitError,"MemoryAllocationFailed");
  if (image->quality != UndefinedCompressionQuality)
    configure.quality=(float) image->quality;

  // configure options here, https://developers.google.com/speed/webp/docs/api

  // lossless
  // todo:  if user specifies quality of 100, do they mean they want lossless, or lossy @ q100?
  value=GetImageOption(image_info,"webp:lossless");
  if (value != (char *) NULL)
	  configure.lossless = StringToInteger( value );
  else if ( image->quality == 0 )	
  {
	  value = GetImageArtifact( image, "webp:lossless" );
	  if (value != (char *) NULL)
		configure.lossless = StringToInteger( value );
  }
  // else, the user set the quality to something other than the default (0).  ignore lossless flag

  // method
  value=GetImageOption(image_info,"webp:method");
  if (value != (char *) NULL)
	  configure.method = StringToInteger( value );

  // image_hint
  value=GetImageOption(image_info,"webp:image_hint");
  if (value != (char *) NULL)
	  configure.image_hint = (WebPImageHint) StringToInteger( value );

  // target size
  value=GetImageOption(image_info,"webp:target_size");
  if (value != (char *) NULL)
	  configure.target_size = StringToInteger( value );

  // target psnr
  value=GetImageOption(image_info,"webp:target_psnr");
  if (value != (char *) NULL)
	  configure.target_PSNR = (float) StringToDouble(value, (char **) NULL);

  // segments
  value=GetImageOption(image_info,"webp:segments");
  if (value != (char *) NULL)
	  configure.segments = StringToInteger( value );

  // sns strength
  value=GetImageOption(image_info,"webp:sns_strength");
  if (value != (char *) NULL)
	  configure.sns_strength = StringToInteger( value );

  // filter strength
  value=GetImageOption(image_info,"webp:filter_strength");
  if (value != (char *) NULL)
	  configure.filter_strength = StringToInteger( value );
  
  // filter sharpness
  value=GetImageOption(image_info,"webp:filter_sharpness");
  if (value != (char *) NULL)
	  configure.filter_sharpness = StringToInteger( value );

  // filter_type
  value=GetImageOption(image_info,"webp:filter_type");
  if (value != (char *) NULL)
	  configure.filter_type = StringToInteger( value );

  // autofilter
  value=GetImageOption(image_info,"webp:autofilter");
  if (value != (char *) NULL)
	  configure.autofilter = StringToInteger( value );

  // alpha_compression
  value=GetImageOption(image_info,"webp:alpha_compression");
  if (value != (char *) NULL)
	  configure.alpha_compression = StringToInteger( value );

  // alpha_filtering
  value=GetImageOption(image_info,"webp:alpha_filtering");
  if (value != (char *) NULL)
	  configure.alpha_filtering = StringToInteger( value );

  // alpha_quality
  value=GetImageOption(image_info,"webp:alpha_quality");
  if (value != (char *) NULL)
	  configure.alpha_quality = StringToInteger( value );

  // pass
  value=GetImageOption(image_info,"webp:pass");
  if (value != (char *) NULL)
	  configure.pass = StringToInteger( value );

  // show_compressed
  value=GetImageOption(image_info,"webp:show_compressed");
  if (value != (char *) NULL)
	  configure.show_compressed = StringToInteger( value );

  // preprocessing
  value=GetImageOption(image_info,"webp:preprocessing");
  if (value != (char *) NULL)
	  configure.preprocessing = StringToInteger( value );

  // partitions
  value=GetImageOption(image_info,"webp:partitions");
  if (value != (char *) NULL)
	  configure.partitions = StringToInteger( value );

  // partition_limit
  value=GetImageOption(image_info,"webp:partition_limit");
  if (value != (char *) NULL)
	  configure.partition_limit = StringToInteger( value );

  // done with configure options, WebPValidateConfig will do range checking
  if (WebPValidateConfig(&configure) == 0)
    ThrowWriterException(CoderFatalError,"EncodingConfigurationError");  // correct error type?

  // Note:  WEBP requires ARGB format for lossless encoding, and will convert to YUV if needed for lossy.  
  //	Therefore, if we just provide ARGB data, the codec does the rest.

  // pixel data malloc
  pixels = (uint32_t*) AcquireMagickMemory( image->columns * image->rows * sizeof( *picture.argb ) );
  if ( pixels == (uint32_t*) NULL)
    ThrowWriterException(ResourceLimitError,"MemoryAllocationFailed");

  // Convert image to WebP raster pixels, ARGB
  q = pixels;
  for (y=0; y < (ssize_t) image->rows; y++)
  {
    p=GetVirtualPixels(image,0,y,image->columns,1,&image->exception);
    if (p == (PixelPacket *) NULL)
      break;
    for (x=0; x < (ssize_t) image->columns; x++)
    {
		*q++ = (uint32_t)
            (image->matte != MagickFalse ? ScaleQuantumToChar(GetPixelAlpha(p)) << 24 : 0xff000000u )  |
            (ScaleQuantumToChar(GetPixelRed(p)) << 16) |
            (ScaleQuantumToChar(GetPixelGreen(p)) << 8) |
            (ScaleQuantumToChar(GetPixelBlue(p)));

		p++;
    }
    status=SetImageProgress(image,SaveImageTag,(MagickOffsetType) y,
      image->rows);
    if (status == MagickFalse)
      break;
  }

  picture.argb = pixels;

  webp_status=WebPEncode(&configure,&picture);
  WebPPictureFree(&picture);
  pixels = (uint32_t*) RelinquishMagickMemory( pixels );	// free pixels

  (void) CloseBlob(image);
  return(webp_status == 0 ? MagickFalse : MagickTrue);
}
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Re: WEBP: Coder enhancements/memory leak fix

Post by magick »

Thanks, for the patch. Give us a day or two to get it into ImageMagick 6.8.3-2 Beta.
tc33
Posts: 40
Joined: 2012-10-21T22:10:21-07:00
Authentication code: 67789

Re: WEBP: Coder enhancements/memory leak fix

Post by tc33 »

Hi, thanks for getting this integrated!

I was reviewing the latest changes to webp prior to downloading/compiling, and I noticed an issue that might need attention:

http://trac.imagemagick.org/browser/Ima ... ers/webp.c

Line 176:

Code: Select all

if ((stream[15] == 'L') || (stream[15] == ' '))
    image->quality=100;
Per the webp spec ( https://developers.google.com/speed/web ... rmat-lossy ), for the simple (non-extended) file header, byte 16 will be an 'L' for lossless, and a ' ' (blank) for lossy. Based on this code, however, the webp coder will set any webp file that has a simple file header to be lossless (q=100).

My recommendation is to change this line to:

Code: Select all

if (stream[15] == 'L')
If I get some more time I'll figure out how to read the extended file header ( stream[15] == 'X' ) to detect lossless.

Also there may be a minor consistency item, line 411: should this be "filter-sharpness" instead of "filter_sharpness"?

Thanks again!
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Re: WEBP: Coder enhancements/memory leak fix

Post by magick »

Look for the patches in ImageMagick 6.8.3-3 Beta by sometime tomorrow. Thanks.
tc33
Posts: 40
Joined: 2012-10-21T22:10:21-07:00
Authentication code: 67789

Re: WEBP: Coder enhancements/memory leak fix

Post by tc33 »

Thank you! One more submission for you; this reads the extended file format to determine lossy or lossless based on the chunk data. Could probably use another set of eyes to ensure this won't crash on edge cases

Code: Select all

// from libwebp, webp.c
static inline uint32_t get_le24(const unsigned char* const data) {
  return data[0] | (data[1] << 8) | (data[2] << 16);
}

static inline uint32_t get_le32(const unsigned char* const data) {
  return (uint32_t)get_le24(data) | (data[3] << 24);
}

// return flag if webp image is lossless based on file header
static MagickBooleanType IsWEBPImageLossless(const unsigned char* stream, const size_t length)
{
	#define VP8_CHUNK_INDEX		15		// position in VP8 header which determines file/header type
	#define LOSSLESS_FLAG		'L'
	#define EXTENDED_HEADER		'X'
	#define VP8_CHUNK_HEADER	"VP8"
	#define VP8_CHUNK_HEADER_SIZE	3

	// from libwebp/format_constants.h
	#define RIFF_HEADER_SIZE	12	// Size of the RIFF header ("RIFFnnnnWEBP").
	#define VP8X_CHUNK_SIZE		10	// Size of a VP8X chunk
	#define TAG_SIZE           4     // Size of a chunk tag (e.g. "VP8L").
	#define CHUNK_SIZE_BYTES   4     // Size needed to store chunk's size.
	#define CHUNK_HEADER_SIZE  8     // Size of a chunk header.
	// Maximum chunk payload is such that adding the header and padding won't overflow a uint32_t.
	#define MAX_CHUNK_PAYLOAD (~0U - CHUNK_HEADER_SIZE - 1)

	// position in stream
	size_t pos = 0;

	// read simple header
	if (stream[VP8_CHUNK_INDEX] != EXTENDED_HEADER)	// simple header?
		return (stream[VP8_CHUNK_INDEX] == LOSSLESS_FLAG) ? MagickTrue : MagickFalse;

	// read extended header
	pos = RIFF_HEADER_SIZE + TAG_SIZE + CHUNK_SIZE_BYTES + VP8X_CHUNK_SIZE;     // RIFF + VP8X chunk header + VP8X chunk size + VP8X chunk data

	// based on libwebp, webp.c, ParseOptionalChunks
	while (pos <= length) 
	{
		uint32_t chunk_size;
		uint32_t disk_chunk_size;   // chunk_size with padding

		chunk_size = get_le32(stream + pos + TAG_SIZE);
		if (chunk_size > MAX_CHUNK_PAYLOAD)		// Not a valid chunk size.
		  break;

		// For odd-sized chunk-payload, there's one byte padding at the end.
		disk_chunk_size = (CHUNK_HEADER_SIZE + chunk_size + 1) & ~1;

		if ( !memcmp( stream + pos, VP8_CHUNK_HEADER, VP8_CHUNK_HEADER_SIZE ) )	// found VP8? chunk.  check for lossless flag
			return (*(stream + pos + VP8_CHUNK_HEADER_SIZE) == LOSSLESS_FLAG) ? MagickTrue : MagickFalse;

		// We have a full and valid chunk; skip it.
		pos += disk_chunk_size;
	}	// while

	// didnt find VP8 chunk, return false
	return MagickFalse;

}	// IsWEBPImageLossless
And the change in ReadWEBPImage, line 210:

Code: Select all

  //if (stream[15] == 'L')  // old code here
  if ( IsWEBPImageLossless( stream, length ) == MagickTrue )
    image->quality=100;
Thanks again! :)
User avatar
magick
Site Admin
Posts: 11064
Joined: 2003-05-31T11:32:55-07:00

Re: WEBP: Coder enhancements/memory leak fix

Post by magick »

We'll have your patch in ImageMagick 6.8.3-5 Beta by sometime tomorrow. Thanks.
tc33
Posts: 40
Joined: 2012-10-21T22:10:21-07:00
Authentication code: 67789

Re: WEBP: Coder enhancements/memory leak fix

Post by tc33 »

Just got 6.8.3-5, the changes are working well. Thanks again! :)
Post Reply