Re: [FFmpeg-devel] [PATCH v3 1/4] swscale/input: add rgbaf32 input support

2022-11-14 Thread Mark Reid 
On Mon, Nov 14, 2022 at 1:08 PM Michael Niedermayer 
wrote:

> On Sun, Nov 13, 2022 at 05:50:37PM -0800, Mark Reid wrote:
> > On Sun, Nov 13, 2022 at 1:25 PM Michael Niedermayer <
> mich...@niedermayer.cc>
> > wrote:
> >
> > > On Wed, Nov 02, 2022 at 09:00:07PM -0700, mindm...@gmail.com wrote:
> > > > From: Mark Reid 
> > > >
> > > > ---
> > > >  libswscale/input.c | 172
> +
> > > >  libswscale/utils.c |   4 ++
> > > >  2 files changed, 176 insertions(+)
> > > >
> > > > diff --git a/libswscale/input.c b/libswscale/input.c
> > > > index 7ff7bfaa01..4683284b0b 100644
> > > > --- a/libswscale/input.c
> > > > +++ b/libswscale/input.c
> > > > @@ -1284,6 +1284,136 @@ static void
> rgbaf16##endian_name##ToA_c(uint8_t
> > > *_dst, const uint8_t *_src, cons
> > > >  rgbaf16_funcs_endian(le, 0)
> > > >  rgbaf16_funcs_endian(be, 1)
> > > >
> > > > +#define rdpx(src) (is_be ? av_int2float(AV_RB32()):
> > > av_int2float(AV_RL32()))
> > > > +
> > > > +static av_always_inline void rgbaf32ToUV_half_endian(uint16_t *dstU,
> > > uint16_t *dstV, int is_be,
> > > > + const float
> *src,
> > > int width,
> > > > + int32_t
> *rgb2yuv,
> > > int comp)
> > > > +{
> > > > +int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu =
> > > rgb2yuv[BU_IDX];
> > > > +int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv =
> > > rgb2yuv[BV_IDX];
> > > > +int i;
> > > > +for (i = 0; i < width; i++) {
> > >
> > > > +int r = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+0]),
> > > 0.0f, 65535.0f)) +
> > > > + lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+4]),
> > > 0.0f, 65535.0f))) >> 1;
> > > > +int g = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+1]),
> > > 0.0f, 65535.0f)) +
> > > > + lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+5]),
> > > 0.0f, 65535.0f))) >> 1;
> > > > +int b = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+2]),
> > > 0.0f, 65535.0f)) +
> > > > + lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+6]),
> > > 0.0f, 65535.0f))) >> 1;
> > > > +
> > > > +dstU[i] = (ru*r + gu*g + bu*b +
> (0x10001<<(RGB2YUV_SHIFT-1)))
> > > >> RGB2YUV_SHIFT;
> > > > +dstV[i] = (rv*r + gv*g + bv*b +
> (0x10001<<(RGB2YUV_SHIFT-1)))
> > > >> RGB2YUV_SHIFT;
> > >
> > > I would expect this sort of code to use 2 lrintf() and 2 av_clipf()
> not 6
> > >
> > >
> > ya it is a bit excessive, I'll just remove the _half conversions for now,
> > they aren't strictly necessary as far as I can tell.
>
> do you see a problem with just factorizing them out ?
> it shouldnt be hard to reorder the operations
>

It's just fate checksums and float math that make me apprehensive :p.
hmm this code path doesn't actually seem to get tested by fate.
Now that I relook at it, the indexing looks wrong for the 3 channel formats
too.


