Hi Hans
On Tue, Apr 16, 2019 at 6:00 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> Hi Ricardo,
>
> On 16-04-19 14:02, Ricardo Ribalda Delgado wrote:
> > NV12 is a two-plane version YUV 4:2:0, where the U and V components
> > are subsampled 2x2.
> >
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo@xxxxxxxxxxx>
>
> Your patch looks good to me, but it pushed the array-size of
> the supported_src_pixfmts array over 64 entries (right now it is
> exactly 64 entries big).
>
> Which breaks supported_src_formats as that is only 64 bits big:
Good catch. Thanks for detailed description. Will redo and send back
>
> struct v4lconvert_data {
> int fd;
> ...
> int64_t supported_src_formats; /* bitfield */
>
>
> See e.g. :
>
> for (j = 0; j < ARRAY_SIZE(supported_src_pixfmts); j++)
> if (fmt.pixelformat == supported_src_pixfmts[j].fmt)
> break;
>
> if (j < ARRAY_SIZE(supported_src_pixfmts)) {
> data->supported_src_formats |= 1ULL << j;
>
> I believe this is best fixed by a preparation patch which introduces the
> bit-ops from the kernel for this:
>
> include/asm-generic/bitops/non-atomic.h:
>
> static inline void __set_bit(int nr, volatile unsigned long *addr)
> {
> unsigned long mask = BIT_MASK(nr);
> unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
>
> *p |= mask;
> }
>
> static inline void __clear_bit(int nr, volatile unsigned long *addr)
> {
> unsigned long mask = BIT_MASK(nr);
> unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
>
> *p &= ~mask;
> }
>
> static inline int test_bit(int nr, const volatile unsigned long *addr)
> {
> return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
> }
>
> So you will have to change your patch into a 2 patch patch-set with
> the first patch introducing set_bit / clear_bit / test_bit helpers
> modelled after the kernel and changing the declaration of
> supported_src_formats to:
>
> unsigned long supported_src_formats[128 / BITS_PER_LONG];
>
> Thanks & Regards,
>
> Hans
>
>
>
>
>
>
>
>
>
> > ---
> > The code has ben tested with qv4l2 and vivid.
> > v4lconvert_nv12_to_yuv420 has not been tested!!, any suggestions for
> > how to do it?
> >
> > Thanks!
> > lib/libv4lconvert/libv4lconvert-priv.h | 6 +++
> > lib/libv4lconvert/libv4lconvert.c | 19 +++++++++
> > lib/libv4lconvert/rgbyuv.c | 56 ++++++++++++++++++++++++++
> > 3 files changed, 81 insertions(+)
> >
> > diff --git a/lib/libv4lconvert/libv4lconvert-priv.h b/lib/libv4lconvert/libv4lconvert-priv.h
> > index a8046ce2..c45b086e 100644
> > --- a/lib/libv4lconvert/libv4lconvert-priv.h
> > +++ b/lib/libv4lconvert/libv4lconvert-priv.h
> > @@ -285,6 +285,12 @@ void v4lconvert_hm12_to_yuv420(const unsigned char *src,
> > void v4lconvert_hsv_to_rgb24(const unsigned char *src, unsigned char *dest,
> > int width, int height, int bgr, int Xin, unsigned char hsv_enc);
> >
> > +void v4lconvert_nv12_to_rgb24(const unsigned char *src, unsigned char *dest,
> > + int width, int height, int bgr);
> > +
> > +void v4lconvert_nv12_to_yuv420(const unsigned char *src, unsigned char *dest,
> > + int width, int height, int yvu);
> > +
> > void v4lconvert_rotate90(unsigned char *src, unsigned char *dest,
> > struct v4l2_format *fmt);
> >
> > diff --git a/lib/libv4lconvert/libv4lconvert.c b/lib/libv4lconvert/libv4lconvert.c
> > index 78fb3432..2111d19f 100644
> > --- a/lib/libv4lconvert/libv4lconvert.c
> > +++ b/lib/libv4lconvert/libv4lconvert.c
> > @@ -116,6 +116,7 @@ static const struct v4lconvert_pixfmt supported_src_pixfmts[] = {
> > { V4L2_PIX_FMT_SN9C20X_I420, 12, 6, 3, 1 },
> > { V4L2_PIX_FMT_M420, 12, 6, 3, 1 },
> > { V4L2_PIX_FMT_HM12, 12, 6, 3, 1 },
> > + { V4L2_PIX_FMT_NV12, 12, 6, 3, 1 },
> > { V4L2_PIX_FMT_CPIA1, 0, 6, 3, 1 },
> > /* JPEG and variants */
> > { V4L2_PIX_FMT_MJPEG, 0, 7, 7, 0 },
> > @@ -902,6 +903,24 @@ static int v4lconvert_convert_pixfmt(struct v4lconvert_data *data,
> > }
> > break;
> >
> > + /* NV12 formats */
> > + case V4L2_PIX_FMT_NV12:
> > + switch (dest_pix_fmt) {
> > + case V4L2_PIX_FMT_RGB24:
> > + v4lconvert_nv12_to_rgb24(src, dest, width, height, 0);
> > + break;
> > + case V4L2_PIX_FMT_BGR24:
> > + v4lconvert_nv12_to_rgb24(src, dest, width, height, 1);
> > + break;
> > + case V4L2_PIX_FMT_YUV420:
> > + v4lconvert_nv12_to_yuv420(src, dest, width, height, 0);
> > + break;
> > + case V4L2_PIX_FMT_YVU420:
> > + v4lconvert_nv12_to_yuv420(src, dest, width, height, 1);
> > + break;
> > + }
> > + break;
> > +
> > /* compressed bayer formats */
> > case V4L2_PIX_FMT_SPCA561:
> > case V4L2_PIX_FMT_SN9C10X:
> > diff --git a/lib/libv4lconvert/rgbyuv.c b/lib/libv4lconvert/rgbyuv.c
> > index 02c8cb5b..bfe3b15f 100644
> > --- a/lib/libv4lconvert/rgbyuv.c
> > +++ b/lib/libv4lconvert/rgbyuv.c
> > @@ -845,3 +845,59 @@ void v4lconvert_hsv_to_rgb24(const unsigned char *src, unsigned char *dest,
> > src += bppIN;
> > }
> > }
> > +
> > +void v4lconvert_nv12_to_rgb24(const unsigned char *src, unsigned char *dest,
> > + int width, int height, int bgr)
> > +{
> > + int i, j;
> > + const unsigned char *ysrc = src;
> > + const unsigned char *uvsrc = src + width * height;
> > +
> > + for (i = 0; i < height; i++) {
> > + for (j = 0; j < width; j ++) {
> > + if (bgr) {
> > + *dest++ = YUV2B(*ysrc, *uvsrc, *(uvsrc + 1));
> > + *dest++ = YUV2G(*ysrc, *uvsrc, *(uvsrc + 1));
> > + *dest++ = YUV2R(*ysrc, *uvsrc, *(uvsrc + 1));
> > + } else {
> > + *dest++ = YUV2R(*ysrc, *uvsrc, *(uvsrc + 1));
> > + *dest++ = YUV2G(*ysrc, *uvsrc, *(uvsrc + 1));
> > + *dest++ = YUV2B(*ysrc, *uvsrc, *(uvsrc + 1));
> > + }
> > + ysrc++;
> > + if (j&1)
> > + uvsrc += 2;
> > + }
> > +
> > + /* Rewind u and v for next line */
> > + if (!(i&1))
> > + uvsrc -= width;
> > + }
> > +}
> > +
> > +void v4lconvert_nv12_to_yuv420(const unsigned char *src, unsigned char *dest,
> > + int width, int height, int yvu)
> > +{
> > + int i, j;
> > + const unsigned char *ysrc = src;
> > + const unsigned char *uvsrc = src + width * height;
> > + unsigned char *ydst = dest;
> > + unsigned char *udst, *vdst;
> > +
> > + if (yvu) {
> > + vdst = ydst + width * height;
> > + udst = vdst + ((width / 2) * (height / 2));
> > + } else {
> > + udst = ydst + width * height;
> > + vdst = udst + ((width / 2) * (height / 2));
> > + }
> > +
> > + for (i = 0; i < height; i++)
> > + for (j = 0; i < width; j++) {
> > + *ydst++ = *ysrc++;
> > + if (((i % 2) == 0) && ((j % 2) == 0)) {
> > + *udst++ = *uvsrc++;
> > + *vdst++ = *uvsrc++;
> > + }
> > + }
> > +}
> >
--
Ricardo Ribalda
