[FFmpeg-devel] [PATCH] Efficiently support several output pixel formats in Cinepak decoder

[u-9iep at aetey.se]

On Thu, Feb 02, 2017 at 05:52:29PM +0100, wm4 wrote:
> On Thu, 2 Feb 2017 16:59:51 +0100
> u-9iep at aetey.se wrote:
> > On Thu, Feb 02, 2017 at 04:25:05PM +0100, wm4 wrote:
> > > I can see how conversion in the decoder could speed it up somewhat

> > I would not call "twice" for "somewhat" :)
> 
> Well, your original mail mentioned only speedups up to 20%.

I have to recite from the original mail:
"
Avoiding frame pixel format conversion by generating rgb565 in the decoder
for a corresponsing video buffer yields in our tests (on MMX-capable
i*86) more than twice [sic] the playback speed compared to decoding to rgb24.
"

> The heavy code duplication has other downsides. What if someone fixes
> a bug, but only in the rgb32 version and ignores the rgb565 version?

There is no guarantee that this is the same bug or that the same fix
would apply, because the functions are subtly different.

> > Have you got a suggestion how to do avoid this in this case,
> > without sacrificing the speed?
> 
> Is there any value in this at all? It's a very old codec, that was
> apparently used by some video games. What modern application of it
> would there possibly be? And that in addition would require special
> optimizations done by no other codec?

Cinepak is not a "game codec" but a solid video codec and has been used
very widely, for a long time, for a large range of applications.

For some niche applications it still provides the best result, being
a single and far away leader in decoding speed.

On a 16-bit-per-pixel output with a CPU-based decoder you will
not find _any_ over 25% of Cinepak speed. Raw video can not compete
either when indata delivery bandwidth si limited.

It has also an unused improvement margin in the encoder, still keeping
legacy decoders compatibility. The current encoder is already performing
a _way_ better than the proprietary one, so why leave this nice tool
unused where it can help?

> > Any suggestion which can replace this approach?
> 
> get_format would be more appropriate.

get_format() does not belong to the decoder, nor is it up to the task,
which I explained in a recent message.

> > This is very useful when you wish to check that you got it right
> > for a particular visual buffer / device, given that an apllication can
> > try to make its own (and possibly bad) choices. Not critical, I admit.
> 
> Add that to your application code. Or alternatively, make ffmpeg.c
> print the format (this would be useful for a few things).

I would be fine with commenting out these info-messages.

Regards,
Rune