>
> >
> >
> > >
> > > > +}
> > > > +}
> > > > +
> > > > +static av_always_inline void rgbaf32ToUV_endian(uint16_t *dstU,
> > > uint16_t *dstV, int is_be,
> > > > +const float *src,
> int
> > > width,
> > > > +int32_t *rgb2yuv,
> int
> > > comp)
> > > > +{
> > > > +int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu =
> > > rgb2yuv[BU_IDX];
> > > > +int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv =
> > > rgb2yuv[BV_IDX];
> > > > +int i;
> > > > +for (i = 0; i < width; i++) {
> > > > +int r = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+0]),
> 0.0f,
> > > 65535.0f));
> > > > +int g = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+1]),
> 0.0f,
> > > 65535.0f));
> > > > +int b = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+2]),
> 0.0f,
> > > 65535.0f));
> > > > +
> > > > +dstU[i] = (ru*r + gu*g + bu*b +
> (0x10001<<(RGB2YUV_SHIFT-1)))
> > > >> RGB2YUV_SHIFT;
> > > > +dstV[i] = (rv*r + gv*g + bv*b +
> (0x10001<<(RGB2YUV_SHIFT-1)))
> > > >> RGB2YUV_SHIFT;
> > > > +}
> > > > +}
> > > > +
> > >
> > > > +static av_always_inline void rgbaf32ToY_endian(uint16_t *dst, const
> > > float *src, int is_be,
> > > > +   int width, int32_t
> > > *rgb2yuv, int comp)
> > > > +{
> > > > +int32_t ry = rgb2yuv[RY_IDX], gy = rgb2yuv[GY_IDX], by =
> > > rgb2yuv[BY_IDX];
> > > > +int i;
> > > > +for (i = 0; i < width; i++) {
> > > > +int r = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+0]),
> 0.0f,
> > > 65535.0f));
> > > > +int g = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+1]),
> 0.0f,
> > > 65535.0f));
> > > > +int b = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+2]),
> 0.0f,
> > > 65535.0f));
> > > > +
> > >
> > > > +dst[i] = (ry*r + gy*g + by*b + (0x2001<<(RGB2YUV_SHIFT-1)))
> >>
> > > RGB2YUV_SHIFT;
> > >
> > > there is one output so there should be only need for one

Re: [FFmpeg-devel] [PATCH v3 1/4] swscale/input: add rgbaf32 input support

2022-11-13 Thread Mark Reid 
On Sun, Nov 13, 2022 at 1:25 PM Michael Niedermayer 
wrote:

> On Wed, Nov 02, 2022 at 09:00:07PM -0700, mindm...@gmail.com wrote:
> > From: Mark Reid 
> >
> > ---
> >  libswscale/input.c | 172 +
> >  libswscale/utils.c |   4 ++
> >  2 files changed, 176 insertions(+)
> >
> > diff --git a/libswscale/input.c b/libswscale/input.c
> > index 7ff7bfaa01..4683284b0b 100644
> > --- a/libswscale/input.c
> > +++ b/libswscale/input.c
> > @@ -1284,6 +1284,136 @@ static void rgbaf16##endian_name##ToA_c(uint8_t
> *_dst, const uint8_t *_src, cons
> >  rgbaf16_funcs_endian(le, 0)
> >  rgbaf16_funcs_endian(be, 1)
> >
> > +#define rdpx(src) (is_be ? av_int2float(AV_RB32()):
> av_int2float(AV_RL32()))
> > +
> > +static av_always_inline void rgbaf32ToUV_half_endian(uint16_t *dstU,
> uint16_t *dstV, int is_be,
> > + const float *src,
> int width,
> > + int32_t *rgb2yuv,
> int comp)
> > +{
> > +int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu =
> rgb2yuv[BU_IDX];
> > +int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv =
> rgb2yuv[BV_IDX];
> > +int i;
> > +for (i = 0; i < width; i++) {
>
> > +int r = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+0]),
> 0.0f, 65535.0f)) +
> > + lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+4]),
> 0.0f, 65535.0f))) >> 1;
> > +int g = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+1]),
> 0.0f, 65535.0f)) +
> > + lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+5]),
> 0.0f, 65535.0f))) >> 1;
> > +int b = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+2]),
> 0.0f, 65535.0f)) +
> > + lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+6]),
> 0.0f, 65535.0f))) >> 1;
> > +
> > +dstU[i] = (ru*r + gu*g + bu*b + (0x10001<<(RGB2YUV_SHIFT-1)))
> >> RGB2YUV_SHIFT;
> > +dstV[i] = (rv*r + gv*g + bv*b + (0x10001<<(RGB2YUV_SHIFT-1)))
> >> RGB2YUV_SHIFT;
>
> I would expect this sort of code to use 2 lrintf() and 2 av_clipf() not 6
>
>
ya it is a bit excessive, I'll just remove the _half conversions for now,
they aren't strictly necessary as far as I can tell.


