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

[u-9iep at aetey.se]

On Sat, Feb 25, 2017 at 09:27:05PM +0100, Paul B Mahol wrote:
> On 2/25/17, u-9iep at aetey.se <u-9iep at aetey.se> wrote:
> > Hello,
> >
> > Here comes the latest version of the patch, with adjustments
> > made according to all substantial feedback comments.
> >
> > Among others the code has been further deduplicated to ease maintenance.
> >
> > Hopefully the 2-4 times improvement of the decoding speed justifies the
> > growth of the affected source file (from 491 to 979 lines) and also the
> 
> Unacceptable.

Any comments on the criteria?

Is no code growth acceptable no matter what, or is there possibly
some percentage which would be ok from your perspective for such a speedup?

There are ways to look for a common ground:

It would be trivial to cut the code, cutting at the same time
the class of situations where the speedup would happen.

Excluding pal8 (I am not aware of any usage case for it, though I do not
pretend to know for all the ffmpeg/cinepak users) and yuv420p (hardware
scalers in X11 like it but they are in practice slower than cinepak :)
would remove about half the new code. Further removing rgb24 would
reduce it even more.

But neither me nor you are qualified to estimate the usefulness
of the formats you/me do not use ourselves.

I am qualified to state the need for _fast_ decoding, for the moment using
rgb565 in the first hand and rgba in some other situations. I expect that
the need for the other formats can become vital anytime when hardware
changes. That's another reason why even the formats without a _known_
use have a value and my motivation for having implemented them.

OTOH regarding your negative stance to the patch,

you did not come back when I kindly asked you to comment on these two
fully opposite opinions of yours about the handling of cinepak in ffmeg:

-----------------
https://ffmpeg.org/pipermail/ffmpeg-devel/2013-February/138811.html
Paul B Mahol onemda at gmail.com
Sun Feb 10 19:34:53 CET 2013

> i think hes asking if it should be a new colorspace in swscale, or            
> added into the cinepak decoder/encoder common code.                           
> 
> by your answer i think it has to be a new swscale colorspace?                 

Please leave libswscale alone.

-----------------
https://ffmpeg.org/pipermail/ffmpeg-devel/2017-February/207134.html
Paul B Mahol onemda at gmail.com
Tue Feb 14 11:03:41 EET 2017

Correct way in solving this is outputing in cinepak decoder actual
native format that it
uses and not do any conversions of colorspace which is currently done.
Implement correct colorspace conversions of cinepak format to others
in libswscale.

-----------------

This gives me a reasonable ground to assume that your stance can change.

Friendly yours,
Rune