[FFmpeg-devel] deduplicated [PATCH] Cinepak: speed up decoding several-fold, depending on the scenario, by supporting multiple output pixel formats.

Michael Niedermayer michael at niedermayer.cc
Mon Feb 13 15:19:45 EET 2017


On Sat, Feb 11, 2017 at 10:25:03PM +0100, u-9iep at aetey.se wrote:
> Hello,
> 
> This is my best effort attempt to make the patch acceptable
> by the upstream's criteria.
> 
> Daniel, do you mind that I referred to your message in the commit?
> I believe is is best to indicate numbers from a third party measurement.
> 
> The code seems to be equvalent to the previous patch,
> with about 20% less LOC.
> 
> This hurts readability (my subjective impression) but on the positive side
> the change makes the structure of the code more explicit.
> 
> Attaching the patch.
> 
> Now I have done what I can, have to leave.
> Unless there are bugs there in the patch, my attempt to contribute ends
> at this point.
> 
> Thanks to everyone who cared to objectively discuss a specific case
> of ffmpeg usage, the implications of techniques around VQ and whether/why
> some non-traditional approaches can make sense.
> 
> Good luck to the ffmpeg project, it is very useful and valuable.
> 
> Best regards,
> Rune

>  cinepak.c |  844 ++++++++++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 692 insertions(+), 152 deletions(-)
> cc2ab45b7633651bc0ff80ca57c78ef4fc649d3c  0001-Cinepak-speed-up-decoding-several-fold-depending-on-.patch
> From 0c9badec5d144b995c0bb52c7a80939b672be3f5 Mon Sep 17 00:00:00 2001
> From: Rl <addr-see-the-website at aetey.se>
> Date: Sat, 11 Feb 2017 20:28:54 +0100
> Subject: [PATCH] Cinepak: speed up decoding several-fold, depending on the
>  scenario, by supporting multiple output pixel formats.
> 
> Decoding to rgb24 and pal8 is optimized.
> 
> Added rgb32, rgb565, yuv420p, each with faster decoding than to rgb24.
> 
> The most noticeable gain is achieved by the created possibility
> to skip format conversions, for example when decoding to rgb565
> ----
> Using matrixbench_mpeg2.mpg (720x567) encoded with ffmpeg into Cinepak
> using default settings, decoding on an i5 3570K, 3.4 GHz:
> 
> bicubic (default):          ~24x realtime
> fast_bilinear:              ~65x realtime
> patch w/rgb565 override:    ~154x realtime
> ----
> (https://ffmpeg.org/pipermail/ffmpeg-devel/2017-February/206799.html)
> 
> palettized input can be decoded to any of the output formats,
> pal8 output is still limited to palettized input
> 
> with input other than palettized/grayscale
> yuv420 is approximated by the Cinepak colorspace
> 
> The output format can be chosen at runtime by an option or via the API.
> ---
>  libavcodec/cinepak.c | 844 +++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 692 insertions(+), 152 deletions(-)

you may want to add yourself to MAINTAINERs (after talking with
roberto, who i belive has less interrest in cinepak than you do
nowadays)


> 
> diff --git a/libavcodec/cinepak.c b/libavcodec/cinepak.c
> index d657e9c0c1..7b08e20e06 100644
> --- a/libavcodec/cinepak.c
> +++ b/libavcodec/cinepak.c
> @@ -31,6 +31,8 @@
>   *
>   * Cinepak colorspace support (c) 2013 Rl, Aetey Global Technologies AB
>   * @author Cinepak colorspace, Rl, Aetey Global Technologies AB
> + * Extra output formats / optimizations (c) 2017 Rl, Aetey Global Technologies AB
> + * @author Extra output formats / optimizations, Rl, Aetey Global Technologies AB
>   */
>  
>  #include <stdio.h>
> @@ -39,23 +41,48 @@
>  
>  #include "libavutil/common.h"
>  #include "libavutil/intreadwrite.h"
> +#include "libavutil/opt.h"

> +/* #include "libavutil/avassert.h" */

useless commented out code

[...]
> +    switch (avctx->pix_fmt) {
> +    case AV_PIX_FMT_RGB32:
> +        s->decode_codebook = cinepak_decode_codebook_rgb32;
> +        s->decode_vectors  = cinepak_decode_vectors_rgb32;
> +        break;
> +    case AV_PIX_FMT_RGB24:
> +        s->decode_codebook = cinepak_decode_codebook_rgb24;
> +        s->decode_vectors  = cinepak_decode_vectors_rgb24;
> +        break;
> +    case AV_PIX_FMT_RGB565:
> +        s->decode_codebook = cinepak_decode_codebook_rgb565;
> +        s->decode_vectors  = cinepak_decode_vectors_rgb565;
> +        break;
> +    case AV_PIX_FMT_YUV420P:
> +        s->decode_codebook = cinepak_decode_codebook_yuv420;
> +        s->decode_vectors  = cinepak_decode_vectors_yuv420;
> +        break;
> +    case AV_PIX_FMT_PAL8:
> +        if (!s->palette_video) {
> +            av_log(avctx, AV_LOG_ERROR, "Palettized output not supported without palettized input\n");
> +            return AVERROR(EINVAL);
> +        }
> +        s->decode_codebook = cinepak_decode_codebook_pal8;
> +        s->decode_vectors  = cinepak_decode_vectors_pal8;
> +        break;
> +    default:

> +        av_log(avctx, AV_LOG_ERROR, "Unsupported pixel format %d\n", avctx->pix_fmt);

av_get_pix_fmt_name()


[...]

> @@ -488,4 +1026,6 @@ AVCodec ff_cinepak_decoder = {
>      .close          = cinepak_decode_end,
>      .decode         = cinepak_decode_frame,
>      .capabilities   = AV_CODEC_CAP_DR1,
> +    .pix_fmts       = pixfmt_list,

This is possibly unneeded

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170213/29c77684/attachment.sig>


More information about the ffmpeg-devel mailing list