>
> > +}
> > +}
> > +
> > +static av_always_inline void rgbaf32ToUV_endian(uint16_t *dstU,
> uint16_t *dstV, int is_be,
> > +const float *src, int
> width,
> > +int32_t *rgb2yuv, int
> comp)
> > +{
> > +int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu =
> rgb2yuv[BU_IDX];
> > +int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv =
> rgb2yuv[BV_IDX];
> > +int i;
> > +for (i = 0; i < width; i++) {
> > +int r = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+0]), 0.0f,
> 65535.0f));
> > +int g = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+1]), 0.0f,
> 65535.0f));
> > +int b = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+2]), 0.0f,
> 65535.0f));
> > +
> > +dstU[i] = (ru*r + gu*g + bu*b + (0x10001<<(RGB2YUV_SHIFT-1)))
> >> RGB2YUV_SHIFT;
> > +dstV[i] = (rv*r + gv*g + bv*b + (0x10001<<(RGB2YUV_SHIFT-1)))
> >> RGB2YUV_SHIFT;
> > +}
> > +}
> > +
>
> > +static av_always_inline void rgbaf32ToY_endian(uint16_t *dst, const
> float *src, int is_be,
> > +   int width, int32_t
> *rgb2yuv, int comp)
> > +{
> > +int32_t ry = rgb2yuv[RY_IDX], gy = rgb2yuv[GY_IDX], by =
> rgb2yuv[BY_IDX];
> > +int i;
> > +for (i = 0; i < width; i++) {
> > +int r = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+0]), 0.0f,
> 65535.0f));
> > +int g = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+1]), 0.0f,
> 65535.0f));
> > +int b = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+2]), 0.0f,
> 65535.0f));
> > +
>
> > +dst[i] = (ry*r + gy*g + by*b + (0x2001<<(RGB2YUV_SHIFT-1))) >>
> RGB2YUV_SHIFT;
>
> there is one output so there should be only need for one clip and one
> float->int
>

This is matching the f32 planar version. I think I was paranoid about
things being bitexact for tests and that's why it's currently being done
this way.
I'll see what happens if I introduce more float operations, could I perhaps
do this in a later patch? some asm might have to change too.


> thx
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Any man who breaks a law that conscience tells him is unjust and willingly
> accepts the penalty by staying in jail in order to arouse the conscience
> of
> the community on the injustice of the law is at that moment expressing the
> very highest respect for law. - Martin Luther King Jr
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject

[FFmpeg-devel] [PATCH v3 1/4] swscale/input: add rgbaf32 input support

2022-11-02 Thread mindmark 
From: Mark Reid 

---
 libswscale/input.c | 172 +
 libswscale/utils.c |   4 ++
 2 files changed, 176 insertions(+)

diff --git a/libswscale/input.c b/libswscale/input.c
index 7ff7bfaa01..4683284b0b 100644
--- a/libswscale/input.c
+++ b/libswscale/input.c
@@ -1284,6 +1284,136 @@ static void rgbaf16##endian_name##ToA_c(uint8_t *_dst, 
const uint8_t *_src, cons
 rgbaf16_funcs_endian(le, 0)
 rgbaf16_funcs_endian(be, 1)
 
