Add support to more pixel formats in vicodec.
cover Add support to more pixel formats in vicodec.
The new supported formats areV4L2_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(-)
Signature · 1 line
2.17.1
PATCH 1/3 media: vicodec: prepare support for various number of planes
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 <[email protected]>
--- 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(-)
drivers/media/platform/vicodec/codec-fwht.h +3 -1
@@ -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)
drivers/media/platform/vicodec/codec-v4l2-fwht.c +25 -21
58 lines not on this page · raw message
drivers/media/platform/vicodec/codec-v4l2-fwht.h +1 -0
@@ -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 {
drivers/media/platform/vicodec/vicodec-core.c +1 -1
@@ -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)
Signature · 1 line
2.17.1
Re: [PATCH 1/3] media: vicodec: prepare support for various number of planes
Hi Dafna,
Here is a quick review:
Quoted 18 lines · Dafna Hirschfeld
Add fields to the structs
fwht_raw_frame,v4l2_fwht_pixfmtsto 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 <[email protected]>--- 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(-)drivers/media/platform/vicodec/codec-fwht.h +0
-0
drivers/media/platform/vicodec/codec-fwht.h struct fwht_raw_frame { · line 109
Quoted 5 lines
drivers/media/platform/vicodec/codec-fwht.h +1
-1@@ -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.
Quoted 87 lines
drivers/media/platform/vicodec/codec-fwht.h +2
-0+unsigned int components_num; +u8 *luma, *cb, *cr, *alpha; };#define FWHT_FRAME_PCODED BIT(0)drivers/media/platform/vicodec/codec-v4l2-fwht.c +25
-21@@ -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 :drivers/media/platform/vicodec/codec-v4l2-fwht.h +1
-0@@ -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 {drivers/media/platform/vicodec/vicodec-core.c +0
-0
drivers/media/platform/vicodec/vicodec-core.c struct pixfmt_info { · line 67
Quoted 8 lines
drivers/media/platform/vicodec/vicodec-core.c +1
-1@@ -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
PATCH 2/3 media: vicodec: Add support of greyscale format
In reply to Dafna HirschfeldAdd 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 <[email protected]>
--- 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(-)
drivers/media/platform/vicodec/codec-fwht.c +32 -24
@@ -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 ); + } }
drivers/media/platform/vicodec/codec-fwht.h +3 -2
@@ -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
drivers/media/platform/vicodec/codec-v4l2-fwht.c +11 -1
@@ -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);
drivers/media/platform/vicodec/vicodec-core.c +15 -6
@@ -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 ;
Signature · 1 line
2.17.1
Re: [PATCH 2/3] media: vicodec: Add support of greyscale format
Hi Dafna,
Looks good overall, but I do have some - mostly minor - comments:
Quoted 27 lines · Dafna Hirschfeld
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 <[email protected]>--- 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(-)drivers/media/platform/vicodec/codec-fwht.c +0
-3@@ -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,
drivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm, · line 765
Quoted 7 lines
drivers/media/platform/vicodec/codec-fwht.c +2
-2@@ -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.
Quoted 43 lines
drivers/media/platform/vicodec/codec-fwht.c +16
-13+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 ;
drivers/media/platform/vicodec/codec-fwht.c static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref, · line 850
Quoted 10 lines
drivers/media/platform/vicodec/codec-fwht.c +2
-6-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
Quoted 27 lines
drivers/media/platform/vicodec/codec-fwht.c +12
-0+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 ); + } }drivers/media/platform/vicodec/codec-fwht.h +1
-1@@ -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)
drivers/media/platform/vicodec/codec-fwht.h struct fwht_cframe_hdr { · line 84
Quoted 4 lines
drivers/media/platform/vicodec/codec-fwht.h +1
-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.
Quoted 42 lines
drivers/media/platform/vicodec/codec-fwht.h +1
-1__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 drivers/media/platform/vicodec/codec-v4l2-fwht.c +5
-0@@ -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 )
drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) · line 204
Quoted 4 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +1
-0@@ -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.
drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) · line 213
Quoted 8 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +1
-0if (!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.
Quoted 22 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +4
-1state->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);drivers/media/platform/vicodec/vicodec-core.c +0
-0
drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q, · line 998
Quoted 6 lines
drivers/media/platform/vicodec/vicodec-core.c +3
-0@@ -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
Quoted 30 lines
drivers/media/platform/vicodec/vicodec-core.c +6
-6+ 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
drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q, · line 1029
Quoted 9 lines
drivers/media/platform/vicodec/vicodec-core.c +6
-0+ 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
Re: [PATCH 2/3] media: vicodec: Add support of greyscale format
Quoted 248 lines · Hans Verkuil
Hi Dafna,
Looks good overall, but I do have some - mostly minor - comments:
Quoted 5 lines · Dafna Hirschfeld
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 <[email protected]>Review
Quoted 8 lines
--- 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(-)(quoted diff) +0
-0
b/drivers/media/platform/vicodec/codec-fwht.cdrivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm, · line 758
Quoted 13 lines
index
36656031b295..91bfd1d9182b100644drivers/media/platform/vicodec/codec-fwht.c +0
-3@@ -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,
drivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm, · line 765
Quoted 8 lines
drivers/media/platform/vicodec/codec-fwht.c +2
-2@@ -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 >= 3since the code inside the if statements assumes that there are at least three components and it would fail if
components_num== 2.drivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm,
Quoted 6 lines
drivers/media/platform/vicodec/codec-fwht.c +6
-0+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,drivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm, · line 773
Quoted 2 lines
drivers/media/platform/vicodec/codec-fwht.c +0
-0chroma_h, chroma_w, frm->chroma_step, is_intra,
next_is_intra);drivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm,
Quoted 10 lines
drivers/media/platform/vicodec/codec-fwht.c +5
-5-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,drivers/media/platform/vicodec/codec-fwht.c u32 fwht_encode_frame(struct fwht_raw_frame *frm, · line 780
Quoted 2 lines
drivers/media/platform/vicodec/codec-fwht.c +0
-0chroma_h, chroma_w, frm->chroma_step, is_intra,
next_is_intra);drivers/media/platform/vicodec/codec-fwht.c static void decode_plane(struct fwht_cframe *cf,
Quoted 11 lines
drivers/media/platform/vicodec/codec-fwht.c +4
-3-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,drivers/media/platform/vicodec/codec-fwht.c static void decode_plane(struct fwht_cframe *cf, · line 842
Quoted 3 lines
drivers/media/platform/vicodec/codec-fwht.c +0
-0} void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame*ref,
Quoted 9 lines
drivers/media/platform/vicodec/codec-fwht.c +1
-5- 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 ;drivers/media/platform/vicodec/codec-fwht.c static void decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref, · line 850
Quoted 10 lines
drivers/media/platform/vicodec/codec-fwht.c +2
-6-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
Review
Quoted 14 lines
drivers/media/platform/vicodec/codec-fwht.c +12
-0+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 ); + } }(quoted diff) +0
-0
b/drivers/media/platform/vicodec/codec-fwht.hQuoted 13 lines
index
5750e21f16ac..5e6813aeaa96100644drivers/media/platform/vicodec/codec-fwht.h +1
-1@@ -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)drivers/media/platform/vicodec/codec-fwht.h struct fwht_cframe_hdr { · line 84
Quoted 4 lines
drivers/media/platform/vicodec/codec-fwht.h +1
-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.drivers/media/platform/vicodec/codec-fwht.h u32 fwht_encode_frame(struct fwht_raw_frame *frm, · line 125
Quoted 7 lines
drivers/media/platform/vicodec/codec-fwht.h +0
-0__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,
Review
Quoted 5 lines
drivers/media/platform/vicodec/codec-fwht.h +1
-1- u32 hdr_flags); + u32 hdr_flags, unsigned int components_num);#endif (quoted diff) +0
-0
b/drivers/media/platform/vicodec/codec-v4l2-fwht.cdrivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8
Quoted 12 lines
index
64ad01e99bad..cc9275f3c6cb100644drivers/media/platform/vicodec/codec-v4l2-fwht.c +1
-0@@ -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)drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_encode(struct v4l2_fwht_state *state,
Quoted 10 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +3
-0rf.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)drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state,
Quoted 8 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +1
-0p_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)drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) · line 204
Quoted 4 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +1
-0struct fwht_cframe_hdr *p_hdr;struct fwht_cframe cf;u8 *p; +unsigned int components_num;This should default to 3.
drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state,
Quoted 4 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +0
-0if (!state->info)return -EINVAL ;@@ -204,6 +210,7 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state,u8
*p_in, u8*p_out)drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) · line 213
Quoted 4 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +1
-0cf.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.drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state,
Quoted 4 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +0
-0state->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)Review
Quoted 14 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +4
-1if (!(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);(quoted diff) +0
-0
b/drivers/media/platform/vicodec/vicodec-core.cdrivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue
Quoted 4 lines
index
fb6d5e9a06ab..945b54efd4bd100644drivers/media/platform/vicodec/vicodec-core.c +0
-0@@ -993,6 +993,12 @@ static int vicodec_start_streaming(struct vb2_queue*q,
Quoted 9 lines
drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q, · line 998
Quoted 6 lines
drivers/media/platform/vicodec/vicodec-core.c +3
-0unsigned 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.
Quoted 48 lines
drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct
Quoted 7 lines
drivers/media/platform/vicodec/vicodec-core.c +3
-0+ 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,drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q,
Quoted 9 lines
drivers/media/platform/vicodec/vicodec-core.c +2
-4state->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);drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct
Quoted 4 lines
drivers/media/platform/vicodec/vicodec-core.c +0
-0state->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,drivers/media/platform/vicodec/vicodec-core.c
Quoted 11 lines
drivers/media/platform/vicodec/vicodec-core.c +1
-2vicodec_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
drivers/media/platform/vicodec/vicodec-core.c +2
-0+ state->ref_frame.cb = state->ref_frame.luma + size; + state->ref_frame.cr = state->ref_frame.cb + size /
chroma_div;drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q, · line 1029
Quoted 8 lines
drivers/media/platform/vicodec/vicodec-core.c +4
-0+ }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
PATCH 3/3 media: vicodec: Add support for 4 planes formats
In reply to Dafna HirschfeldAdd 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 <[email protected]>
--- 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(-)
drivers/media/platform/vicodec/codec-fwht.c +17 -0
@@ -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 ); + } }
drivers/media/platform/vicodec/codec-fwht.h +4 -2
@@ -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,
drivers/media/platform/vicodec/codec-v4l2-fwht.c +35 -0
@@ -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 ; }
drivers/media/platform/vicodec/vicodec-core.c +11 -1
@@ -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 ;
Signature · 1 line
2.17.1
Re: [PATCH 3/3] media: vicodec: Add support for 4 planes formats
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.
Quoted 35 lines · Dafna Hirschfeld
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 <[email protected]>--- 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(-)drivers/media/platform/vicodec/codec-fwht.c +11
-0@@ -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; }
drivers/media/platform/vicodec/codec-fwht.c void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref, · line 879
Quoted 9 lines
drivers/media/platform/vicodec/codec-fwht.c +6
-0@@ -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.
Quoted 6 lines
drivers/media/platform/vicodec/codec-fwht.c +0
-0}drivers/media/platform/vicodec/codec-fwht.h +0
-0
drivers/media/platform/vicodec/codec-fwht.h line 78
Quoted 8 lines
drivers/media/platform/vicodec/codec-fwht.h +3
-2@@ -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.
Quoted 32 lines
drivers/media/platform/vicodec/codec-fwht.h +1
-0struct 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,drivers/media/platform/vicodec/codec-v4l2-fwht.c +6
-0@@ -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++;
drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) · line 164
Quoted 10 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +10
-0+ 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.
Quoted 23 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +9
-0default :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]; + }
drivers/media/platform/vicodec/codec-v4l2-fwht.c int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out) · line 376
Quoted 10 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +10
-0+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.
Quoted 8 lines
drivers/media/platform/vicodec/codec-v4l2-fwht.c +0
-0default :return -EINVAL ; }drivers/media/platform/vicodec/vicodec-core.c +0
-0
drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q, · line 999
Quoted 6 lines
drivers/media/platform/vicodec/vicodec-core.c +2
-1@@ -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?
drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q, · line 1000
Quoted 1 line
drivers/media/platform/vicodec/vicodec-core.c +1
-0+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.
Quoted 9 lines
drivers/media/platform/vicodec/vicodec-core.c +2
-0+ 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 ;
drivers/media/platform/vicodec/vicodec-core.c static int vicodec_start_streaming(struct vb2_queue *q, · line 1039
Quoted 10 lines
drivers/media/platform/vicodec/vicodec-core.c +6
-0} + +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