Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
Le lundi 27 mars 2017 à 10:45 +0200, Hans Verkuil a écrit : > > > timestamp and sequence are only set for CAPTURE, not OUTPUT. Is > > > that correct? > > > > Correct. I can add sequence for the OUTPUT queue too, but I have no > > idea how that sequence is used by userspace. > > You set V4L2_BUF_FLAG_TIMESTAMP_COPY, so you have to copy the > timestamp from the output buffer > to the capture buffer, if that makes sense for this codec. If not, > then you shouldn't use that > V4L2_BUF_FLAG and just generate new timestamps whenever a capture > buffer is ready. > > For sequence numbering just give the output queue its own sequence > counter. Btw, GStreamer and Chromium only supports TIMESTAMP_COPY, and will most likely leak frames if you craft timestamp. Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
Hi Hans, On 03/27/2017 11:45 AM, Hans Verkuil wrote: > On 25/03/17 23:30, Stanimir Varbanov wrote: >> Thanks for the comments! +static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, + u32 tag, u32 bytesused, u32 data_offset, u32 flags, + u64 timestamp_us) +{ +struct vb2_v4l2_buffer *vbuf; +struct vb2_buffer *vb; +unsigned int type; + +if (buf_type == HFI_BUFFER_INPUT) +type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; +else +type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; + +vbuf = helper_find_buf(inst, type, tag); +if (!vbuf) +return; + +vbuf->flags = flags; + +if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { +vb = &vbuf->vb2_buf; +vb->planes[0].bytesused = +max_t(unsigned int, inst->output_buf_size, bytesused); +vb->planes[0].data_offset = data_offset; +vb->timestamp = timestamp_us * NSEC_PER_USEC; +vbuf->sequence = inst->sequence++; >>> >>> timestamp and sequence are only set for CAPTURE, not OUTPUT. Is that >>> correct? >> >> Correct. I can add sequence for the OUTPUT queue too, but I have no idea how >> that sequence is used by userspace. > > You set V4L2_BUF_FLAG_TIMESTAMP_COPY, so you have to copy the timestamp from > the output buffer > to the capture buffer, if that makes sense for this codec. If not, then you > shouldn't use that The timestamp_us is filled by firmware and it is the timestamp of the output buffer which is used to produce the uncompressed capture buffer. So I think V4L2_BUF_FLAG_TIMESTAMP_COPY is correctly used here. > V4L2_BUF_FLAG and just generate new timestamps whenever a capture buffer is > ready. > > For sequence numbering just give the output queue its own sequence counter. OK will do. -- regards, Stan
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
On 27/03/17 04:18, Nicolas Dufresne wrote: > Le dimanche 26 mars 2017 à 00:30 +0200, Stanimir Varbanov a écrit : +vb->planes[0].data_offset = data_offset; +vb->timestamp = timestamp_us * NSEC_PER_USEC; +vbuf->sequence = inst->sequence++; >>> >>> timestamp and sequence are only set for CAPTURE, not OUTPUT. Is >>> that correct? >> >> Correct. I can add sequence for the OUTPUT queue too, but I have no idea >> how that sequence is used by userspace. > > Neither GStreamer or Chromium seems to use it. What does that number > means for a m2m driver ? Does it really means something ? It can be used to detect dropped frame (the sequence counter will skip in that case). Unlikely to happen for m2m devices, and most apps ignore it as well. But you still need to fill it in, it's a V4L2 requirement. Regards, Hans
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
On 25/03/17 23:30, Stanimir Varbanov wrote: > Thanks for the comments! > > On 03/24/2017 04:41 PM, Hans Verkuil wrote: >> Some comments and questions below: >> >> On 03/13/17 17:37, Stanimir Varbanov wrote: >>> This consists of video decoder implementation plus decoder >>> controls. >>> >>> Signed-off-by: Stanimir Varbanov >>> --- >>> drivers/media/platform/qcom/venus/vdec.c | 1091 >>> >>> drivers/media/platform/qcom/venus/vdec.h | 23 + >>> drivers/media/platform/qcom/venus/vdec_ctrls.c | 149 >>> 3 files changed, 1263 insertions(+) >>> create mode 100644 drivers/media/platform/qcom/venus/vdec.c >>> create mode 100644 drivers/media/platform/qcom/venus/vdec.h >>> create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c >>> >>> diff --git a/drivers/media/platform/qcom/venus/vdec.c >>> b/drivers/media/platform/qcom/venus/vdec.c >>> new file mode 100644 >>> index ..ec5203f2ba81 >>> --- /dev/null >>> +++ b/drivers/media/platform/qcom/venus/vdec.c >>> @@ -0,0 +1,1091 @@ >>> +/* >>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved. >>> + * Copyright (C) 2017 Linaro Ltd. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 and >>> + * only version 2 as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + */ >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include "hfi_venus_io.h" >>> +#include "core.h" >>> +#include "helpers.h" >>> +#include "vdec.h" >>> + >>> +static u32 get_framesize_uncompressed(unsigned int plane, u32 width, u32 >>> height) >>> +{ >>> +u32 y_stride, uv_stride, y_plane; >>> +u32 y_sclines, uv_sclines, uv_plane; >>> +u32 size; >>> + >>> +y_stride = ALIGN(width, 128); >>> +uv_stride = ALIGN(width, 128); >>> +y_sclines = ALIGN(height, 32); >>> +uv_sclines = ALIGN(((height + 1) >> 1), 16); >>> + >>> +y_plane = y_stride * y_sclines; >>> +uv_plane = uv_stride * uv_sclines + SZ_4K; >>> +size = y_plane + uv_plane + SZ_8K; >>> + >>> +return ALIGN(size, SZ_4K); >>> +} >>> + >>> +static u32 get_framesize_compressed(unsigned int width, unsigned int >>> height) >>> +{ >>> +return ((width * height * 3 / 2) / 2) + 128; >>> +} >>> + >>> +static const struct venus_format vdec_formats[] = { >>> +{ >>> +.pixfmt = V4L2_PIX_FMT_NV12, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, >> >> Just curious: is NV12 the only uncompressed format supported by the hardware? >> Or just the only one that is implemented here? >> >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_MPEG4, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_MPEG2, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_H263, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_H264, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_VP8, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_XVID, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, >> >> num_planes is always 1, do you need it at all? And if it is always one, >> why use _MPLANE at all? Is this for future additions? >> >>> +}; >>> + > > three reasons: > - _MPLAIN allows one plane only > - downstream qualcomm driver use _MPLAIN (the second plain is used for > extaradata, I ignored the extaradata support for now until v4l2 metadata api > is merged) > - I still believe that qualcomm firmware guys will add support the second or > even third plain at some point. Please add a comment in the code explaining the reason. Just before this format list would be a good place for that. Hans > >>> + >>> +static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, >>> + u32 tag, u32 bytesused, u32 data_offset, u32 flags, >>> +
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
On 25/03/17 23:30, Stanimir Varbanov wrote: > Thanks for the comments! > > On 03/24/2017 04:41 PM, Hans Verkuil wrote: >> Some comments and questions below: >> >> On 03/13/17 17:37, Stanimir Varbanov wrote: >>> This consists of video decoder implementation plus decoder >>> controls. >>> >>> Signed-off-by: Stanimir Varbanov >>> --- >>> drivers/media/platform/qcom/venus/vdec.c | 1091 >>> >>> drivers/media/platform/qcom/venus/vdec.h | 23 + >>> drivers/media/platform/qcom/venus/vdec_ctrls.c | 149 >>> 3 files changed, 1263 insertions(+) >>> create mode 100644 drivers/media/platform/qcom/venus/vdec.c >>> create mode 100644 drivers/media/platform/qcom/venus/vdec.h >>> create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c >>> >>> diff --git a/drivers/media/platform/qcom/venus/vdec.c >>> b/drivers/media/platform/qcom/venus/vdec.c >>> new file mode 100644 >>> index ..ec5203f2ba81 >>> --- /dev/null >>> +++ b/drivers/media/platform/qcom/venus/vdec.c >>> @@ -0,0 +1,1091 @@ >>> +/* >>> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved. >>> + * Copyright (C) 2017 Linaro Ltd. >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 and >>> + * only version 2 as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + */ >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include "hfi_venus_io.h" >>> +#include "core.h" >>> +#include "helpers.h" >>> +#include "vdec.h" >>> + >>> +static u32 get_framesize_uncompressed(unsigned int plane, u32 width, u32 >>> height) >>> +{ >>> +u32 y_stride, uv_stride, y_plane; >>> +u32 y_sclines, uv_sclines, uv_plane; >>> +u32 size; >>> + >>> +y_stride = ALIGN(width, 128); >>> +uv_stride = ALIGN(width, 128); >>> +y_sclines = ALIGN(height, 32); >>> +uv_sclines = ALIGN(((height + 1) >> 1), 16); >>> + >>> +y_plane = y_stride * y_sclines; >>> +uv_plane = uv_stride * uv_sclines + SZ_4K; >>> +size = y_plane + uv_plane + SZ_8K; >>> + >>> +return ALIGN(size, SZ_4K); >>> +} >>> + >>> +static u32 get_framesize_compressed(unsigned int width, unsigned int >>> height) >>> +{ >>> +return ((width * height * 3 / 2) / 2) + 128; >>> +} >>> + >>> +static const struct venus_format vdec_formats[] = { >>> +{ >>> +.pixfmt = V4L2_PIX_FMT_NV12, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, >> >> Just curious: is NV12 the only uncompressed format supported by the hardware? >> Or just the only one that is implemented here? >> >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_MPEG4, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_MPEG2, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_H263, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_H264, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_VP8, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, { >>> +.pixfmt = V4L2_PIX_FMT_XVID, >>> +.num_planes = 1, >>> +.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, >>> +}, >> >> num_planes is always 1, do you need it at all? And if it is always one, >> why use _MPLANE at all? Is this for future additions? >> >>> +}; >>> + > > three reasons: > - _MPLAIN allows one plane only > - downstream qualcomm driver use _MPLAIN (the second plain is used for > extaradata, I ignored the extaradata support for now until v4l2 metadata api > is merged) > - I still believe that qualcomm firmware guys will add support the second or > even third plain at some point. > >>> + >>> +static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, >>> + u32 tag, u32 bytesused, u32 data_offset, u32 flags, >>> + u64 timestamp_us) >>> +{ >>> +struct vb2_v4l2_buffer *vbuf; >>> +struct vb2_buffer *vb; >>> +unsigned int type; >>> +
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
Le dimanche 26 mars 2017 à 00:30 +0200, Stanimir Varbanov a écrit : > > > +vb->planes[0].data_offset = data_offset; > > > +vb->timestamp = timestamp_us * NSEC_PER_USEC; > > > +vbuf->sequence = inst->sequence++; > > > > timestamp and sequence are only set for CAPTURE, not OUTPUT. Is > > that correct? > > Correct. I can add sequence for the OUTPUT queue too, but I have no idea > how that sequence is used by userspace. Neither GStreamer or Chromium seems to use it. What does that number means for a m2m driver ? Does it really means something ? Nicolas signature.asc Description: This is a digitally signed message part
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
Hi, On 24.03.2017 20:21, Nicolas Dufresne wrote: Le vendredi 24 mars 2017 à 15:41 +0100, Hans Verkuil a écrit : +static const struct venus_format vdec_formats[] = { + { + .pixfmt = V4L2_PIX_FMT_NV12, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, Just curious: is NV12 the only uncompressed format supported by the hardware? Or just the only one that is implemented here? yes, at least to my knowledge (except below UBWC). The downstream kernel[0], from Qualcomm have: { .name = "UBWC YCbCr Semiplanar 4:2:0", .description = "UBWC Y/CbCr 4:2:0", .fourcc = V4L2_PIX_FMT_NV12_UBWC, .num_planes = 2, .get_frame_size = get_frame_size_nv12_ubwc, .type = CAPTURE_PORT, }, { .name = "UBWC YCbCr Semiplanar 4:2:0 10bit", .description = "UBWC Y/CbCr 4:2:0 10bit", .fourcc = V4L2_PIX_FMT_NV12_TP10_UBWC, .num_planes = 2, .get_frame_size = get_frame_size_nv12_ubwc_10bit, .type = CAPTURE_PORT, }, I have no idea what UBWC stands for. The performance in NV12 is more then decent from my testing. Though, there is no 10bit variant. UBWC is some kind of compressed format for NV12 [1]. This format is applicable for the newer venus hardware revisions and I planed to add it later on (when Adreno GPU driver starts handle it). regards, Stan [1] https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/include/media/msm_media_info.h#151
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
Thanks for the comments! On 03/24/2017 04:41 PM, Hans Verkuil wrote: Some comments and questions below: On 03/13/17 17:37, Stanimir Varbanov wrote: This consists of video decoder implementation plus decoder controls. Signed-off-by: Stanimir Varbanov --- drivers/media/platform/qcom/venus/vdec.c | 1091 drivers/media/platform/qcom/venus/vdec.h | 23 + drivers/media/platform/qcom/venus/vdec_ctrls.c | 149 3 files changed, 1263 insertions(+) create mode 100644 drivers/media/platform/qcom/venus/vdec.c create mode 100644 drivers/media/platform/qcom/venus/vdec.h create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c new file mode 100644 index ..ec5203f2ba81 --- /dev/null +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -0,0 +1,1091 @@ +/* + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved. + * Copyright (C) 2017 Linaro Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 and + * only version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "hfi_venus_io.h" +#include "core.h" +#include "helpers.h" +#include "vdec.h" + +static u32 get_framesize_uncompressed(unsigned int plane, u32 width, u32 height) +{ + u32 y_stride, uv_stride, y_plane; + u32 y_sclines, uv_sclines, uv_plane; + u32 size; + + y_stride = ALIGN(width, 128); + uv_stride = ALIGN(width, 128); + y_sclines = ALIGN(height, 32); + uv_sclines = ALIGN(((height + 1) >> 1), 16); + + y_plane = y_stride * y_sclines; + uv_plane = uv_stride * uv_sclines + SZ_4K; + size = y_plane + uv_plane + SZ_8K; + + return ALIGN(size, SZ_4K); +} + +static u32 get_framesize_compressed(unsigned int width, unsigned int height) +{ + return ((width * height * 3 / 2) / 2) + 128; +} + +static const struct venus_format vdec_formats[] = { + { + .pixfmt = V4L2_PIX_FMT_NV12, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, Just curious: is NV12 the only uncompressed format supported by the hardware? Or just the only one that is implemented here? + }, { + .pixfmt = V4L2_PIX_FMT_MPEG4, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, { + .pixfmt = V4L2_PIX_FMT_MPEG2, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, { + .pixfmt = V4L2_PIX_FMT_H263, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, { + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, { + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, { + .pixfmt = V4L2_PIX_FMT_H264, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, { + .pixfmt = V4L2_PIX_FMT_VP8, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, { + .pixfmt = V4L2_PIX_FMT_XVID, + .num_planes = 1, + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + }, num_planes is always 1, do you need it at all? And if it is always one, why use _MPLANE at all? Is this for future additions? +}; + three reasons: - _MPLAIN allows one plane only - downstream qualcomm driver use _MPLAIN (the second plain is used for extaradata, I ignored the extaradata support for now until v4l2 metadata api is merged) - I still believe that qualcomm firmware guys will add support the second or even third plain at some point. + +static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, + u32 tag, u32 bytesused, u32 data_offset, u32 flags, + u64 timestamp_us) +{ + struct vb2_v4l2_buffer *vbuf; + struct vb2_buffer *vb; + unsigned int type; + + if (buf_type == HFI_BUFFER_INPUT) + type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; + else + type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; + + vbuf = helper_find_buf(inst, type, tag); + if (!vbuf) + return; + + vbuf->flags = flags; +
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
Le vendredi 24 mars 2017 à 15:41 +0100, Hans Verkuil a écrit : > > +static const struct venus_format vdec_formats[] = { > > + { > > + .pixfmt = V4L2_PIX_FMT_NV12, > > + .num_planes = 1, > > + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, > > Just curious: is NV12 the only uncompressed format supported by the > hardware? > Or just the only one that is implemented here? The downstream kernel[0], from Qualcomm have: { .name = "UBWC YCbCr Semiplanar 4:2:0", .description = "UBWC Y/CbCr 4:2:0", .fourcc = V4L2_PIX_FMT_NV12_UBWC, .num_planes = 2, .get_frame_size = get_frame_size_nv12_ubwc, .type = CAPTURE_PORT, }, { .name = "UBWC YCbCr Semiplanar 4:2:0 10bit", .description = "UBWC Y/CbCr 4:2:0 10bit", .fourcc = V4L2_PIX_FMT_NV12_TP10_UBWC, .num_planes = 2, .get_frame_size = get_frame_size_nv12_ubwc_10bit, .type = CAPTURE_PORT, }, I have no idea what UBWC stands for. The performance in NV12 is more then decent from my testing. Though, there is no 10bit variant. regards, Nicolas [0] https://android.googlesource.com/kernel/msm/+/android-7.1.0_r0.2/dr ivers/media/platform/msm/vidc/msm_vdec.c#695 signature.asc Description: This is a digitally signed message part
Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files
Some comments and questions below: On 03/13/17 17:37, Stanimir Varbanov wrote: > This consists of video decoder implementation plus decoder > controls. > > Signed-off-by: Stanimir Varbanov > --- > drivers/media/platform/qcom/venus/vdec.c | 1091 > > drivers/media/platform/qcom/venus/vdec.h | 23 + > drivers/media/platform/qcom/venus/vdec_ctrls.c | 149 > 3 files changed, 1263 insertions(+) > create mode 100644 drivers/media/platform/qcom/venus/vdec.c > create mode 100644 drivers/media/platform/qcom/venus/vdec.h > create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c > > diff --git a/drivers/media/platform/qcom/venus/vdec.c > b/drivers/media/platform/qcom/venus/vdec.c > new file mode 100644 > index ..ec5203f2ba81 > --- /dev/null > +++ b/drivers/media/platform/qcom/venus/vdec.c > @@ -0,0 +1,1091 @@ > +/* > + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved. > + * Copyright (C) 2017 Linaro Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "hfi_venus_io.h" > +#include "core.h" > +#include "helpers.h" > +#include "vdec.h" > + > +static u32 get_framesize_uncompressed(unsigned int plane, u32 width, u32 > height) > +{ > + u32 y_stride, uv_stride, y_plane; > + u32 y_sclines, uv_sclines, uv_plane; > + u32 size; > + > + y_stride = ALIGN(width, 128); > + uv_stride = ALIGN(width, 128); > + y_sclines = ALIGN(height, 32); > + uv_sclines = ALIGN(((height + 1) >> 1), 16); > + > + y_plane = y_stride * y_sclines; > + uv_plane = uv_stride * uv_sclines + SZ_4K; > + size = y_plane + uv_plane + SZ_8K; > + > + return ALIGN(size, SZ_4K); > +} > + > +static u32 get_framesize_compressed(unsigned int width, unsigned int height) > +{ > + return ((width * height * 3 / 2) / 2) + 128; > +} > + > +static const struct venus_format vdec_formats[] = { > + { > + .pixfmt = V4L2_PIX_FMT_NV12, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, Just curious: is NV12 the only uncompressed format supported by the hardware? Or just the only one that is implemented here? > + }, { > + .pixfmt = V4L2_PIX_FMT_MPEG4, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, { > + .pixfmt = V4L2_PIX_FMT_MPEG2, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, { > + .pixfmt = V4L2_PIX_FMT_H263, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, { > + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, { > + .pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, { > + .pixfmt = V4L2_PIX_FMT_H264, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, { > + .pixfmt = V4L2_PIX_FMT_VP8, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, { > + .pixfmt = V4L2_PIX_FMT_XVID, > + .num_planes = 1, > + .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, > + }, num_planes is always 1, do you need it at all? And if it is always one, why use _MPLANE at all? Is this for future additions? > +}; > + > +static const struct venus_format *find_format(u32 pixfmt, u32 type) > +{ > + const struct venus_format *fmt = vdec_formats; > + unsigned int size = ARRAY_SIZE(vdec_formats); > + unsigned int i; > + > + for (i = 0; i < size; i++) { > + if (fmt[i].pixfmt == pixfmt) > + break; > + } > + > + if (i == size || fmt[i].type != type) > + return NULL; > + > + return &fmt[i]; > +} > + > +static const struct venus_format *find_format_by_index(int index, u32 type) > +{ > + const struct venus_format *fmt = vdec_formats; > + unsigned int size = ARRAY_SIZE(vdec_formats); > + int i, k = 0; > + > + if (index < 0 || index > size) > + return NULL; > + > + for (i = 0; i < size; i++) { > + if (fmt[i].type != type) > + continue; > +