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

u-9iep at aetey.se u-9iep at aetey.se
Tue Feb 7 19:32:33 EET 2017


On Tue, Feb 07, 2017 at 04:23:50PM +0100, wm4 wrote:
> > Still, given the disapproval of the "code quality" without a tangible
> > criteria to follow, I can hardly take any accomodating steps, barring
> > the omission of the unused code - would this step be enough?
> 
> Bad:
> - dead code

Already slated to be removed, I wrote.

> - code duplication

Would you give me an estimation of how many lines of code are actually
duplicated. I believe you just see the superficial resemblance, not
the differences.

> - not using standard API mechanisms (get_format)

You have to take this back and look at the patch.

> - using unusual mechanisms that are normally not used in FFmpeg

This is the whole point of the improvement.
If doing unusual useful things is a bad style here, I am leaving :)

I do not believe you really insist on this point.

>   libraries or libraries in general (configuration via getenv)

Ehh, wasn't this the "dead code" you complained about above?
Let's strike away this point.

So the only remaining unsettled point is which lines of code are
duplicated / how they are to be refactored.

I wrote about this change not being exactly trivial. May be I am not
fit for this particular task? Give me a hand, show how to do refactoring
without impacting readability and speed?

Otherwise it would be a pity to throw away an improvement just
because the author has his/her limitations.

> It's not so complicated if you make an effort to try to understand.

Indeed.

Friendly yours,
Rune



More information about the ffmpeg-devel mailing list