[FFmpeg-devel] [PATCH] allow setting png compression level

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> writes:

> Hello,
> On Sat, Oct 06, 2007 at 11:42:30PM +0100, M?ns Rullg?rd wrote:
>> Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> writes:
>> > attached patch would allow setting png compression level via the
>> > coder_type option. This might be considered a bit of a misuse, but I
>> > personally find it preferable over adding a new option (and am not
>> > completely sure if it is a misuse).
>> > Other have suggested (mis-)using some other option like qscale or
>> > md_decision but that seems like a complete misuse to me.
>> > What are your opinions?
>> 
>> What's wrong with the existing AVCodecContext.compression_level?
>
> I didn't know about it (and assumed nothing exactly fitting existed
> since it was not used).
> So is attached patch ok?
>
> Greetings,
> Reimar D?ffinger
>
> Index: libavcodec/pngenc.c
> ===================================================================
> --- libavcodec/pngenc.c	(revision 10668)
> +++ libavcodec/pngenc.c	(working copy)
> @@ -187,7 +187,7 @@
>      s->zstream.zalloc = ff_png_zalloc;
>      s->zstream.zfree = ff_png_zfree;
>      s->zstream.opaque = NULL;
> -    ret = deflateInit2(&s->zstream, Z_DEFAULT_COMPRESSION,
> +    ret = deflateInit2(&s->zstream, avctx->compression_level,
>                         Z_DEFLATED, 15, 8, Z_DEFAULT_STRATEGY);
>      if (ret != Z_OK)
>          return -1;

This will work, but only because FF_COMPRESSION_DEFAULT happens to be
equal to Z_DEFAULT_STRATEGY.  I'd prefer explicitly setting this in
the init function.  Some range checking probably wouldn't hurt either.

-- 
M?ns Rullg?rd
mans at mansr.com