* [PATCH 0/3] Add support to more pixel formats in vicodec.
@ 2018-10-24 18:41 Dafna Hirschfeld
2018-10-24 18:41 ` [PATCH 1/3] media: vicodec: prepare support for various number of planes Dafna Hirschfeld
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Dafna Hirschfeld @ 2018-10-24 18:41 UTC (permalink / raw)
To: mchehab, helen.koike, hverkuil; +Cc: Dafna Hirschfeld, outreachy-kernel
The new supported formats are
V4L2_PIX_FMT_GREY, V4L2_PIX_FMT_ARGB32, V4L2_PIX_FMT_ABGR32.
The returned encoded format is chaned to support various numbers
of planes instead of assuming 3 planes.
The first patch adds new fields to structs.
The second patch adds support for V4L2_PIX_FMT_GREY.
The third patch adds support for V4L2_PIX_FMT_ARGB32, V4L2_PIX_FMT_ABGR32.
Dafna Hirschfeld (3):
media: vicodec: prepare support for various number of planes
media: vicodec: Add support of greyscale format
media: vicodec: Add support for 4 planes formats
drivers/media/platform/vicodec/codec-fwht.c | 73 ++++++++++-----
drivers/media/platform/vicodec/codec-fwht.h | 15 ++-
.../media/platform/vicodec/codec-v4l2-fwht.c | 93 ++++++++++++++-----
.../media/platform/vicodec/codec-v4l2-fwht.h | 1 +
drivers/media/platform/vicodec/vicodec-core.c | 33 +++++--
5 files changed, 157 insertions(+), 58 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] media: vicodec: prepare support for various number of planes
2018-10-24 18:41 [PATCH 0/3] Add support to more pixel formats in vicodec Dafna Hirschfeld
@ 2018-10-24 18:41 ` Dafna Hirschfeld
2018-10-29 9:10 ` Hans Verkuil
2018-10-24 18:41 ` [PATCH 2/3] media: vicodec: Add support of greyscale format Dafna Hirschfeld
2018-10-24 18:41 ` [PATCH 3/3] media: vicodec: Add support for 4 planes formats Dafna Hirschfeld
2 siblings, 1 reply; 8+ messages in thread
From: Dafna Hirschfeld @ 2018-10-24 18:41 UTC (permalink / raw)
To: mchehab, helen.koike, hverkuil; +Cc: Dafna Hirschfeld, outreachy-kernel
Add fields to the structs `fwht_raw_frame`, `v4l2_fwht_pixfmts`
to support various number of planes - formats
with alpha channel that have 4 planes and greyscale formats
that have 1 plane.
Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
drivers/media/platform/vicodec/codec-fwht.h | 4 +-
.../media/platform/vicodec/codec-v4l2-fwht.c | 46 ++++++++++---------
.../media/platform/vicodec/codec-v4l2-fwht.h | 1 +
drivers/media/platform/vicodec/vicodec-core.c | 2 +-
4 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
index 3e9391fec5fe..5750e21f16ac 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -106,7 +106,9 @@ struct fwht_raw_frame {
unsigned int height_div;
unsigned int luma_step;
unsigned int chroma_step;
- u8 *luma, *cb, *cr;
+ unsigned int alpha_step;
+ unsigned int components_num;
+ u8 *luma, *cb, *cr, *alpha;
};
#define FWHT_FRAME_PCODED BIT(0)
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index e5b68fb38aac..64ad01e99bad 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -11,27 +11,28 @@
#include "codec-v4l2-fwht.h"
static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
- { V4L2_PIX_FMT_YUV420, 1, 3, 2, 1, 1, 2, 2 },
- { V4L2_PIX_FMT_YVU420, 1, 3, 2, 1, 1, 2, 2 },
- { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1 },
- { V4L2_PIX_FMT_NV12, 1, 3, 2, 1, 2, 2, 2 },
- { V4L2_PIX_FMT_NV21, 1, 3, 2, 1, 2, 2, 2 },
- { V4L2_PIX_FMT_NV16, 1, 2, 1, 1, 2, 2, 1 },
- { V4L2_PIX_FMT_NV61, 1, 2, 1, 1, 2, 2, 1 },
- { V4L2_PIX_FMT_NV24, 1, 3, 1, 1, 2, 1, 1 },
- { V4L2_PIX_FMT_NV42, 1, 3, 1, 1, 2, 1, 1 },
- { V4L2_PIX_FMT_YUYV, 2, 2, 1, 2, 4, 2, 1 },
- { V4L2_PIX_FMT_YVYU, 2, 2, 1, 2, 4, 2, 1 },
- { V4L2_PIX_FMT_UYVY, 2, 2, 1, 2, 4, 2, 1 },
- { V4L2_PIX_FMT_VYUY, 2, 2, 1, 2, 4, 2, 1 },
- { V4L2_PIX_FMT_BGR24, 3, 3, 1, 3, 3, 1, 1 },
- { V4L2_PIX_FMT_RGB24, 3, 3, 1, 3, 3, 1, 1 },
- { V4L2_PIX_FMT_HSV24, 3, 3, 1, 3, 3, 1, 1 },
- { V4L2_PIX_FMT_BGR32, 4, 4, 1, 4, 4, 1, 1 },
- { V4L2_PIX_FMT_XBGR32, 4, 4, 1, 4, 4, 1, 1 },
- { V4L2_PIX_FMT_RGB32, 4, 4, 1, 4, 4, 1, 1 },
- { V4L2_PIX_FMT_XRGB32, 4, 4, 1, 4, 4, 1, 1 },
- { V4L2_PIX_FMT_HSV32, 4, 4, 1, 4, 4, 1, 1 },
+ { V4L2_PIX_FMT_YUV420, 1, 3, 2, 1, 1, 2, 2, 3},
+ { V4L2_PIX_FMT_YVU420, 1, 3, 2, 1, 1, 2, 2, 3},
+ { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3},
+ { V4L2_PIX_FMT_NV12, 1, 3, 2, 1, 2, 2, 2, 3},
+ { V4L2_PIX_FMT_NV21, 1, 3, 2, 1, 2, 2, 2, 3},
+ { V4L2_PIX_FMT_NV16, 1, 2, 1, 1, 2, 2, 1, 3},
+ { V4L2_PIX_FMT_NV61, 1, 2, 1, 1, 2, 2, 1, 3},
+ { V4L2_PIX_FMT_NV24, 1, 3, 1, 1, 2, 1, 1, 3},
+ { V4L2_PIX_FMT_NV42, 1, 3, 1, 1, 2, 1, 1, 3},
+ { V4L2_PIX_FMT_YUYV, 2, 2, 1, 2, 4, 2, 1, 3},
+ { V4L2_PIX_FMT_YVYU, 2, 2, 1, 2, 4, 2, 1, 3},
+ { V4L2_PIX_FMT_UYVY, 2, 2, 1, 2, 4, 2, 1, 3},
+ { V4L2_PIX_FMT_VYUY, 2, 2, 1, 2, 4, 2, 1, 3},
+ { V4L2_PIX_FMT_BGR24, 3, 3, 1, 3, 3, 1, 1, 3},
+ { V4L2_PIX_FMT_RGB24, 3, 3, 1, 3, 3, 1, 1, 3},
+ { V4L2_PIX_FMT_HSV24, 3, 3, 1, 3, 3, 1, 1, 3},
+ { V4L2_PIX_FMT_BGR32, 4, 4, 1, 4, 4, 1, 1, 3},
+ { V4L2_PIX_FMT_XBGR32, 4, 4, 1, 4, 4, 1, 1, 3},
+ { V4L2_PIX_FMT_RGB32, 4, 4, 1, 4, 4, 1, 1, 3},
+ { V4L2_PIX_FMT_XRGB32, 4, 4, 1, 4, 4, 1, 1, 3},
+ { V4L2_PIX_FMT_HSV32, 4, 4, 1, 4, 4, 1, 1, 3},
+
};
const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat)
@@ -70,6 +71,9 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
rf.height_div = info->height_div;
rf.luma_step = info->luma_step;
rf.chroma_step = info->chroma_step;
+ rf.alpha_step = 0;
+ rf.alpha = NULL;
+ rf.components_num = info->components_num;
switch (info->id) {
case V4L2_PIX_FMT_YUV420:
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
index 162465b78067..aa73d18d69dd 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
@@ -18,6 +18,7 @@ struct v4l2_fwht_pixfmt_info {
/* Chroma plane subsampling */
unsigned int width_div;
unsigned int height_div;
+ unsigned int components_num;
};
struct v4l2_fwht_state {
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 1eb9132bfc85..fb6d5e9a06ab 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -61,7 +61,7 @@ struct pixfmt_info {
};
static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
- V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1
+ V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0
};
static void vicodec_dev_release(struct device *dev)
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] media: vicodec: Add support of greyscale format
2018-10-24 18:41 [PATCH 0/3] Add support to more pixel formats in vicodec Dafna Hirschfeld
2018-10-24 18:41 ` [PATCH 1/3] media: vicodec: prepare support for various number of planes Dafna Hirschfeld
@ 2018-10-24 18:41 ` Dafna Hirschfeld
2018-10-29 9:23 ` Hans Verkuil
2018-10-24 18:41 ` [PATCH 3/3] media: vicodec: Add support for 4 planes formats Dafna Hirschfeld
2 siblings, 1 reply; 8+ messages in thread
From: Dafna Hirschfeld @ 2018-10-24 18:41 UTC (permalink / raw)
To: mchehab, helen.koike, hverkuil; +Cc: Dafna Hirschfeld, outreachy-kernel
Add support for single plane greyscale format V4L2_PIX_FMT_GREY.
Also change the header of the encoded file to include the number
of components.
Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
drivers/media/platform/vicodec/codec-fwht.c | 56 +++++++++++--------
drivers/media/platform/vicodec/codec-fwht.h | 5 +-
.../media/platform/vicodec/codec-v4l2-fwht.c | 12 +++-
drivers/media/platform/vicodec/vicodec-core.c | 21 +++++--
4 files changed, 61 insertions(+), 33 deletions(-)
diff --git a/drivers/media/platform/vicodec/codec-fwht.c b/drivers/media/platform/vicodec/codec-fwht.c
index 36656031b295..91bfd1d9182b 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -753,9 +753,6 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
__be16 *rlco = cf->rlc_data;
__be16 *rlco_max;
u32 encoding;
- u32 chroma_h = frm->height / frm->height_div;
- u32 chroma_w = frm->width / frm->width_div;
- unsigned int chroma_size = chroma_h * chroma_w;
rlco_max = rlco + size / 2 - 256;
encoding = encode_plane(frm->luma, ref_frm->luma, &rlco, rlco_max, cf,
@@ -764,20 +761,27 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
if (encoding & FWHT_FRAME_UNENCODED)
encoding |= FWHT_LUMA_UNENCODED;
encoding &= ~FWHT_FRAME_UNENCODED;
- rlco_max = rlco + chroma_size / 2 - 256;
- encoding |= encode_plane(frm->cb, ref_frm->cb, &rlco, rlco_max, cf,
+
+ if (frm->components_num > 1) {
+ u32 chroma_h = frm->height / frm->height_div;
+ u32 chroma_w = frm->width / frm->width_div;
+ unsigned int chroma_size = chroma_h * chroma_w;
+
+ rlco_max = rlco + chroma_size / 2 - 256;
+ encoding |= encode_plane(frm->cb, ref_frm->cb, &rlco, rlco_max, cf,
chroma_h, chroma_w,
frm->chroma_step, is_intra, next_is_intra);
- if (encoding & FWHT_FRAME_UNENCODED)
- encoding |= FWHT_CB_UNENCODED;
- encoding &= ~FWHT_FRAME_UNENCODED;
- rlco_max = rlco + chroma_size / 2 - 256;
- encoding |= encode_plane(frm->cr, ref_frm->cr, &rlco, rlco_max, cf,
+ if (encoding & FWHT_FRAME_UNENCODED)
+ encoding |= FWHT_CB_UNENCODED;
+ encoding &= ~FWHT_FRAME_UNENCODED;
+ rlco_max = rlco + chroma_size / 2 - 256;
+ encoding |= encode_plane(frm->cr, ref_frm->cr, &rlco, rlco_max, cf,
chroma_h, chroma_w,
frm->chroma_step, is_intra, next_is_intra);
- if (encoding & FWHT_FRAME_UNENCODED)
- encoding |= FWHT_CR_UNENCODED;
- encoding &= ~FWHT_FRAME_UNENCODED;
+ if (encoding & FWHT_FRAME_UNENCODED)
+ encoding |= FWHT_CR_UNENCODED;
+ encoding &= ~FWHT_FRAME_UNENCODED;
+ }
cf->size = (rlco - cf->rlc_data) * sizeof(*rlco);
return encoding;
}
@@ -836,20 +840,24 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
}
void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
- u32 hdr_flags)
+ u32 hdr_flags, unsigned int components_num)
{
const __be16 *rlco = cf->rlc_data;
- u32 h = cf->height / 2;
- u32 w = cf->width / 2;
- if (hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT)
- h *= 2;
- if (hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH)
- w *= 2;
decode_plane(cf, &rlco, ref->luma, cf->height, cf->width,
hdr_flags & FWHT_FL_LUMA_IS_UNCOMPRESSED);
- decode_plane(cf, &rlco, ref->cb, h, w,
- hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
- decode_plane(cf, &rlco, ref->cr, h, w,
- hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
+
+ if (components_num > 1) {
+ u32 h = cf->height;
+ u32 w = cf->width;
+
+ if (!(hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT))
+ h /= 2;
+ if (!(hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH))
+ w /= 2;
+ decode_plane(cf, &rlco, ref->cb, h, w,
+ hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
+ decode_plane(cf, &rlco, ref->cr, h, w,
+ hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
+ }
}
diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
index 5750e21f16ac..5e6813aeaa96 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -56,7 +56,7 @@
#define FWHT_MAGIC1 0x4f4f4f4f
#define FWHT_MAGIC2 0xffffffff
-#define FWHT_VERSION 1
+#define FWHT_VERSION 2
/* Set if this is an interlaced format */
#define FWHT_FL_IS_INTERLACED BIT(0)
@@ -81,6 +81,7 @@ struct fwht_cframe_hdr {
u32 magic2;
__be32 version;
__be32 width, height;
+ u8 components_num;
__be32 flags;
__be32 colorspace;
__be32 xfer_func;
@@ -122,6 +123,6 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
struct fwht_cframe *cf,
bool is_intra, bool next_is_intra);
void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
- u32 hdr_flags);
+ u32 hdr_flags, unsigned int components_num);
#endif
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index 64ad01e99bad..cc9275f3c6cb 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -11,6 +11,7 @@
#include "codec-v4l2-fwht.h"
static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
+ { V4L2_PIX_FMT_GREY, 1, 1, 1, 1, 0, 1, 1, 1},
{ V4L2_PIX_FMT_YUV420, 1, 3, 2, 1, 1, 2, 2, 3},
{ V4L2_PIX_FMT_YVU420, 1, 3, 2, 1, 1, 2, 2, 3},
{ V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3},
@@ -76,6 +77,9 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
rf.components_num = info->components_num;
switch (info->id) {
+ case V4L2_PIX_FMT_GREY:
+ rf.cb = rf.cr = NULL;
+ break;
case V4L2_PIX_FMT_YUV420:
rf.cb = rf.luma + size;
rf.cr = rf.cb + size / 4;
@@ -166,6 +170,7 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
p_hdr->version = htonl(FWHT_VERSION);
p_hdr->width = htonl(cf.width);
p_hdr->height = htonl(cf.height);
+ p_hdr->components_num = info->components_num;
if (encoding & FWHT_LUMA_UNENCODED)
flags |= FWHT_FL_LUMA_IS_UNCOMPRESSED;
if (encoding & FWHT_CB_UNENCODED)
@@ -196,6 +201,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
struct fwht_cframe_hdr *p_hdr;
struct fwht_cframe cf;
u8 *p;
+ unsigned int components_num;
if (!state->info)
return -EINVAL;
@@ -204,6 +210,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
cf.width = ntohl(p_hdr->width);
cf.height = ntohl(p_hdr->height);
flags = ntohl(p_hdr->flags);
+ components_num = p_hdr->components_num;
state->colorspace = ntohl(p_hdr->colorspace);
state->xfer_func = ntohl(p_hdr->xfer_func);
state->ycbcr_enc = ntohl(p_hdr->ycbcr_enc);
@@ -225,9 +232,12 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
if (!(flags & FWHT_FL_CHROMA_FULL_HEIGHT))
chroma_size /= 2;
- fwht_decode_frame(&cf, &state->ref_frame, flags);
+ fwht_decode_frame(&cf, &state->ref_frame, flags, components_num);
switch (state->info->id) {
+ case V4L2_PIX_FMT_GREY:
+ memcpy(p_out, state->ref_frame.luma, size);
+ break;
case V4L2_PIX_FMT_YUV420:
case V4L2_PIX_FMT_YUV422P:
memcpy(p_out, state->ref_frame.luma, size);
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index fb6d5e9a06ab..945b54efd4bd 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -993,6 +993,12 @@ static int vicodec_start_streaming(struct vb2_queue *q,
unsigned int size = q_data->width * q_data->height;
const struct v4l2_fwht_pixfmt_info *info = q_data->info;
unsigned int chroma_div = info->width_div * info->height_div;
+ unsigned int total_planes_size;
+
+ if (info->components_num == 1)
+ total_planes_size = size;
+ else
+ total_planes_size = size + 2 * (size / chroma_div);
q_data->sequence = 0;
@@ -1002,10 +1008,8 @@ static int vicodec_start_streaming(struct vb2_queue *q,
state->width = q_data->width;
state->height = q_data->height;
state->ref_frame.width = state->ref_frame.height = 0;
- state->ref_frame.luma = kvmalloc(size + 2 * size / chroma_div,
- GFP_KERNEL);
- ctx->comp_max_size = size + 2 * size / chroma_div +
- sizeof(struct fwht_cframe_hdr);
+ state->ref_frame.luma = kvmalloc(total_planes_size, GFP_KERNEL);
+ ctx->comp_max_size = total_planes_size + sizeof(struct fwht_cframe_hdr);
state->compressed_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
if (!state->ref_frame.luma || !state->compressed_frame) {
kvfree(state->ref_frame.luma);
@@ -1013,8 +1017,13 @@ static int vicodec_start_streaming(struct vb2_queue *q,
vicodec_return_bufs(q, VB2_BUF_STATE_QUEUED);
return -ENOMEM;
}
- state->ref_frame.cb = state->ref_frame.luma + size;
- state->ref_frame.cr = state->ref_frame.cb + size / chroma_div;
+ if (info->id == V4L2_PIX_FMT_FWHT || info->components_num > 1) {
+ state->ref_frame.cb = state->ref_frame.luma + size;
+ state->ref_frame.cr = state->ref_frame.cb + size / chroma_div;
+ } else {
+ state->ref_frame.cb = NULL;
+ state->ref_frame.cr = NULL;
+ }
ctx->last_src_buf = NULL;
ctx->last_dst_buf = NULL;
state->gop_cnt = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] media: vicodec: Add support for 4 planes formats
2018-10-24 18:41 [PATCH 0/3] Add support to more pixel formats in vicodec Dafna Hirschfeld
2018-10-24 18:41 ` [PATCH 1/3] media: vicodec: prepare support for various number of planes Dafna Hirschfeld
2018-10-24 18:41 ` [PATCH 2/3] media: vicodec: Add support of greyscale format Dafna Hirschfeld
@ 2018-10-24 18:41 ` Dafna Hirschfeld
2018-10-29 9:49 ` Hans Verkuil
2 siblings, 1 reply; 8+ messages in thread
From: Dafna Hirschfeld @ 2018-10-24 18:41 UTC (permalink / raw)
To: mchehab, helen.koike, hverkuil; +Cc: Dafna Hirschfeld, outreachy-kernel
Add support for formats with 4 planes: V4L2_PIX_FMT_ABGR32,
V4L2_PIX_FMT_ARGB32.
Also add alpha plane related flags to the header of the encoded file.
Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
drivers/media/platform/vicodec/codec-fwht.c | 17 +++++++++
drivers/media/platform/vicodec/codec-fwht.h | 6 ++--
.../media/platform/vicodec/codec-v4l2-fwht.c | 35 +++++++++++++++++++
drivers/media/platform/vicodec/vicodec-core.c | 12 ++++++-
4 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/vicodec/codec-fwht.c b/drivers/media/platform/vicodec/codec-fwht.c
index 91bfd1d9182b..36a08d81807b 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -782,6 +782,17 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
encoding |= FWHT_CR_UNENCODED;
encoding &= ~FWHT_FRAME_UNENCODED;
}
+
+ if (frm->components_num == 4) {
+ rlco_max = rlco + size / 2 - 256;
+ encoding = encode_plane(frm->alpha, ref_frm->alpha, &rlco, rlco_max, cf,
+ frm->height, frm->width,
+ frm->alpha_step, is_intra, next_is_intra);
+ if (encoding & FWHT_FRAME_UNENCODED)
+ encoding |= FWHT_ALPHA_UNENCODED;
+ encoding &= ~FWHT_FRAME_UNENCODED;
+ }
+
cf->size = (rlco - cf->rlc_data) * sizeof(*rlco);
return encoding;
}
@@ -860,4 +871,10 @@ void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
decode_plane(cf, &rlco, ref->cr, h, w,
hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
}
+
+
+ if (components_num == 4) {
+ decode_plane(cf, &rlco, ref->alpha, cf->height, cf->width,
+ hdr_flags & FWHT_FL_ALPHA_IS_UNCOMPRESSED);
+ }
}
diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
index 5e6813aeaa96..40ec137cfd4b 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -73,8 +73,9 @@
#define FWHT_FL_LUMA_IS_UNCOMPRESSED BIT(4)
#define FWHT_FL_CB_IS_UNCOMPRESSED BIT(5)
#define FWHT_FL_CR_IS_UNCOMPRESSED BIT(6)
-#define FWHT_FL_CHROMA_FULL_HEIGHT BIT(7)
-#define FWHT_FL_CHROMA_FULL_WIDTH BIT(8)
+#define FWHT_FL_ALPHA_IS_UNCOMPRESSED BIT(7)
+#define FWHT_FL_CHROMA_FULL_HEIGHT BIT(8)
+#define FWHT_FL_CHROMA_FULL_WIDTH BIT(9)
struct fwht_cframe_hdr {
u32 magic1;
@@ -117,6 +118,7 @@ struct fwht_raw_frame {
#define FWHT_LUMA_UNENCODED BIT(2)
#define FWHT_CB_UNENCODED BIT(3)
#define FWHT_CR_UNENCODED BIT(4)
+#define FWHT_ALPHA_UNENCODED BIT(5)
u32 fwht_encode_frame(struct fwht_raw_frame *frm,
struct fwht_raw_frame *ref_frm,
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index cc9275f3c6cb..42e29a4343c2 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -33,6 +33,8 @@ static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
{ V4L2_PIX_FMT_RGB32, 4, 4, 1, 4, 4, 1, 1, 3},
{ V4L2_PIX_FMT_XRGB32, 4, 4, 1, 4, 4, 1, 1, 3},
{ V4L2_PIX_FMT_HSV32, 4, 4, 1, 4, 4, 1, 1, 3},
+ { V4L2_PIX_FMT_ARGB32, 4, 4, 1, 4, 4, 1, 1, 4},
+ { V4L2_PIX_FMT_ABGR32, 4, 4, 1, 4, 4, 1, 1, 4},
};
@@ -146,6 +148,20 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
rf.cr = rf.cb + 2;
rf.luma++;
break;
+ case V4L2_PIX_FMT_ABGR32:
+ rf.cb = rf.luma;
+ rf.cr = rf.cb + 2;
+ rf.luma++;
+ rf.alpha = rf.cr + 1;
+ rf.alpha_step = rf.luma_step;
+ break;
+ case V4L2_PIX_FMT_ARGB32:
+ rf.alpha = rf.luma;
+ rf.cr = rf.luma + 1;
+ rf.cb = rf.cr + 2;
+ rf.luma += 2;
+ rf.alpha_step = rf.luma_step;
+ break;
default:
return -EINVAL;
}
@@ -177,6 +193,8 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
flags |= FWHT_FL_CB_IS_UNCOMPRESSED;
if (encoding & FWHT_CR_UNENCODED)
flags |= FWHT_FL_CR_IS_UNCOMPRESSED;
+ if (encoding & FWHT_ALPHA_UNENCODED)
+ flags |= FWHT_FL_ALPHA_IS_UNCOMPRESSED;
if (rf.height_div == 1)
flags |= FWHT_FL_CHROMA_FULL_HEIGHT;
if (rf.width_div == 1)
@@ -339,6 +357,23 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
*p++ = 0;
}
break;
+ case V4L2_PIX_FMT_ABGR32:
+ for (i = 0, p = p_out; i < size; i++) {
+ *p++ = state->ref_frame.cb[i];
+ *p++ = state->ref_frame.luma[i];
+ *p++ = state->ref_frame.cr[i];
+ *p++ = state->ref_frame.alpha[i];
+ }
+ break;
+
+ case V4L2_PIX_FMT_ARGB32:
+ for (i = 0, p = p_out; i < size; i++) {
+ *p++ = state->ref_frame.alpha[i];
+ *p++ = state->ref_frame.cr[i];
+ *p++ = state->ref_frame.luma[i];
+ *p++ = state->ref_frame.cb[i];
+ }
+ break;
default:
return -EINVAL;
}
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 945b54efd4bd..2cfd3b85fa26 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -995,7 +995,11 @@ static int vicodec_start_streaming(struct vb2_queue *q,
unsigned int chroma_div = info->width_div * info->height_div;
unsigned int total_planes_size;
- if (info->components_num == 1)
+ //we don't know ahead how many components are in the encoding type
+ //V4L2_PIX_FMT_FWHT, so we will allocate space for 4 planes.
+ if (info->id == V4L2_PIX_FMT_FWHT || info->components_num == 4)
+ total_planes_size = 2 * size + 2 * (size / chroma_div);
+ else if (info->components_num == 1)
total_planes_size = size;
else
total_planes_size = size + 2 * (size / chroma_div);
@@ -1024,6 +1028,12 @@ static int vicodec_start_streaming(struct vb2_queue *q,
state->ref_frame.cb = NULL;
state->ref_frame.cr = NULL;
}
+
+ if (info->id == V4L2_PIX_FMT_FWHT || info->components_num == 4)
+ state->ref_frame.alpha = state->ref_frame.cr + size / chroma_div;
+ else
+ state->ref_frame.alpha = NULL;
+
ctx->last_src_buf = NULL;
ctx->last_dst_buf = NULL;
state->gop_cnt = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] media: vicodec: prepare support for various number of planes
2018-10-24 18:41 ` [PATCH 1/3] media: vicodec: prepare support for various number of planes Dafna Hirschfeld
@ 2018-10-29 9:10 ` Hans Verkuil
0 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2018-10-29 9:10 UTC (permalink / raw)
To: Dafna Hirschfeld, mchehab, helen.koike; +Cc: outreachy-kernel
Hi Dafna,
Here is a quick review:
On 10/24/2018 08:41 PM, Dafna Hirschfeld wrote:
> Add fields to the structs `fwht_raw_frame`, `v4l2_fwht_pixfmts`
> to support various number of planes - formats
> with alpha channel that have 4 planes and greyscale formats
> that have 1 plane.
>
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
> drivers/media/platform/vicodec/codec-fwht.h | 4 +-
> .../media/platform/vicodec/codec-v4l2-fwht.c | 46 ++++++++++---------
> .../media/platform/vicodec/codec-v4l2-fwht.h | 1 +
> drivers/media/platform/vicodec/vicodec-core.c | 2 +-
> 4 files changed, 30 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
> index 3e9391fec5fe..5750e21f16ac 100644
> --- a/drivers/media/platform/vicodec/codec-fwht.h
> +++ b/drivers/media/platform/vicodec/codec-fwht.h
> @@ -106,7 +106,9 @@ struct fwht_raw_frame {
> unsigned int height_div;
> unsigned int luma_step;
> unsigned int chroma_step;
> - u8 *luma, *cb, *cr;
> + unsigned int alpha_step;
You can drop alpha_step: it will always be the same as luma_step.
You can, however, rename luma_step to luma_alpha_step. That would make
sense.
> + unsigned int components_num;
> + u8 *luma, *cb, *cr, *alpha;
> };
>
> #define FWHT_FRAME_PCODED BIT(0)
> diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> index e5b68fb38aac..64ad01e99bad 100644
> --- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> +++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> @@ -11,27 +11,28 @@
> #include "codec-v4l2-fwht.h"
>
> static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
> - { V4L2_PIX_FMT_YUV420, 1, 3, 2, 1, 1, 2, 2 },
> - { V4L2_PIX_FMT_YVU420, 1, 3, 2, 1, 1, 2, 2 },
> - { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1 },
> - { V4L2_PIX_FMT_NV12, 1, 3, 2, 1, 2, 2, 2 },
> - { V4L2_PIX_FMT_NV21, 1, 3, 2, 1, 2, 2, 2 },
> - { V4L2_PIX_FMT_NV16, 1, 2, 1, 1, 2, 2, 1 },
> - { V4L2_PIX_FMT_NV61, 1, 2, 1, 1, 2, 2, 1 },
> - { V4L2_PIX_FMT_NV24, 1, 3, 1, 1, 2, 1, 1 },
> - { V4L2_PIX_FMT_NV42, 1, 3, 1, 1, 2, 1, 1 },
> - { V4L2_PIX_FMT_YUYV, 2, 2, 1, 2, 4, 2, 1 },
> - { V4L2_PIX_FMT_YVYU, 2, 2, 1, 2, 4, 2, 1 },
> - { V4L2_PIX_FMT_UYVY, 2, 2, 1, 2, 4, 2, 1 },
> - { V4L2_PIX_FMT_VYUY, 2, 2, 1, 2, 4, 2, 1 },
> - { V4L2_PIX_FMT_BGR24, 3, 3, 1, 3, 3, 1, 1 },
> - { V4L2_PIX_FMT_RGB24, 3, 3, 1, 3, 3, 1, 1 },
> - { V4L2_PIX_FMT_HSV24, 3, 3, 1, 3, 3, 1, 1 },
> - { V4L2_PIX_FMT_BGR32, 4, 4, 1, 4, 4, 1, 1 },
> - { V4L2_PIX_FMT_XBGR32, 4, 4, 1, 4, 4, 1, 1 },
> - { V4L2_PIX_FMT_RGB32, 4, 4, 1, 4, 4, 1, 1 },
> - { V4L2_PIX_FMT_XRGB32, 4, 4, 1, 4, 4, 1, 1 },
> - { V4L2_PIX_FMT_HSV32, 4, 4, 1, 4, 4, 1, 1 },
> + { V4L2_PIX_FMT_YUV420, 1, 3, 2, 1, 1, 2, 2, 3},
> + { V4L2_PIX_FMT_YVU420, 1, 3, 2, 1, 1, 2, 2, 3},
> + { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3},
> + { V4L2_PIX_FMT_NV12, 1, 3, 2, 1, 2, 2, 2, 3},
> + { V4L2_PIX_FMT_NV21, 1, 3, 2, 1, 2, 2, 2, 3},
> + { V4L2_PIX_FMT_NV16, 1, 2, 1, 1, 2, 2, 1, 3},
> + { V4L2_PIX_FMT_NV61, 1, 2, 1, 1, 2, 2, 1, 3},
> + { V4L2_PIX_FMT_NV24, 1, 3, 1, 1, 2, 1, 1, 3},
> + { V4L2_PIX_FMT_NV42, 1, 3, 1, 1, 2, 1, 1, 3},
> + { V4L2_PIX_FMT_YUYV, 2, 2, 1, 2, 4, 2, 1, 3},
> + { V4L2_PIX_FMT_YVYU, 2, 2, 1, 2, 4, 2, 1, 3},
> + { V4L2_PIX_FMT_UYVY, 2, 2, 1, 2, 4, 2, 1, 3},
> + { V4L2_PIX_FMT_VYUY, 2, 2, 1, 2, 4, 2, 1, 3},
> + { V4L2_PIX_FMT_BGR24, 3, 3, 1, 3, 3, 1, 1, 3},
> + { V4L2_PIX_FMT_RGB24, 3, 3, 1, 3, 3, 1, 1, 3},
> + { V4L2_PIX_FMT_HSV24, 3, 3, 1, 3, 3, 1, 1, 3},
> + { V4L2_PIX_FMT_BGR32, 4, 4, 1, 4, 4, 1, 1, 3},
> + { V4L2_PIX_FMT_XBGR32, 4, 4, 1, 4, 4, 1, 1, 3},
> + { V4L2_PIX_FMT_RGB32, 4, 4, 1, 4, 4, 1, 1, 3},
> + { V4L2_PIX_FMT_XRGB32, 4, 4, 1, 4, 4, 1, 1, 3},
> + { V4L2_PIX_FMT_HSV32, 4, 4, 1, 4, 4, 1, 1, 3},
> +
> };
>
> const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat)
> @@ -70,6 +71,9 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> rf.height_div = info->height_div;
> rf.luma_step = info->luma_step;
> rf.chroma_step = info->chroma_step;
> + rf.alpha_step = 0;
> + rf.alpha = NULL;
> + rf.components_num = info->components_num;
>
> switch (info->id) {
> case V4L2_PIX_FMT_YUV420:
> diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
> index 162465b78067..aa73d18d69dd 100644
> --- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
> +++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
> @@ -18,6 +18,7 @@ struct v4l2_fwht_pixfmt_info {
> /* Chroma plane subsampling */
> unsigned int width_div;
> unsigned int height_div;
> + unsigned int components_num;
> };
>
> struct v4l2_fwht_state {
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 1eb9132bfc85..fb6d5e9a06ab 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -61,7 +61,7 @@ struct pixfmt_info {
> };
>
> static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
> - V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1
> + V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0
> };
>
> static void vicodec_dev_release(struct device *dev)
>
Regards,
Hans
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] media: vicodec: Add support of greyscale format
2018-10-24 18:41 ` [PATCH 2/3] media: vicodec: Add support of greyscale format Dafna Hirschfeld
@ 2018-10-29 9:23 ` Hans Verkuil
2018-10-30 15:52 ` Dafna Hirschfeld
0 siblings, 1 reply; 8+ messages in thread
From: Hans Verkuil @ 2018-10-29 9:23 UTC (permalink / raw)
To: Dafna Hirschfeld, mchehab, helen.koike; +Cc: outreachy-kernel
Hi Dafna,
Looks good overall, but I do have some - mostly minor - comments:
On 10/24/2018 08:41 PM, Dafna Hirschfeld wrote:
> Add support for single plane greyscale format V4L2_PIX_FMT_GREY.
> Also change the header of the encoded file to include the number
> of components.
>
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
> drivers/media/platform/vicodec/codec-fwht.c | 56 +++++++++++--------
> drivers/media/platform/vicodec/codec-fwht.h | 5 +-
> .../media/platform/vicodec/codec-v4l2-fwht.c | 12 +++-
> drivers/media/platform/vicodec/vicodec-core.c | 21 +++++--
> 4 files changed, 61 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/media/platform/vicodec/codec-fwht.c b/drivers/media/platform/vicodec/codec-fwht.c
> index 36656031b295..91bfd1d9182b 100644
> --- a/drivers/media/platform/vicodec/codec-fwht.c
> +++ b/drivers/media/platform/vicodec/codec-fwht.c
> @@ -753,9 +753,6 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> __be16 *rlco = cf->rlc_data;
> __be16 *rlco_max;
> u32 encoding;
> - u32 chroma_h = frm->height / frm->height_div;
> - u32 chroma_w = frm->width / frm->width_div;
> - unsigned int chroma_size = chroma_h * chroma_w;
>
> rlco_max = rlco + size / 2 - 256;
> encoding = encode_plane(frm->luma, ref_frm->luma, &rlco, rlco_max, cf,
> @@ -764,20 +761,27 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> if (encoding & FWHT_FRAME_UNENCODED)
> encoding |= FWHT_LUMA_UNENCODED;
> encoding &= ~FWHT_FRAME_UNENCODED;
> - rlco_max = rlco + chroma_size / 2 - 256;
> - encoding |= encode_plane(frm->cb, ref_frm->cb, &rlco, rlco_max, cf,
> +
> + if (frm->components_num > 1) {
Just to be on the safe side, change this to:
frm->components_num >= 3
since the code inside the if statements assumes that there are at least
three components and it would fail if components_num == 2.
> + u32 chroma_h = frm->height / frm->height_div;
> + u32 chroma_w = frm->width / frm->width_div;
> + unsigned int chroma_size = chroma_h * chroma_w;
> +
> + rlco_max = rlco + chroma_size / 2 - 256;
> + encoding |= encode_plane(frm->cb, ref_frm->cb, &rlco, rlco_max, cf,
> chroma_h, chroma_w,
> frm->chroma_step, is_intra, next_is_intra);
> - if (encoding & FWHT_FRAME_UNENCODED)
> - encoding |= FWHT_CB_UNENCODED;
> - encoding &= ~FWHT_FRAME_UNENCODED;
> - rlco_max = rlco + chroma_size / 2 - 256;
> - encoding |= encode_plane(frm->cr, ref_frm->cr, &rlco, rlco_max, cf,
> + if (encoding & FWHT_FRAME_UNENCODED)
> + encoding |= FWHT_CB_UNENCODED;
> + encoding &= ~FWHT_FRAME_UNENCODED;
> + rlco_max = rlco + chroma_size / 2 - 256;
> + encoding |= encode_plane(frm->cr, ref_frm->cr, &rlco, rlco_max, cf,
> chroma_h, chroma_w,
> frm->chroma_step, is_intra, next_is_intra);
> - if (encoding & FWHT_FRAME_UNENCODED)
> - encoding |= FWHT_CR_UNENCODED;
> - encoding &= ~FWHT_FRAME_UNENCODED;
> + if (encoding & FWHT_FRAME_UNENCODED)
> + encoding |= FWHT_CR_UNENCODED;
> + encoding &= ~FWHT_FRAME_UNENCODED;
> + }
> cf->size = (rlco - cf->rlc_data) * sizeof(*rlco);
> return encoding;
> }
> @@ -836,20 +840,24 @@ static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
> }
>
> void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
> - u32 hdr_flags)
> + u32 hdr_flags, unsigned int components_num)
> {
> const __be16 *rlco = cf->rlc_data;
> - u32 h = cf->height / 2;
> - u32 w = cf->width / 2;
>
> - if (hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT)
> - h *= 2;
> - if (hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH)
> - w *= 2;
> decode_plane(cf, &rlco, ref->luma, cf->height, cf->width,
> hdr_flags & FWHT_FL_LUMA_IS_UNCOMPRESSED);
> - decode_plane(cf, &rlco, ref->cb, h, w,
> - hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
> - decode_plane(cf, &rlco, ref->cr, h, w,
> - hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
> +
> + if (components_num > 1) {
Ditto: >= 3
> + u32 h = cf->height;
> + u32 w = cf->width;
> +
> + if (!(hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT))
> + h /= 2;
> + if (!(hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH))
> + w /= 2;
> + decode_plane(cf, &rlco, ref->cb, h, w,
> + hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
> + decode_plane(cf, &rlco, ref->cr, h, w,
> + hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
> + }
> }
> diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
> index 5750e21f16ac..5e6813aeaa96 100644
> --- a/drivers/media/platform/vicodec/codec-fwht.h
> +++ b/drivers/media/platform/vicodec/codec-fwht.h
> @@ -56,7 +56,7 @@
> #define FWHT_MAGIC1 0x4f4f4f4f
> #define FWHT_MAGIC2 0xffffffff
>
> -#define FWHT_VERSION 1
> +#define FWHT_VERSION 2
>
> /* Set if this is an interlaced format */
> #define FWHT_FL_IS_INTERLACED BIT(0)
> @@ -81,6 +81,7 @@ struct fwht_cframe_hdr {
> u32 magic2;
> __be32 version;
> __be32 width, height;
> + u8 components_num;
Rather than adding a new field, I prefer that it is encoded in the flags field.
Take two bits (say bits 16 and 17) where you store the number of components - 1.
This simplifies decoders since they don't have to deal with different structure
sizes.
> __be32 flags;
> __be32 colorspace;
> __be32 xfer_func;
> @@ -122,6 +123,6 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> struct fwht_cframe *cf,
> bool is_intra, bool next_is_intra);
> void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
> - u32 hdr_flags);
> + u32 hdr_flags, unsigned int components_num);
>
> #endif
> diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> index 64ad01e99bad..cc9275f3c6cb 100644
> --- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> +++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> @@ -11,6 +11,7 @@
> #include "codec-v4l2-fwht.h"
>
> static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
> + { V4L2_PIX_FMT_GREY, 1, 1, 1, 1, 0, 1, 1, 1},
> { V4L2_PIX_FMT_YUV420, 1, 3, 2, 1, 1, 2, 2, 3},
> { V4L2_PIX_FMT_YVU420, 1, 3, 2, 1, 1, 2, 2, 3},
> { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3},
> @@ -76,6 +77,9 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> rf.components_num = info->components_num;
>
> switch (info->id) {
> + case V4L2_PIX_FMT_GREY:
> + rf.cb = rf.cr = NULL;
> + break;
> case V4L2_PIX_FMT_YUV420:
> rf.cb = rf.luma + size;
> rf.cr = rf.cb + size / 4;
> @@ -166,6 +170,7 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> p_hdr->version = htonl(FWHT_VERSION);
> p_hdr->width = htonl(cf.width);
> p_hdr->height = htonl(cf.height);
> + p_hdr->components_num = info->components_num;
> if (encoding & FWHT_LUMA_UNENCODED)
> flags |= FWHT_FL_LUMA_IS_UNCOMPRESSED;
> if (encoding & FWHT_CB_UNENCODED)
> @@ -196,6 +201,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> struct fwht_cframe_hdr *p_hdr;
> struct fwht_cframe cf;
> u8 *p;
> + unsigned int components_num;
This should default to 3.
>
> if (!state->info)
> return -EINVAL;
> @@ -204,6 +210,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> cf.width = ntohl(p_hdr->width);
> cf.height = ntohl(p_hdr->height);
> flags = ntohl(p_hdr->flags);
> + components_num = p_hdr->components_num;
Only read this if the version is 2. If the version is 1, then it defaults to 3.
And of course it should read the flags for the number of components as mentioned
above.
> state->colorspace = ntohl(p_hdr->colorspace);
> state->xfer_func = ntohl(p_hdr->xfer_func);
> state->ycbcr_enc = ntohl(p_hdr->ycbcr_enc);
> @@ -225,9 +232,12 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> if (!(flags & FWHT_FL_CHROMA_FULL_HEIGHT))
> chroma_size /= 2;
>
> - fwht_decode_frame(&cf, &state->ref_frame, flags);
> + fwht_decode_frame(&cf, &state->ref_frame, flags, components_num);
>
> switch (state->info->id) {
> + case V4L2_PIX_FMT_GREY:
> + memcpy(p_out, state->ref_frame.luma, size);
> + break;
> case V4L2_PIX_FMT_YUV420:
> case V4L2_PIX_FMT_YUV422P:
> memcpy(p_out, state->ref_frame.luma, size);
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index fb6d5e9a06ab..945b54efd4bd 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -993,6 +993,12 @@ static int vicodec_start_streaming(struct vb2_queue *q,
> unsigned int size = q_data->width * q_data->height;
> const struct v4l2_fwht_pixfmt_info *info = q_data->info;
> unsigned int chroma_div = info->width_div * info->height_div;
> + unsigned int total_planes_size;
> +
> + if (info->components_num == 1)
Use < 3
> + total_planes_size = size;
> + else
> + total_planes_size = size + 2 * (size / chroma_div);
>
> q_data->sequence = 0;
>
> @@ -1002,10 +1008,8 @@ static int vicodec_start_streaming(struct vb2_queue *q,
> state->width = q_data->width;
> state->height = q_data->height;
> state->ref_frame.width = state->ref_frame.height = 0;
> - state->ref_frame.luma = kvmalloc(size + 2 * size / chroma_div,
> - GFP_KERNEL);
> - ctx->comp_max_size = size + 2 * size / chroma_div +
> - sizeof(struct fwht_cframe_hdr);
> + state->ref_frame.luma = kvmalloc(total_planes_size, GFP_KERNEL);
> + ctx->comp_max_size = total_planes_size + sizeof(struct fwht_cframe_hdr);
> state->compressed_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
> if (!state->ref_frame.luma || !state->compressed_frame) {
> kvfree(state->ref_frame.luma);
> @@ -1013,8 +1017,13 @@ static int vicodec_start_streaming(struct vb2_queue *q,
> vicodec_return_bufs(q, VB2_BUF_STATE_QUEUED);
> return -ENOMEM;
> }
> - state->ref_frame.cb = state->ref_frame.luma + size;
> - state->ref_frame.cr = state->ref_frame.cb + size / chroma_div;
> + if (info->id == V4L2_PIX_FMT_FWHT || info->components_num > 1) {
>= 3
> + state->ref_frame.cb = state->ref_frame.luma + size;
> + state->ref_frame.cr = state->ref_frame.cb + size / chroma_div;
> + } else {
> + state->ref_frame.cb = NULL;
> + state->ref_frame.cr = NULL;
> + }
> ctx->last_src_buf = NULL;
> ctx->last_dst_buf = NULL;
> state->gop_cnt = 0;
>
Regards,
Hans
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] media: vicodec: Add support for 4 planes formats
2018-10-24 18:41 ` [PATCH 3/3] media: vicodec: Add support for 4 planes formats Dafna Hirschfeld
@ 2018-10-29 9:49 ` Hans Verkuil
0 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2018-10-29 9:49 UTC (permalink / raw)
To: Dafna Hirschfeld, mchehab, helen.koike; +Cc: outreachy-kernel
Hi Dafna,
Some comments below. Note that (at mentioned in the first patch) alpha_step can be
dropped and luma_step should be renamed to luma_alpha_step.
On 10/24/2018 08:41 PM, Dafna Hirschfeld wrote:
> Add support for formats with 4 planes: V4L2_PIX_FMT_ABGR32,
> V4L2_PIX_FMT_ARGB32.
> Also add alpha plane related flags to the header of the encoded file.
>
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
> drivers/media/platform/vicodec/codec-fwht.c | 17 +++++++++
> drivers/media/platform/vicodec/codec-fwht.h | 6 ++--
> .../media/platform/vicodec/codec-v4l2-fwht.c | 35 +++++++++++++++++++
> drivers/media/platform/vicodec/vicodec-core.c | 12 ++++++-
> 4 files changed, 67 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/vicodec/codec-fwht.c b/drivers/media/platform/vicodec/codec-fwht.c
> index 91bfd1d9182b..36a08d81807b 100644
> --- a/drivers/media/platform/vicodec/codec-fwht.c
> +++ b/drivers/media/platform/vicodec/codec-fwht.c
> @@ -782,6 +782,17 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> encoding |= FWHT_CR_UNENCODED;
> encoding &= ~FWHT_FRAME_UNENCODED;
> }
> +
> + if (frm->components_num == 4) {
> + rlco_max = rlco + size / 2 - 256;
> + encoding = encode_plane(frm->alpha, ref_frm->alpha, &rlco, rlco_max, cf,
> + frm->height, frm->width,
> + frm->alpha_step, is_intra, next_is_intra);
> + if (encoding & FWHT_FRAME_UNENCODED)
> + encoding |= FWHT_ALPHA_UNENCODED;
> + encoding &= ~FWHT_FRAME_UNENCODED;
> + }
> +
> cf->size = (rlco - cf->rlc_data) * sizeof(*rlco);
> return encoding;
> }
> @@ -860,4 +871,10 @@ void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
> decode_plane(cf, &rlco, ref->cr, h, w,
> hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
> }
> +
> +
> + if (components_num == 4) {
> + decode_plane(cf, &rlco, ref->alpha, cf->height, cf->width,
> + hdr_flags & FWHT_FL_ALPHA_IS_UNCOMPRESSED);
> + }
No need for { }: checkpatch.pl --strict should have warned about this, I think.
> }
> diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
> index 5e6813aeaa96..40ec137cfd4b 100644
> --- a/drivers/media/platform/vicodec/codec-fwht.h
> +++ b/drivers/media/platform/vicodec/codec-fwht.h
> @@ -73,8 +73,9 @@
> #define FWHT_FL_LUMA_IS_UNCOMPRESSED BIT(4)
> #define FWHT_FL_CB_IS_UNCOMPRESSED BIT(5)
> #define FWHT_FL_CR_IS_UNCOMPRESSED BIT(6)
> -#define FWHT_FL_CHROMA_FULL_HEIGHT BIT(7)
> -#define FWHT_FL_CHROMA_FULL_WIDTH BIT(8)
> +#define FWHT_FL_ALPHA_IS_UNCOMPRESSED BIT(7)
> +#define FWHT_FL_CHROMA_FULL_HEIGHT BIT(8)
> +#define FWHT_FL_CHROMA_FULL_WIDTH BIT(9)
Don't change existing flags, just add the new flag at the end. Otherwise
you would have problems interpreting different versions of the same struct.
>
> struct fwht_cframe_hdr {
> u32 magic1;
> @@ -117,6 +118,7 @@ struct fwht_raw_frame {
> #define FWHT_LUMA_UNENCODED BIT(2)
> #define FWHT_CB_UNENCODED BIT(3)
> #define FWHT_CR_UNENCODED BIT(4)
> +#define FWHT_ALPHA_UNENCODED BIT(5)
>
> u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> struct fwht_raw_frame *ref_frm,
> diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> index cc9275f3c6cb..42e29a4343c2 100644
> --- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> +++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> @@ -33,6 +33,8 @@ static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
> { V4L2_PIX_FMT_RGB32, 4, 4, 1, 4, 4, 1, 1, 3},
> { V4L2_PIX_FMT_XRGB32, 4, 4, 1, 4, 4, 1, 1, 3},
> { V4L2_PIX_FMT_HSV32, 4, 4, 1, 4, 4, 1, 1, 3},
> + { V4L2_PIX_FMT_ARGB32, 4, 4, 1, 4, 4, 1, 1, 4},
> + { V4L2_PIX_FMT_ABGR32, 4, 4, 1, 4, 4, 1, 1, 4},
>
> };
>
> @@ -146,6 +148,20 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> rf.cr = rf.cb + 2;
> rf.luma++;
> break;
> + case V4L2_PIX_FMT_ABGR32:
> + rf.cb = rf.luma;
> + rf.cr = rf.cb + 2;
> + rf.luma++;
> + rf.alpha = rf.cr + 1;
> + rf.alpha_step = rf.luma_step;
> + break;
> + case V4L2_PIX_FMT_ARGB32:
> + rf.alpha = rf.luma;
> + rf.cr = rf.luma + 1;
> + rf.cb = rf.cr + 2;
> + rf.luma += 2;
> + rf.alpha_step = rf.luma_step;
> + break;
Can you swap these two cases? In the RGB cases above this the RGB always comes
before the BGR, so let's keep this consistent.
> default:
> return -EINVAL;
> }
> @@ -177,6 +193,8 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> flags |= FWHT_FL_CB_IS_UNCOMPRESSED;
> if (encoding & FWHT_CR_UNENCODED)
> flags |= FWHT_FL_CR_IS_UNCOMPRESSED;
> + if (encoding & FWHT_ALPHA_UNENCODED)
> + flags |= FWHT_FL_ALPHA_IS_UNCOMPRESSED;
> if (rf.height_div == 1)
> flags |= FWHT_FL_CHROMA_FULL_HEIGHT;
> if (rf.width_div == 1)
> @@ -339,6 +357,23 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
> *p++ = 0;
> }
> break;
> + case V4L2_PIX_FMT_ABGR32:
> + for (i = 0, p = p_out; i < size; i++) {
> + *p++ = state->ref_frame.cb[i];
> + *p++ = state->ref_frame.luma[i];
> + *p++ = state->ref_frame.cr[i];
> + *p++ = state->ref_frame.alpha[i];
> + }
> + break;
> +
> + case V4L2_PIX_FMT_ARGB32:
> + for (i = 0, p = p_out; i < size; i++) {
> + *p++ = state->ref_frame.alpha[i];
> + *p++ = state->ref_frame.cr[i];
> + *p++ = state->ref_frame.luma[i];
> + *p++ = state->ref_frame.cb[i];
> + }
> + break;
Ditto, please swap.
> default:
> return -EINVAL;
> }
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 945b54efd4bd..2cfd3b85fa26 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -995,7 +995,11 @@ static int vicodec_start_streaming(struct vb2_queue *q,
> unsigned int chroma_div = info->width_div * info->height_div;
> unsigned int total_planes_size;
>
> - if (info->components_num == 1)
> + //we don't know ahead how many components are in the encoding type
> + //V4L2_PIX_FMT_FWHT, so we will allocate space for 4 planes.
Use /* */ instead of //
Did you run checkpatch.pl --strict?
> + if (info->id == V4L2_PIX_FMT_FWHT || info->components_num == 4)
I like this better than what you did in the previous patch.
Can you use this condition in the previous patch as well? Except == 4 becomes == 3 there.
> + total_planes_size = 2 * size + 2 * (size / chroma_div);
> + else if (info->components_num == 1)
> total_planes_size = size;
> else
> total_planes_size = size + 2 * (size / chroma_div);
> @@ -1024,6 +1028,12 @@ static int vicodec_start_streaming(struct vb2_queue *q,
> state->ref_frame.cb = NULL;
> state->ref_frame.cr = NULL;
> }
> +
> + if (info->id == V4L2_PIX_FMT_FWHT || info->components_num == 4)
> + state->ref_frame.alpha = state->ref_frame.cr + size / chroma_div;
> + else
> + state->ref_frame.alpha = NULL;
> +
> ctx->last_src_buf = NULL;
> ctx->last_dst_buf = NULL;
> state->gop_cnt = 0;
>
Regards,
Hans
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] media: vicodec: Add support of greyscale format
2018-10-29 9:23 ` Hans Verkuil
@ 2018-10-30 15:52 ` Dafna Hirschfeld
0 siblings, 0 replies; 8+ messages in thread
From: Dafna Hirschfeld @ 2018-10-30 15:52 UTC (permalink / raw)
To: hverkuil; +Cc: mchehab, helen.koike, outreachy-kernel
[-- Attachment #1: Type: text/plain, Size: 12324 bytes --]
On Mon, Oct 29, 2018 at 11:23 AM Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Dafna,
>
> Looks good overall, but I do have some - mostly minor - comments:
>
> On 10/24/2018 08:41 PM, Dafna Hirschfeld wrote:
> > Add support for single plane greyscale format V4L2_PIX_FMT_GREY.
> > Also change the header of the encoded file to include the number
> > of components.
> >
> > Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> > ---
> > drivers/media/platform/vicodec/codec-fwht.c | 56 +++++++++++--------
> > drivers/media/platform/vicodec/codec-fwht.h | 5 +-
> > .../media/platform/vicodec/codec-v4l2-fwht.c | 12 +++-
> > drivers/media/platform/vicodec/vicodec-core.c | 21 +++++--
> > 4 files changed, 61 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/media/platform/vicodec/codec-fwht.c
> b/drivers/media/platform/vicodec/codec-fwht.c
> > index 36656031b295..91bfd1d9182b 100644
> > --- a/drivers/media/platform/vicodec/codec-fwht.c
> > +++ b/drivers/media/platform/vicodec/codec-fwht.c
> > @@ -753,9 +753,6 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> > __be16 *rlco = cf->rlc_data;
> > __be16 *rlco_max;
> > u32 encoding;
> > - u32 chroma_h = frm->height / frm->height_div;
> > - u32 chroma_w = frm->width / frm->width_div;
> > - unsigned int chroma_size = chroma_h * chroma_w;
> >
> > rlco_max = rlco + size / 2 - 256;
> > encoding = encode_plane(frm->luma, ref_frm->luma, &rlco, rlco_max,
> cf,
> > @@ -764,20 +761,27 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> > if (encoding & FWHT_FRAME_UNENCODED)
> > encoding |= FWHT_LUMA_UNENCODED;
> > encoding &= ~FWHT_FRAME_UNENCODED;
> > - rlco_max = rlco + chroma_size / 2 - 256;
> > - encoding |= encode_plane(frm->cb, ref_frm->cb, &rlco, rlco_max, cf,
> > +
> > + if (frm->components_num > 1) {
>
> Just to be on the safe side, change this to:
>
> frm->components_num >= 3
>
> since the code inside the if statements assumes that there are at least
> three components and it would fail if components_num == 2.
>
> > + u32 chroma_h = frm->height / frm->height_div;
> > + u32 chroma_w = frm->width / frm->width_div;
> > + unsigned int chroma_size = chroma_h * chroma_w;
> > +
> > + rlco_max = rlco + chroma_size / 2 - 256;
> > + encoding |= encode_plane(frm->cb, ref_frm->cb, &rlco,
> rlco_max, cf,
> > chroma_h, chroma_w,
> > frm->chroma_step, is_intra,
> next_is_intra);
> > - if (encoding & FWHT_FRAME_UNENCODED)
> > - encoding |= FWHT_CB_UNENCODED;
> > - encoding &= ~FWHT_FRAME_UNENCODED;
> > - rlco_max = rlco + chroma_size / 2 - 256;
> > - encoding |= encode_plane(frm->cr, ref_frm->cr, &rlco, rlco_max, cf,
> > + if (encoding & FWHT_FRAME_UNENCODED)
> > + encoding |= FWHT_CB_UNENCODED;
> > + encoding &= ~FWHT_FRAME_UNENCODED;
> > + rlco_max = rlco + chroma_size / 2 - 256;
> > + encoding |= encode_plane(frm->cr, ref_frm->cr, &rlco,
> rlco_max, cf,
> > chroma_h, chroma_w,
> > frm->chroma_step, is_intra,
> next_is_intra);
> > - if (encoding & FWHT_FRAME_UNENCODED)
> > - encoding |= FWHT_CR_UNENCODED;
> > - encoding &= ~FWHT_FRAME_UNENCODED;
> > + if (encoding & FWHT_FRAME_UNENCODED)
> > + encoding |= FWHT_CR_UNENCODED;
> > + encoding &= ~FWHT_FRAME_UNENCODED;
> > + }
> > cf->size = (rlco - cf->rlc_data) * sizeof(*rlco);
> > return encoding;
> > }
> > @@ -836,20 +840,24 @@ static void decode_plane(struct fwht_cframe *cf,
> const __be16 **rlco, u8 *ref,
> > }
> >
> > void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame
> *ref,
> > - u32 hdr_flags)
> > + u32 hdr_flags, unsigned int components_num)
> > {
> > const __be16 *rlco = cf->rlc_data;
> > - u32 h = cf->height / 2;
> > - u32 w = cf->width / 2;
> >
> > - if (hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT)
> > - h *= 2;
> > - if (hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH)
> > - w *= 2;
> > decode_plane(cf, &rlco, ref->luma, cf->height, cf->width,
> > hdr_flags & FWHT_FL_LUMA_IS_UNCOMPRESSED);
> > - decode_plane(cf, &rlco, ref->cb, h, w,
> > - hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
> > - decode_plane(cf, &rlco, ref->cr, h, w,
> > - hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
> > +
> > + if (components_num > 1) {
>
> Ditto: >= 3
>
> > + u32 h = cf->height;
> > + u32 w = cf->width;
> > +
> > + if (!(hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT))
> > + h /= 2;
> > + if (!(hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH))
> > + w /= 2;
> > + decode_plane(cf, &rlco, ref->cb, h, w,
> > + hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
> > + decode_plane(cf, &rlco, ref->cr, h, w,
> > + hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
> > + }
> > }
> > diff --git a/drivers/media/platform/vicodec/codec-fwht.h
> b/drivers/media/platform/vicodec/codec-fwht.h
> > index 5750e21f16ac..5e6813aeaa96 100644
> > --- a/drivers/media/platform/vicodec/codec-fwht.h
> > +++ b/drivers/media/platform/vicodec/codec-fwht.h
> > @@ -56,7 +56,7 @@
> > #define FWHT_MAGIC1 0x4f4f4f4f
> > #define FWHT_MAGIC2 0xffffffff
> >
> > -#define FWHT_VERSION 1
> > +#define FWHT_VERSION 2
> >
> > /* Set if this is an interlaced format */
> > #define FWHT_FL_IS_INTERLACED BIT(0)
> > @@ -81,6 +81,7 @@ struct fwht_cframe_hdr {
> > u32 magic2;
> > __be32 version;
> > __be32 width, height;
> > + u8 components_num;
>
> Rather than adding a new field, I prefer that it is encoded in the flags
> field.
> Take two bits (say bits 16 and 17) where you store the number of
> components - 1.
>
> This simplifies decoders since they don't have to deal with different
> structure
> sizes.
>
> > __be32 flags;
> > __be32 colorspace;
> > __be32 xfer_func;
> > @@ -122,6 +123,6 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
> > struct fwht_cframe *cf,
> > bool is_intra, bool next_is_intra);
> > void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame
> *ref,
> > - u32 hdr_flags);
> > + u32 hdr_flags, unsigned int components_num);
> >
> > #endif
> > diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> > index 64ad01e99bad..cc9275f3c6cb 100644
> > --- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> > +++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
> > @@ -11,6 +11,7 @@
> > #include "codec-v4l2-fwht.h"
> >
> > static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
> > + { V4L2_PIX_FMT_GREY, 1, 1, 1, 1, 0, 1, 1, 1},
> > { V4L2_PIX_FMT_YUV420, 1, 3, 2, 1, 1, 2, 2, 3},
> > { V4L2_PIX_FMT_YVU420, 1, 3, 2, 1, 1, 2, 2, 3},
> > { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3},
> > @@ -76,6 +77,9 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8
> *p_in, u8 *p_out)
> > rf.components_num = info->components_num;
> >
> > switch (info->id) {
> > + case V4L2_PIX_FMT_GREY:
> > + rf.cb = rf.cr = NULL;
> > + break;
> > case V4L2_PIX_FMT_YUV420:
> > rf.cb = rf.luma + size;
> > rf.cr = rf.cb + size / 4;
> > @@ -166,6 +170,7 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state,
> u8 *p_in, u8 *p_out)
> > p_hdr->version = htonl(FWHT_VERSION);
> > p_hdr->width = htonl(cf.width);
> > p_hdr->height = htonl(cf.height);
> > + p_hdr->components_num = info->components_num;
> > if (encoding & FWHT_LUMA_UNENCODED)
> > flags |= FWHT_FL_LUMA_IS_UNCOMPRESSED;
> > if (encoding & FWHT_CB_UNENCODED)
> > @@ -196,6 +201,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state,
> u8 *p_in, u8 *p_out)
> > struct fwht_cframe_hdr *p_hdr;
> > struct fwht_cframe cf;
> > u8 *p;
> > + unsigned int components_num;
>
> This should default to 3.
>
> >
> > if (!state->info)
> > return -EINVAL;
> > @@ -204,6 +210,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state,
> u8 *p_in, u8 *p_out)
> > cf.width = ntohl(p_hdr->width);
> > cf.height = ntohl(p_hdr->height);
> > flags = ntohl(p_hdr->flags);
> > + components_num = p_hdr->components_num;
>
> Only read this if the version is 2. If the version is 1, then it defaults
> to 3.
> And of course it should read the flags for the number of components as
> mentioned
> above.
>
> > state->colorspace = ntohl(p_hdr->colorspace);
> > state->xfer_func = ntohl(p_hdr->xfer_func);
> > state->ycbcr_enc = ntohl(p_hdr->ycbcr_enc);
> > @@ -225,9 +232,12 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state,
> u8 *p_in, u8 *p_out)
> > if (!(flags & FWHT_FL_CHROMA_FULL_HEIGHT))
> > chroma_size /= 2;
> >
> > - fwht_decode_frame(&cf, &state->ref_frame, flags);
> > + fwht_decode_frame(&cf, &state->ref_frame, flags, components_num);
> >
> > switch (state->info->id) {
> > + case V4L2_PIX_FMT_GREY:
> > + memcpy(p_out, state->ref_frame.luma, size);
> > + break;
> > case V4L2_PIX_FMT_YUV420:
> > case V4L2_PIX_FMT_YUV422P:
> > memcpy(p_out, state->ref_frame.luma, size);
> > diff --git a/drivers/media/platform/vicodec/vicodec-core.c
> b/drivers/media/platform/vicodec/vicodec-core.c
> > index fb6d5e9a06ab..945b54efd4bd 100644
> > --- a/drivers/media/platform/vicodec/vicodec-core.c
> > +++ b/drivers/media/platform/vicodec/vicodec-core.c
> > @@ -993,6 +993,12 @@ static int vicodec_start_streaming(struct vb2_queue
> *q,
> > unsigned int size = q_data->width * q_data->height;
> > const struct v4l2_fwht_pixfmt_info *info = q_data->info;
> > unsigned int chroma_div = info->width_div * info->height_div;
> > + unsigned int total_planes_size;
> > +
> > + if (info->components_num == 1)
>
> Use < 3
>
Hi,
But this case handles the specific case where the number of components is 1.
> > + total_planes_size = size;
> > + else
> > + total_planes_size = size + 2 * (size / chroma_div);
> >
> > q_data->sequence = 0;
> >
> > @@ -1002,10 +1008,8 @@ static int vicodec_start_streaming(struct
> vb2_queue *q,
> > state->width = q_data->width;
> > state->height = q_data->height;
> > state->ref_frame.width = state->ref_frame.height = 0;
> > - state->ref_frame.luma = kvmalloc(size + 2 * size / chroma_div,
> > - GFP_KERNEL);
> > - ctx->comp_max_size = size + 2 * size / chroma_div +
> > - sizeof(struct fwht_cframe_hdr);
> > + state->ref_frame.luma = kvmalloc(total_planes_size, GFP_KERNEL);
> > + ctx->comp_max_size = total_planes_size + sizeof(struct
> fwht_cframe_hdr);
> > state->compressed_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
> > if (!state->ref_frame.luma || !state->compressed_frame) {
> > kvfree(state->ref_frame.luma);
> > @@ -1013,8 +1017,13 @@ static int vicodec_start_streaming(struct
> vb2_queue *q,
> > vicodec_return_bufs(q, VB2_BUF_STATE_QUEUED);
> > return -ENOMEM;
> > }
> > - state->ref_frame.cb = state->ref_frame.luma + size;
> > - state->ref_frame.cr = state->ref_frame.cb + size / chroma_div;
> > + if (info->id == V4L2_PIX_FMT_FWHT || info->components_num > 1) {
>
> >= 3
>
> > + state->ref_frame.cb = state->ref_frame.luma + size;
> > + state->ref_frame.cr = state->ref_frame.cb + size /
> chroma_div;
> > + } else {
> > + state->ref_frame.cb = NULL;
> > + state->ref_frame.cr = NULL;
> > + }
> > ctx->last_src_buf = NULL;
> > ctx->last_dst_buf = NULL;
> > state->gop_cnt = 0;
> >
>
> Regards,
>
> Hans
>
[-- Attachment #2: Type: text/html, Size: 16071 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-10-30 15:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-24 18:41 [PATCH 0/3] Add support to more pixel formats in vicodec Dafna Hirschfeld
2018-10-24 18:41 ` [PATCH 1/3] media: vicodec: prepare support for various number of planes Dafna Hirschfeld
2018-10-29 9:10 ` Hans Verkuil
2018-10-24 18:41 ` [PATCH 2/3] media: vicodec: Add support of greyscale format Dafna Hirschfeld
2018-10-29 9:23 ` Hans Verkuil
2018-10-30 15:52 ` Dafna Hirschfeld
2018-10-24 18:41 ` [PATCH 3/3] media: vicodec: Add support for 4 planes formats Dafna Hirschfeld
2018-10-29 9:49 ` Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.