+#define rdpx(src) (is_be ? av_int2float(AV_RB32()): 
av_int2float(AV_RL32()))
+
+static av_always_inline void rgbaf32ToUV_half_endian(uint16_t *dstU, uint16_t 
*dstV, int is_be,
+ const float *src, int 
width,
+ int32_t *rgb2yuv, int 
comp)
+{
+int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu = rgb2yuv[BU_IDX];
+int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv = rgb2yuv[BV_IDX];
+int i;
+for (i = 0; i < width; i++) {
+int r = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+0]), 0.0f, 
65535.0f)) +
+ lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+4]), 0.0f, 
65535.0f))) >> 1;
+int g = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+1]), 0.0f, 
65535.0f)) +
+ lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+5]), 0.0f, 
65535.0f))) >> 1;
+int b = (lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+2]), 0.0f, 
65535.0f)) +
+ lrintf(av_clipf(65535.0f * rdpx(src[i*(comp*2)+6]), 0.0f, 
65535.0f))) >> 1;
+
+dstU[i] = (ru*r + gu*g + bu*b + (0x10001<<(RGB2YUV_SHIFT-1))) >> 
RGB2YUV_SHIFT;
+dstV[i] = (rv*r + gv*g + bv*b + (0x10001<<(RGB2YUV_SHIFT-1))) >> 
RGB2YUV_SHIFT;
+}
+}
+
+static av_always_inline void rgbaf32ToUV_endian(uint16_t *dstU, uint16_t 
*dstV, int is_be,
+const float *src, int width,
+int32_t *rgb2yuv, int comp)
+{
+int32_t ru = rgb2yuv[RU_IDX], gu = rgb2yuv[GU_IDX], bu = rgb2yuv[BU_IDX];
+int32_t rv = rgb2yuv[RV_IDX], gv = rgb2yuv[GV_IDX], bv = rgb2yuv[BV_IDX];
+int i;
+for (i = 0; i < width; i++) {
+int r = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+0]), 0.0f, 
65535.0f));
+int g = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+1]), 0.0f, 
65535.0f));
+int b = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+2]), 0.0f, 
65535.0f));
+
+dstU[i] = (ru*r + gu*g + bu*b + (0x10001<<(RGB2YUV_SHIFT-1))) >> 
RGB2YUV_SHIFT;
+dstV[i] = (rv*r + gv*g + bv*b + (0x10001<<(RGB2YUV_SHIFT-1))) >> 
RGB2YUV_SHIFT;
+}
+}
+
+static av_always_inline void rgbaf32ToY_endian(uint16_t *dst, const float 
*src, int is_be,
+   int width, int32_t *rgb2yuv, 
int comp)
+{
+int32_t ry = rgb2yuv[RY_IDX], gy = rgb2yuv[GY_IDX], by = rgb2yuv[BY_IDX];
+int i;
+for (i = 0; i < width; i++) {
+int r = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+0]), 0.0f, 
65535.0f));
+int g = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+1]), 0.0f, 
65535.0f));
+int b = lrintf(av_clipf(65535.0f * rdpx(src[i*comp+2]), 0.0f, 
65535.0f));
+
+dst[i] = (ry*r + gy*g + by*b + (0x2001<<(RGB2YUV_SHIFT-1))) >> 
RGB2YUV_SHIFT;
+}
+}
+
+static av_always_inline void rgbaf32ToA_endian(uint16_t *dst, const float 
*src, int is_be,
+   int width, void *opq)
+{
+int i;
+for (i=0; isrcFormat;
@@ -1570,6 +1700,18 @@ av_cold void ff_sws_init_input_funcs(SwsContext *c)
 case AV_PIX_FMT_RGBAF16LE:
 c->chrToYV12 = rgbaf16leToUV_half_c;
 break;
+case AV_PIX_FMT_RGBF32BE:
+c->chrToYV12 = rgbf32beToUV_half_c;
+break;
+case AV_PIX_FMT_RGBAF32BE:
+c->chrToYV12 = rgbaf32beToUV_half_c;
+break;
+case AV_PIX_FMT_RGBF32LE:
+c->chrToYV12 = rgbf32leToUV_half_c;
+break;
+case AV_PIX_FMT_RGBAF32LE:
+c->chrToYV12 = rgbaf32leToUV_half_c;
+break;
 }
 } else {
 switch (srcFormat) {
@@ -1663,6 +1805,18 @@ av_cold void ff_sws_init_input_funcs(SwsContext *c)
 case AV_PIX_FMT_RGBAF16LE:
 c->chrToYV12 = rgbaf16leToUV_c;
 break;
+case AV_PIX_FMT_RGBF32BE:
+c->chrToYV12 = rgbf32beToUV_c;
+break;
+case AV_PIX_FMT_RGBAF32BE:
+c->chrToYV12 = rgbaf32beToUV_c;
+break;
+case AV_PIX_FMT_RGBF32LE:
+c->chrToYV12 = rgbf32leToUV_c;
+break;
+case AV_PIX_FMT_RGBAF32LE:
+c->chrToYV12 = rgbaf32leToUV_c;
+break;
 }
 }
 
@@ -1973,6 +2127,18 @@ av_cold void ff_sws_init_input_funcs(SwsContext *c)
 case AV_PIX_FMT_RGBAF16LE:
 c->lumToYV12 = rgbaf16leToY_c;
 break;
+case AV_PIX_FMT_RGBF32BE:
+