[FFmpeg-devel] [PATCH] yuv8->yuv16 vertical scaler.

Michael Niedermayer michaelni
Sat Aug 15 03:03:15 CEST 2009


On Fri, Aug 14, 2009 at 01:35:56PM -0300, Ramiro Polla wrote:
> On Thu, Aug 13, 2009 at 6:15 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> > On Thu, Aug 13, 2009 at 04:00:28PM -0300, Ramiro Polla wrote:
> >> On Thu, Aug 13, 2009 at 1:10 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> > [...]
> >> > [...]
> >> >> ?swscale.c ? ? ? ? ?| ?101 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >> ?swscale_template.c | ? 20 ++++++++++
> >> >> ?2 files changed, 121 insertions(+)
> >> >> b74deb46c8fb7701b24a0624654e5acae91280db ?yuv8_yuv16.diff
> >> >> diff --git a/swscale.c b/swscale.c
> >> >> index 2adf393..0576dde 100644
> >> >> --- a/swscale.c
> >> >> +++ b/swscale.c
> >> >> @@ -74,6 +74,7 @@ untested special converters
> >> >> ?#include "swscale.h"
> >> >> ?#include "swscale_internal.h"
> >> >> ?#include "rgb2rgb.h"
> >> >> +#include "libavutil/intreadwrite.h"
> >> >> ?#include "libavutil/x86_cpu.h"
> >> >> ?#include "libavutil/bswap.h"
> >> >>
> >> >> @@ -474,6 +475,106 @@ const char *sws_format_name(enum PixelFormat format)
> >> >> ? ? ?}
> >> >> ?}
> >> >>
> >> >> +static av_always_inline void yuv2yuvX16inC_template(const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t **alpSrc, uint16_t *dest, uint16_t *uDest, uint16_t *vDest, uint16_t *aDest, int dstW, int chrDstW,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int big_endian)
> >> >> +{
> >> >> + ? ?//FIXME Optimize (just quickly written not optimized..)
> >> >> + ? ?int i;
> >> >> + ? ?for (i=0; i<dstW; i++)
> >> >> + ? ?{
> >> >> + ? ? ? ?int val=1<<10;
> >> >> + ? ? ? ?int j;
> >> >> + ? ? ? ?for (j=0; j<lumFilterSize; j++)
> >> >> + ? ? ? ? ? ?val += lumSrc[j][i] * lumFilter[j];
> >> >> +
> >> >> + ? ? ? ?if (big_endian) {
> >> >> + ? ? ? ? ? ?AV_WB16(&dest[i], av_clip_uint16(val>>11));
> >> >> + ? ? ? ?} else {
> >> >> + ? ? ? ? ? ?AV_WL16(&dest[i], av_clip_uint16(val>>11));
> >> >> + ? ? ? ?}
> >> >> + ? ?}
> >> >> +
> >> >> + ? ?if (uDest)
> >> >> + ? ? ? ?for (i=0; i<chrDstW; i++)
> >> >> + ? ? ? ?{
> >> >> + ? ? ? ? ? ?int u=1<<10;
> >> >> + ? ? ? ? ? ?int v=1<<10;
> >> >> + ? ? ? ? ? ?int j;
> >> >> + ? ? ? ? ? ?for (j=0; j<chrFilterSize; j++)
> >> >> + ? ? ? ? ? ?{
> >> >> + ? ? ? ? ? ? ? ?u += chrSrc[j][i] * chrFilter[j];
> >> >> + ? ? ? ? ? ? ? ?v += chrSrc[j][i + VOFW] * chrFilter[j];
> >> >> + ? ? ? ? ? ?}
> >> >> +
> >> >> + ? ? ? ? ? ?if (big_endian) {
> >> >> + ? ? ? ? ? ? ? ?AV_WB16(&uDest[i], av_clip_uint16(u>>11));
> >> >> + ? ? ? ? ? ? ? ?AV_WB16(&vDest[i], av_clip_uint16(v>>11));
> >> >> + ? ? ? ? ? ?} else {
> >> >> + ? ? ? ? ? ? ? ?AV_WL16(&uDest[i], av_clip_uint16(u>>11));
> >> >> + ? ? ? ? ? ? ? ?AV_WL16(&vDest[i], av_clip_uint16(v>>11));
> >> >> + ? ? ? ? ? ?}
> >> >> + ? ? ? ?}
> >> >> +
> >> >> + ? ?if (CONFIG_SWSCALE_ALPHA && aDest)
> >> >> + ? ? ? ?for (i=0; i<dstW; i++){
> >> >> + ? ? ? ? ? ?int val=1<<10;
> >> >> + ? ? ? ? ? ?int j;
> >> >> + ? ? ? ? ? ?for (j=0; j<lumFilterSize; j++)
> >> >> + ? ? ? ? ? ? ? ?val += alpSrc[j][i] * lumFilter[j];
> >> >> +
> >> >> + ? ? ? ? ? ?if (big_endian) {
> >> >> + ? ? ? ? ? ? ? ?AV_WB16(&aDest[i], av_clip_uint16(val>>11));
> >> >> + ? ? ? ? ? ?} else {
> >> >> + ? ? ? ? ? ? ? ?AV_WL16(&aDest[i], av_clip_uint16(val>>11));
> >> >> + ? ? ? ? ? ?}
> >> >> + ? ? ? ?}
> >> >> +
> >> >> +}
> >> >> +
> >> >> +static inline void yuv2yuvX16BEinC(const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t **alpSrc, uint16_t *dest, uint16_t *uDest, uint16_t *vDest, uint16_t *aDest, int dstW, int chrDstW)
> >> >> +{
> >> >> + ? ? ? ?yuv2yuvX16inC_template(lumFilter, lumSrc, lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chrFilter, chrSrc, chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? alpSrc,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dest, uDest, vDest, aDest,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dstW, chrDstW, 1);
> >> >> +}
> >> >> +
> >> >> +static inline void yuv2yuvX16LEinC(const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t **alpSrc, uint16_t *dest, uint16_t *uDest, uint16_t *vDest, uint16_t *aDest, int dstW, int chrDstW)
> >> >> +{
> >> >> + ? ? ? ?yuv2yuvX16inC_template(lumFilter, lumSrc, lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? chrFilter, chrSrc, chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? alpSrc,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dest, uDest, vDest, aDest,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dstW, chrDstW, 0);
> >> >> +}
> >> >> +
> >> >> +static inline void yuv2yuvX16inC(const int16_t *lumFilter, const int16_t **lumSrc, int lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t *chrFilter, const int16_t **chrSrc, int chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const int16_t **alpSrc, uint16_t *dest, uint16_t *uDest, uint16_t *vDest, uint16_t *aDest, int dstW, int chrDstW,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? enum PixelFormat dstFormat)
> >> >> +{
> >> >> + ? ?if (isBE(dstFormat))
> >> >> + ? ? ? ?yuv2yuvX16BEinC(lumFilter, lumSrc, lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?chrFilter, chrSrc, chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?alpSrc,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?dest, uDest, vDest, aDest,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?dstW, chrDstW);
> >> >> + ? ?else
> >> >> + ? ? ? ?yuv2yuvX16LEinC(lumFilter, lumSrc, lumFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?chrFilter, chrSrc, chrFilterSize,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?alpSrc,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?dest, uDest, vDest, aDest,
> >> >> + ? ? ? ? ? ? ? ? ? ? ? ?dstW, chrDstW);
> >> >> +
> >> >> +}
> >> >
> >> > isnt there one layer (yuv2yuvX16BEinC/yuv2yuvX16LEinC) too much?
> >>
> >> Indeed, there is... New patch attached.
> >>
> >> Ramiro Polla
> >
> >> ?swscale.c ? ? ? ? ?| ? 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> ?swscale_template.c | ? 20 +++++++++++++
> >> ?2 files changed, 98 insertions(+)
> >> 00fd0417f78244689708b8d7fc238738d4113c97 ?yuv8_yuv16_2ndtry.diff
> >
> > iam ok with it if its tested, though i suspect diego might have some K&R
> > comments that you should follow ...
> 
> I talked with diego on IRC, k&r'd it, and applied.
> 
> Is it ok for the documentation to make it abundantly clear that we do
> in fact enforce a style? (it kind of suggests to keep the current
> style of code maintained by others, when actually we have only one).

we do not enforce a style against the maintainers wishes, especially
not for fine print of the style like where to place spaces
but i as maintainer and author of swscale do prefer a style that puts
{ on the same line as if/for/... nowadays, many years ago i prefered some
different { placement 
as my preferance also is quite similar to what most code of ffmpeg uses
i am in favor of the style being changed toward the prefered and common style

it doesnt mean that other authors would be forced to follow every fine obscure
detail of what indent says is K&R style.
Instead really it IS following what the maintainer prefers + K&R where he
does not have a preferance ...


> 
> Also may I prepare a pretty-printing attack to swscale like Diego did
> some time ago? I was planning on working on functional changes first
> and leave cosmetics for later, but the style pressure has been
> growing, so I thought I'd tackle it now. Should I send in patches, how
> should I split them...?

i have no real oppinion on spliting style adaption patches ...
its doesnt really matter to me, maybe others have some preferance though
...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090815/e10878a0/attachment.pgp>



More information about the ffmpeg-devel mailing list