Dne torek, 07. avgust 2018 ob 14:31:03 CEST je Paul Kocialkowski napisal(a):
> Hi,
>
> On Fri, 2018-07-27 at 16:58 +0200, Jernej Škrabec wrote:
> > Dne petek, 27. julij 2018 ob 16:03:41 CEST je Jernej Škrabec napisal(a):
> > > Hi!
> > >
> > > Dne sreda, 25. julij 2018 ob 12:02:52 CEST je Paul Kocialkowski
napisal(a):
> > > > This introduces the Cedrus VPU driver that supports the VPU found in
> > > > Allwinner SoCs, also known as Video Engine. It is implemented through
> > > > a v4l2 m2m decoder device and a media device (used for media
> > > > requests).
> > > > So far, it only supports MPEG2 decoding.
> > > >
> > > > Since this VPU is stateless, synchronization with media requests is
> > > > required in order to ensure consistency between frame headers that
> > > > contain metadata about the frame to process and the raw slice data
> > > > that
> > > > is used to generate the frame.
> > > >
> > > > This driver was made possible thanks to the long-standing effort
> > > > carried out by the linux-sunxi community in the interest of reverse
> > > > engineering, documenting and implementing support for Allwinner VPU.
> > > >
> > > > Signed-off-by: Paul Kocialkowski <[email protected]>
> > > > ---
> > >
> > > <snip>
> > >
> > > > +void cedrus_dst_format_set(struct cedrus_dev *dev,
> > > > + struct v4l2_pix_format_mplane *fmt)
> > > > +{
> > > > + unsigned int width = fmt->width;
> > > > + unsigned int height = fmt->height;
> > > > + u32 chroma_size;
> > > > + u32 reg;
> > > > +
> > > > + switch (fmt->pixelformat) {
> > > > + case V4L2_PIX_FMT_NV12:
> > > > + chroma_size = ALIGN(width, 32) * ALIGN(height / 2, 32);
> > >
> > > After some testing, it turns out that right aligment for untiled format
> > > is
> > > 16.
> > >
> > > > +
> > > > + reg = VE_PRIMARY_OUT_FMT_NV12 |
> > > > + VE_SECONDARY_SPECIAL_OUT_FMT_NV12;
> > > > + cedrus_write(dev, VE_PRIMARY_OUT_FMT, reg);
> > > > +
> > > > + reg = VE_CHROMA_BUF_LEN_SDRT(chroma_size / 2) |
> > > > + VE_SECONDARY_OUT_FMT_SPECIAL;
> > > > + cedrus_write(dev, VE_CHROMA_BUF_LEN, reg);
> > > > +
> > > > + reg = chroma_size / 2;
> > > > + cedrus_write(dev, VE_PRIMARY_CHROMA_BUF_LEN, reg);
> > > > +
> > > > + reg = VE_PRIMARY_FB_LINE_STRIDE_LUMA(ALIGN(width, 32)) |
> > >
> > > ^ that one should be aligned to 16
> > >
> > > > + VE_PRIMARY_FB_LINE_STRIDE_CHROMA(ALIGN(width / 2, 16));
> >
> > It seems that CHROMA has to be aligned to 8 ^
>
> I think the issue here is that the divider should be applied after the
> alignment, not before, such as: ALIGN(width, 16) / 2, which also
> provides a 8-aligned value.
>
> Feel free to let me know if that causes any particular issue!

I think this is only semantics, it doesn't really matter if it is aligned to
16 first and then divided by 2 or divided by 2 and then aligned to 8.

BTW, doc says that chroma should be aligned to 8 and for interleaved formats
chroma line stride should be double... Maybe it should be just ALIGN(width,
8)?

>
> > That, with previosly comments, completely solves issues for one of my
> > sample video. However, there are still sample videos with issues. Those
> > are mostly rendered green with slight impressions of right image. Maybe
> > LUMA issue?
> Can you check whether these videos are interlaced? I think those don't
> do very well with our driver at this point.

Your driver works perfectly with interleaved videos. I tested that already.
It's just that video playback quality is a little lower due to missing
deinterlace step, but still watchable.

It turns out, that video in question [1] also doesn't work with ffmpeg + Intel
vaapi on PC, while it works if SW decoding is used in ffmpeg. I guess that
means that there is some bug in ffmpeg. But please be aware that ffmpeg spits
out some warnings when processing it, so video file might be considered
broken.

Best regards,
Jernej

[1] http://jernej.libreelec.tv/videos/20160219171612-Pink%20SI.mpg

>
> Cheers and thanks for the useful work and feedback!
>
> Paul
>
> > Best regards,
> > Jernej
> >
> > > > + cedrus_write(dev, VE_PRIMARY_FB_LINE_STRIDE, reg);
> > > > +
> > > > + break;
> > > > + case V4L2_PIX_FMT_MB32_NV12:
> > > > + default:
> > > > + reg = VE_PRIMARY_OUT_FMT_MB32_NV12;
> > > > + cedrus_write(dev, VE_PRIMARY_OUT_FMT, reg);
> > > > +
> > > > + reg = VE_SECONDARY_OUT_FMT_MB32_NV12;
> > > > + cedrus_write(dev, VE_CHROMA_BUF_LEN, reg);
> > > > +
> > > > + break;
> > > > + }
> > > > +}
> > >
> > > <snip>
> > >
> > > > +static void cedrus_prepare_plane_format(struct cedrus_format *fmt,
> > > > + struct v4l2_format *f,
> > > > + unsigned int i)
> > > > +{
> > > > + struct v4l2_plane_pix_format *plane_fmt =
> > > > &f->fmt.pix_mp.plane_fmt[i];
> > > > + unsigned int width = f->fmt.pix_mp.width;
> > > > + unsigned int height = f->fmt.pix_mp.height;
> > > > + unsigned int sizeimage = plane_fmt->sizeimage;
> > > > + unsigned int bytesperline = plane_fmt->bytesperline;
> > > > +
> > > > + switch (fmt->pixelformat) {
> > > > + case V4L2_PIX_FMT_MPEG2_SLICE:
> > > > + /* Zero bytes per line. */
> > > > + bytesperline = 0;
> > > > + break;
> > > > +
> > > > + case V4L2_PIX_FMT_MB32_NV12:
> > > > + /* 32-aligned stride. */
> > > > + bytesperline = ALIGN(width, 32);
> > > > +
> > > > + /* 32-aligned (luma) height. */
> > > > + height = ALIGN(height, 32);
> > > > +
> > > > + if (i == 0)
> > > > + /* 32-aligned luma size. */
> > > > + sizeimage = bytesperline * height;
> > > > + else if (i == 1)
> > > > + /* 32-aligned chroma size with 2x2 sub-sampling. */
> > > > + sizeimage = bytesperline * ALIGN(height / 2, 32);
> > > > +
> > > > + break;
> > > > +
> > > > + case V4L2_PIX_FMT_NV12:
> > > > + /* 32-aligned stride. */
> > > > + bytesperline = ALIGN(width, 32);
> > >
> > > ^ and that one should be aligned to 16 too.
> > >
> > > This partially fixes some MPEG2 videos I have tested with Kodi. I think
> > > there are other aligment issues, but I have to find them first.
> > >
> > > Best regards,
> > > Jernej
> > >
> > > > +
> > > > + if (i == 0)
> > > > + /* Regular luma size. */
> > > > + sizeimage = bytesperline * height;
> > > > + else if (i == 1)
> > > > + /* Regular chroma size with 2x2 sub-sampling. */
> > > > + sizeimage = bytesperline * height / 2;
> > > > +
> > > > + break;
> > > > + }
> > > > +
> > > > + f->fmt.pix_mp.width = width;
> > > > + f->fmt.pix_mp.height = height;
> > > > +
> > > > + plane_fmt->bytesperline = bytesperline;
> > > > + plane_fmt->sizeimage = sizeimage;
> > > > +}
> > > > +
> > > > +static void cedrus_prepare_format(struct cedrus_format *fmt,
> > > > + struct v4l2_format *f)
> > > > +{
> > > > + unsigned int i;
> > > > +
> > > > + f->fmt.pix_mp.field = V4L2_FIELD_NONE;
> > > > + f->fmt.pix_mp.num_planes = fmt->num_planes;
> > > > +
> > > > + for (i = 0; i < fmt->num_planes; i++)
> > > > + cedrus_prepare_plane_format(fmt, f, i);
> > > > +}
> > > > +
> > > > +static int cedrus_querycap(struct file *file, void *priv,
> > > > + struct v4l2_capability *cap)
> > > > +{
> > > > + strncpy(cap->driver, CEDRUS_NAME, sizeof(cap->driver) - 1);
> > > > + strncpy(cap->card, CEDRUS_NAME, sizeof(cap->card) - 1);
> > > > + snprintf(cap->bus_info, sizeof(cap->bus_info),
> > > > + "platform:%s", CEDRUS_NAME);
> > > > +
> > > > + cap->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> > > > + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_enum_fmt(struct file *file, struct v4l2_fmtdesc *f,
> > > > + u32 direction)
> > > > +{
> > > > + struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > > > + struct cedrus_dev *dev = ctx->dev;
> > > > + unsigned int capabilities = dev->capabilities;
> > > > + struct cedrus_format *fmt;
> > > > + unsigned int i, index;
> > > > +
> > > > + /* Index among formats that match the requested direction. */
> > > > + index = 0;
> > > > +
> > > > + for (i = 0; i < CEDRUS_FORMATS_COUNT; i++) {
> > > > + fmt = &cedrus_formats[i];
> > > > +
> > > > + if (fmt->capabilities && (fmt->capabilities & capabilities) !=
> > > > + fmt->capabilities)
> > > > + continue;
> > > > +
> > > > + if (!(cedrus_formats[i].directions & direction))
> > > > + continue;
> > > > +
> > > > + if (index == f->index)
> > > > + break;
> > > > +
> > > > + index++;
> > > > + }
> > > > +
> > > > + /* Matched format. */
> > > > + if (i < CEDRUS_FORMATS_COUNT) {
> > > > + f->pixelformat = cedrus_formats[i].pixelformat;
> > > > +
> > > > + return 0;
> > > > + }
> > > > +
> > > > + return -EINVAL;
> > > > +}
> > > > +
> > > > +static int cedrus_enum_fmt_vid_cap(struct file *file, void *priv,
> > > > + struct v4l2_fmtdesc *f)
> > > > +{
> > > > + return cedrus_enum_fmt(file, f, CEDRUS_DECODE_DST);
> > > > +}
> > > > +
> > > > +static int cedrus_enum_fmt_vid_out(struct file *file, void *priv,
> > > > + struct v4l2_fmtdesc *f)
> > > > +{
> > > > + return cedrus_enum_fmt(file, f, CEDRUS_DECODE_SRC);
> > > > +}
> > > > +
> > > > +static int cedrus_g_fmt_vid_cap(struct file *file, void *priv,
> > > > + struct v4l2_format *f)
> > > > +{
> > > > + struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > > > +
> > > > + if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > > > + return -EINVAL;
> > > > +
> > > > + f->fmt.pix_mp = ctx->dst_fmt;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_g_fmt_vid_out(struct file *file, void *priv,
> > > > + struct v4l2_format *f)
> > > > +{
> > > > + struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > > > +
> > > > + if (f->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > > + return -EINVAL;
> > > > +
> > > > + f->fmt.pix_mp = ctx->src_fmt;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_try_fmt_vid_cap(struct file *file, void *priv,
> > > > + struct v4l2_format *f)
> > > > +{
> > > > + struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > > > + struct cedrus_dev *dev = ctx->dev;
> > > > + struct cedrus_format *fmt;
> > > > +
> > > > + fmt = cedrus_find_format(f->fmt.pix_mp.pixelformat,
> > > > CEDRUS_DECODE_DST,
> > > > + dev->capabilities);
> > > > + if (!fmt)
> > > > + return -EINVAL;
> > > > +
> > > > + cedrus_prepare_format(fmt, f);
> > > > +
> > > > + /* Limit to hardware min/max. */
> > > > + f->fmt.pix_mp.width = clamp(f->fmt.pix_mp.width, CEDRUS_MIN_WIDTH,
> > > > + CEDRUS_MAX_WIDTH);
> > > > + f->fmt.pix_mp.height = clamp(f->fmt.pix_mp.height,
> > > > CEDRUS_MIN_HEIGHT,
> > > > + CEDRUS_MAX_HEIGHT);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_try_fmt_vid_out(struct file *file, void *priv,
> > > > + struct v4l2_format *f)
> > > > +{
> > > > + struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > > > + struct cedrus_dev *dev = ctx->dev;
> > > > + struct cedrus_format *fmt;
> > > > + struct v4l2_plane_pix_format *plane_fmt;
> > > > + unsigned int i;
> > > > +
> > > > + fmt = cedrus_find_format(f->fmt.pix_mp.pixelformat,
> > > > CEDRUS_DECODE_SRC,
> > > > + dev->capabilities);
> > > > + if (!fmt)
> > > > + return -EINVAL;
> > > > +
> > > > + cedrus_prepare_format(fmt, f);
> > > > +
> > > > + for (i = 0; i < f->fmt.pix_mp.num_planes; i++) {
> > > > + plane_fmt = &f->fmt.pix_mp.plane_fmt[i];
> > > > +
> > > > + /* Source image size has to be given by userspace. */
> > > > + if (plane_fmt->sizeimage == 0)
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_s_fmt_vid_cap(struct file *file, void *priv,
> > > > + struct v4l2_format *f)
> > > > +{
> > > > + struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > > > + struct cedrus_dev *dev = ctx->dev;
> > > > + int ret;
> > > > +
> > > > + ret = cedrus_try_fmt_vid_cap(file, priv, f);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + ctx->dst_fmt = f->fmt.pix_mp;
> > > > +
> > > > + cedrus_dst_format_set(dev, &ctx->dst_fmt);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
> > > > + struct v4l2_format *f)
> > > > +{
> > > > + struct cedrus_ctx *ctx = cedrus_file2ctx(file);
> > > > + int ret;
> > > > +
> > > > + ret = cedrus_try_fmt_vid_out(file, priv, f);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + ctx->src_fmt = f->fmt.pix_mp;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +const struct v4l2_ioctl_ops cedrus_ioctl_ops = {
> > > > + .vidioc_querycap = cedrus_querycap,
> > > > +
> > > > + .vidioc_enum_fmt_vid_cap_mplane = cedrus_enum_fmt_vid_cap,
> > > > + .vidioc_g_fmt_vid_cap_mplane = cedrus_g_fmt_vid_cap,
> > > > + .vidioc_try_fmt_vid_cap_mplane = cedrus_try_fmt_vid_cap,
> > > > + .vidioc_s_fmt_vid_cap_mplane = cedrus_s_fmt_vid_cap,
> > > > +
> > > > + .vidioc_enum_fmt_vid_out_mplane = cedrus_enum_fmt_vid_out,
> > > > + .vidioc_g_fmt_vid_out_mplane = cedrus_g_fmt_vid_out,
> > > > + .vidioc_try_fmt_vid_out_mplane = cedrus_try_fmt_vid_out,
> > > > + .vidioc_s_fmt_vid_out_mplane = cedrus_s_fmt_vid_out,
> > > > +
> > > > + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
> > > > + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
> > > > + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
> > > > + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
> > > > + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
> > > > + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
> > > > + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
> > > > +
> > > > + .vidioc_streamon = v4l2_m2m_ioctl_streamon,
> > > > + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
> > > > +
> > > > + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> > > > + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> > > > +};
> > > > +
> > > > +static int cedrus_queue_setup(struct vb2_queue *vq, unsigned int
> > > > *nbufs,
> > > > + unsigned int *nplanes, unsigned int sizes[],
> > > > + struct device *alloc_devs[])
> > > > +{
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(vq);
> > > > + struct cedrus_dev *dev = ctx->dev;
> > > > + struct v4l2_pix_format_mplane *mplane_fmt;
> > > > + struct cedrus_format *fmt;
> > > > + unsigned int i;
> > > > +
> > > > + switch (vq->type) {
> > > > + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > > > + mplane_fmt = &ctx->src_fmt;
> > > > + fmt = cedrus_find_format(mplane_fmt->pixelformat,
> > > > + CEDRUS_DECODE_SRC,
> > > > + dev->capabilities);
> > > > + break;
> > > > +
> > > > + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
> > > > + mplane_fmt = &ctx->dst_fmt;
> > > > + fmt = cedrus_find_format(mplane_fmt->pixelformat,
> > > > + CEDRUS_DECODE_DST,
> > > > + dev->capabilities);
> > > > + break;
> > > > +
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (!fmt)
> > > > + return -EINVAL;
> > > > +
> > > > + if (fmt->num_buffers == 1) {
> > > > + sizes[0] = 0;
> > > > +
> > > > + for (i = 0; i < fmt->num_planes; i++)
> > > > + sizes[0] += mplane_fmt->plane_fmt[i].sizeimage;
> > > > + } else if (fmt->num_buffers == fmt->num_planes) {
> > > > + for (i = 0; i < fmt->num_planes; i++)
> > > > + sizes[i] = mplane_fmt->plane_fmt[i].sizeimage;
> > > > + } else {
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + *nplanes = fmt->num_buffers;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_buf_init(struct vb2_buffer *vb)
> > > > +{
> > > > + struct vb2_queue *vq = vb->vb2_queue;
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(vq);
> > > > +
> > > > + if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > > > + ctx->dst_bufs[vb->index] = vb;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static void cedrus_buf_cleanup(struct vb2_buffer *vb)
> > > > +{
> > > > + struct vb2_queue *vq = vb->vb2_queue;
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(vq);
> > > > +
> > > > + if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > > > + ctx->dst_bufs[vb->index] = NULL;
> > > > +}
> > > > +
> > > > +static int cedrus_buf_prepare(struct vb2_buffer *vb)
> > > > +{
> > > > + struct vb2_queue *vq = vb->vb2_queue;
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(vq);
> > > > + struct v4l2_pix_format_mplane *fmt;
> > > > + unsigned int buffer_size = 0;
> > > > + unsigned int format_size = 0;
> > > > + unsigned int i;
> > > > +
> > > > + if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > > + fmt = &ctx->src_fmt;
> > > > + else if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > > > + fmt = &ctx->dst_fmt;
> > > > + else
> > > > + return -EINVAL;
> > > > +
> > > > + for (i = 0; i < vb->num_planes; i++)
> > > > + buffer_size += vb2_plane_size(vb, i);
> > > > +
> > > > + for (i = 0; i < fmt->num_planes; i++)
> > > > + format_size += fmt->plane_fmt[i].sizeimage;
> > > > +
> > > > + if (buffer_size < format_size)
> > > > + return -EINVAL;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int cedrus_start_streaming(struct vb2_queue *q, unsigned int
> > > > count)
> > > > +{
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(q);
> > > > + struct cedrus_dev *dev = ctx->dev;
> > > > + int ret = 0;
> > > > +
> > > > + switch (ctx->src_fmt.pixelformat) {
> > > > + case V4L2_PIX_FMT_MPEG2_SLICE:
> > > > + ctx->current_codec = CEDRUS_CODEC_MPEG2;
> > > > + break;
> > > > + default:
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + if (V4L2_TYPE_IS_OUTPUT(q->type) &&
> > > > + dev->dec_ops[ctx->current_codec]->start)
> > > > + ret = dev->dec_ops[ctx->current_codec]->start(ctx);
> > > > +
> > > > + return ret;
> > > > +}
> > > > +
> > > > +static void cedrus_stop_streaming(struct vb2_queue *q)
> > > > +{
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(q);
> > > > + struct cedrus_dev *dev = ctx->dev;
> > > > + struct vb2_v4l2_buffer *vbuf;
> > > > + unsigned long flags;
> > > > +
> > > > + flush_scheduled_work();
> > > > +
> > > > + if (V4L2_TYPE_IS_OUTPUT(q->type) &&
> > > > + dev->dec_ops[ctx->current_codec]->stop)
> > > > + dev->dec_ops[ctx->current_codec]->stop(ctx);
> > > > +
> > > > + for (;;) {
> > > > + spin_lock_irqsave(&ctx->dev->irq_lock, flags);
> > > > +
> > > > + if (V4L2_TYPE_IS_OUTPUT(q->type))
> > > > + vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> > > > + else
> > > > + vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > > > +
> > > > + spin_unlock_irqrestore(&ctx->dev->irq_lock, flags);
> > > > +
> > > > + if (!vbuf)
> > > > + return;
> > > > +
> > > > + v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req,
> > > > + &ctx->hdl);
> > > > + v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
> > > > + }
> > > > +}
> > > > +
> > > > +static void cedrus_buf_queue(struct vb2_buffer *vb)
> > > > +{
> > > > + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > > > +
> > > > + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
> > > > +}
> > > > +
> > > > +static void cedrus_buf_request_complete(struct vb2_buffer *vb)
> > > > +{
> > > > + struct cedrus_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > > > +
> > > > + v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
> > > > +}
> > > > +
> > > > +static struct vb2_ops cedrus_qops = {
> > > > + .queue_setup = cedrus_queue_setup,
> > > > + .buf_prepare = cedrus_buf_prepare,
> > > > + .buf_init = cedrus_buf_init,
> > > > + .buf_cleanup = cedrus_buf_cleanup,
> > > > + .buf_queue = cedrus_buf_queue,
> > > > + .buf_request_complete = cedrus_buf_request_complete,
> > > > + .start_streaming = cedrus_start_streaming,
> > > > + .stop_streaming = cedrus_stop_streaming,
> > > > + .wait_prepare = vb2_ops_wait_prepare,
> > > > + .wait_finish = vb2_ops_wait_finish,
> > > > +};
> > > > +
> > > > +int cedrus_queue_init(void *priv, struct vb2_queue *src_vq,
> > > > + struct vb2_queue *dst_vq)
> > > > +{
> > > > + struct cedrus_ctx *ctx = priv;
> > > > + int ret;
> > > > +
> > > > + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > > > + src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > > > + src_vq->drv_priv = ctx;
> > > > + src_vq->buf_struct_size = sizeof(struct cedrus_buffer);
> > > > + src_vq->allow_zero_bytesused = 1;
> > > > + src_vq->min_buffers_needed = 1;
> > > > + src_vq->ops = &cedrus_qops;
> > > > + src_vq->mem_ops = &vb2_dma_contig_memops;
> > > > + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > > > + src_vq->lock = &ctx->dev->dev_mutex;
> > > > + src_vq->dev = ctx->dev->dev;
> > > > +
> > > > + ret = vb2_queue_init(src_vq);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > > > + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > > > + dst_vq->drv_priv = ctx;
> > > > + dst_vq->buf_struct_size = sizeof(struct cedrus_buffer);
> > > > + dst_vq->allow_zero_bytesused = 1;
> > > > + dst_vq->min_buffers_needed = 1;
> > > > + dst_vq->ops = &cedrus_qops;
> > > > + dst_vq->mem_ops = &vb2_dma_contig_memops;
> > > > + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > > > + dst_vq->lock = &ctx->dev->dev_mutex;
> > > > + dst_vq->dev = ctx->dev->dev;
> > > > +
> > > > + return vb2_queue_init(dst_vq);
> > > > +}
> > > > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.h
> > > > b/drivers/staging/media/sunxi/cedrus/cedrus_video.h new file mode
> > > > 100644
> > > > index 000000000000..56afcc8c02ba
> > > > --- /dev/null
> > > > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.h
> > > > @@ -0,0 +1,31 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * Sunxi-Cedrus VPU driver
> > > > + *
> > > > + * Copyright (C) 2018 Paul Kocialkowski
> > > > <[email protected]>
> > > > + * Copyright (C) 2016 Florent Revest
> > > > <[email protected]>
> > > > + *
> > > > + * Based on the vim2m driver, that is:
> > > > + *
> > > > + * Copyright (c) 2009-2010 Samsung Electronics Co., Ltd.
> > > > + * Pawel Osciak, <[email protected]>
> > > > + * Marek Szyprowski, <[email protected]>
> > > > + */
> > > > +
> > > > +#ifndef _CEDRUS_VIDEO_H_
> > > > +#define _CEDRUS_VIDEO_H_
> > > > +
> > > > +struct cedrus_format {
> > > > + u32 pixelformat;
> > > > + u32 directions;
> > > > + unsigned int num_planes;
> > > > + unsigned int num_buffers;
> > > > + unsigned int capabilities;
> > > > +};
> > > > +
> > > > +extern const struct v4l2_ioctl_ops cedrus_ioctl_ops;
> > > > +
> > > > +int cedrus_queue_init(void *priv, struct vb2_queue *src_vq,
> > > > + struct vb2_queue *dst_vq);
> > > > +
> > > > +#endif

2018-08-07 15:26:47

by Tomasz Figa

[permalink] [raw]

Subject: Re: [linux-sunxi] [PATCH v6 4/8] media: platform: Add Cedrus VPU decoder driver

On Wed, Aug 8, 2018 at 12:05 AM Jernej Škrabec <[email protected]> wrote:
>
> Dne torek, 07. avgust 2018 ob 14:31:03 CEST je Paul Kocialkowski napisal(a):
> > Hi,
> >
> > On Fri, 2018-07-27 at 16:58 +0200, Jernej Škrabec wrote:
> > > Dne petek, 27. julij 2018 ob 16:03:41 CEST je Jernej Škrabec napisal(a):
> > > > Hi!
> > > >
> > > > Dne sreda, 25. julij 2018 ob 12:02:52 CEST je Paul Kocialkowski
> napisal(a):
> > > > > This introduces the Cedrus VPU driver that supports the VPU found in
> > > > > Allwinner SoCs, also known as Video Engine. It is implemented through
> > > > > a v4l2 m2m decoder device and a media device (used for media
> > > > > requests).
> > > > > So far, it only supports MPEG2 decoding.
> > > > >
> > > > > Since this VPU is stateless, synchronization with media requests is
> > > > > required in order to ensure consistency between frame headers that
> > > > > contain metadata about the frame to process and the raw slice data
> > > > > that
> > > > > is used to generate the frame.
> > > > >
> > > > > This driver was made possible thanks to the long-standing effort
> > > > > carried out by the linux-sunxi community in the interest of reverse
> > > > > engineering, documenting and implementing support for Allwinner VPU.
> > > > >
> > > > > Signed-off-by: Paul Kocialkowski <[email protected]>
> > > > > ---
> > > >
> > > > <snip>
> > > >
> > > > > +void cedrus_dst_format_set(struct cedrus_dev *dev,
> > > > > + struct v4l2_pix_format_mplane *fmt)
> > > > > +{
> > > > > + unsigned int width = fmt->width;
> > > > > + unsigned int height = fmt->height;
> > > > > + u32 chroma_size;
> > > > > + u32 reg;
> > > > > +
> > > > > + switch (fmt->pixelformat) {
> > > > > + case V4L2_PIX_FMT_NV12:
> > > > > + chroma_size = ALIGN(width, 32) * ALIGN(height / 2, 32);
> > > >
> > > > After some testing, it turns out that right aligment for untiled format
> > > > is
> > > > 16.
> > > >
> > > > > +
> > > > > + reg = VE_PRIMARY_OUT_FMT_NV12 |
> > > > > + VE_SECONDARY_SPECIAL_OUT_FMT_NV12;
> > > > > + cedrus_write(dev, VE_PRIMARY_OUT_FMT, reg);
> > > > > +
> > > > > + reg = VE_CHROMA_BUF_LEN_SDRT(chroma_size / 2) |
> > > > > + VE_SECONDARY_OUT_FMT_SPECIAL;
> > > > > + cedrus_write(dev, VE_CHROMA_BUF_LEN, reg);
> > > > > +
> > > > > + reg = chroma_size / 2;
> > > > > + cedrus_write(dev, VE_PRIMARY_CHROMA_BUF_LEN, reg);
> > > > > +
> > > > > + reg = VE_PRIMARY_FB_LINE_STRIDE_LUMA(ALIGN(width, 32)) |
> > > >
> > > > ^ that one should be aligned to 16
> > > >
> > > > > + VE_PRIMARY_FB_LINE_STRIDE_CHROMA(ALIGN(width / 2, 16));
> > >
> > > It seems that CHROMA has to be aligned to 8 ^
> >
> > I think the issue here is that the divider should be applied after the
> > alignment, not before, such as: ALIGN(width, 16) / 2, which also
> > provides a 8-aligned value.
> >
> > Feel free to let me know if that causes any particular issue!
>
> I think this is only semantics, it doesn't really matter if it is aligned to
> 16 first and then divided by 2 or divided by 2 and then aligned to 8.

It depends if |width| is always expected to be aligned to 2. For
example, given |width| = 17,

ALIGN(17, 16) = 32, 32 / 2 = 16
17 / 2 = 8, ALIGN(8, 8) = 8

Best regards,
Tomasz

2018-08-07 16:53:52

Hi,

On Mon, 2018-08-06 at 16:21 +0200, Paul Kocialkowski wrote:
> Hi,
>
> On Fri, 2018-08-03 at 17:49 -0300, Ezequiel Garcia wrote:
> > On Wed, 2018-07-25 at 12:02 +0200, Paul Kocialkowski wrote:
> > > This introduces the Cedrus VPU driver that supports the VPU found in
> > > Allwinner SoCs, also known as Video Engine. It is implemented through
> > > a v4l2 m2m decoder device and a media device (used for media requests).
> > > So far, it only supports MPEG2 decoding.
> > >
> > > Since this VPU is stateless, synchronization with media requests is
> > > required in order to ensure consistency between frame headers that
> > > contain metadata about the frame to process and the raw slice data that
> > > is used to generate the frame.
> > >
> > > This driver was made possible thanks to the long-standing effort
> > > carried out by the linux-sunxi community in the interest of reverse
> > > engineering, documenting and implementing support for Allwinner VPU.
> > >
> > > Signed-off-by: Paul Kocialkowski <[email protected]>
> >
> > [..]
> > > +static int cedrus_probe(struct platform_device *pdev)
> > > +{
> > > + struct cedrus_dev *dev;
> > > + struct video_device *vfd;
> > > + int ret;
> > > +
> > > + dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
> > > + if (!dev)
> > > + return -ENOMEM;
> > > +
> > > + dev->dev = &pdev->dev;
> > > + dev->pdev = pdev;
> > > +
> > > + ret = cedrus_hw_probe(dev);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "Failed to probe hardware\n");
> > > + return ret;
> > > + }
> > > +
> > > + dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
> > > +
> > > + mutex_init(&dev->dev_mutex);
> > > + spin_lock_init(&dev->irq_lock);
> > > +
> >
> > A minor thing.
> >
> > I believe this spinlock is not needed. All the data structures
> > it's accessing are already protected, and some operations
> > (stop_streaming) are guaranteed to not run at the same
> > time as a job.
>
> I think we were afraid of this kind of scenario happening, but
> everything seems to indicate that these data structures are already
> properly protected by the core, as you're suggesting.
>
> Removing the lock does not cause any noticeable issue at first try, but
> I'd like to test decoding for a few hours in a row to reduce the
> probability of missing a corner case that our lock was preventing.

After testing for several hours in a row, I got some cases of CPU stall
which did not happen with the driver lock. So it seems safer to keep the
lock around for now and maybe revisit this later, when there is time to
investigate why it is needed.

Cheers,

Paul

> If that goes well, I guess we can remove it from our driver.
>
> Cheers,
>
> Paul
>
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Attachments:

signature.asc (499.00 B)
This is a digitally signed message part

2018-08-08 11:58:41

by Paul Kocialkowski

[permalink] [raw]

Subject: Re: [PATCH v6 1/8] media: v4l: Add definitions for MPEG2 slice format and metadata

Hi Hans,

On Sat, 2018-08-04 at 13:35 +0200, Hans Verkuil wrote:
> Hi Paul,
>
> Some comments below. It looks pretty good, it's mostly small things that I
> commented upon.

And thanks for the review!

> On 07/25/2018 12:02 PM, Paul Kocialkowski wrote:
> > Stateless video decoding engines require both the MPEG slices and
> > associated metadata from the video stream in order to decode frames.
> >
> > This introduces definitions for a new pixel format, describing buffers
> > with MPEG2 slice data, as well as a control structure for passing the
> > frame metadata to drivers.
> >
> > This is based on work from both Florent Revest and Hugues Fruchet.
> >
> > Signed-off-by: Paul Kocialkowski <[email protected]>
> > ---
> > .../media/uapi/v4l/extended-controls.rst | 122 ++++++++++++++++++
> > .../media/uapi/v4l/pixfmt-compressed.rst | 5 +
> > drivers/media/v4l2-core/v4l2-ctrls.c | 54 ++++++++
> > drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
> > include/media/v4l2-ctrls.h | 18 ++-
> > include/uapi/linux/v4l2-controls.h | 43 ++++++
> > include/uapi/linux/videodev2.h | 5 +
> > 7 files changed, 241 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/media/uapi/v4l/extended-controls.rst b/Documentation/media/uapi/v4l/extended-controls.rst
> > index 9f7312bf3365..4a29d89fd9ac 100644
> > --- a/Documentation/media/uapi/v4l/extended-controls.rst
> > +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> > @@ -1497,6 +1497,128 @@ enum v4l2_mpeg_video_h264_hierarchical_coding_type -
> >
> >
> >
> > +.. _v4l2-mpeg-mpeg2:
> > +
> > +``V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (struct)``
> > + Specifies the slice parameters (also known as slice header) for the
> > + associated MPEG-2 slice data. This includes all the necessary
> > + parameters for configuring a hardware decoder pipeline for MPEG-2.
>
> This seems to be mostly a representation of the MPEG-2 "Picture coding
> extension" (6.2.3.1 in ISO/IEC 13818-2: 1995).
> ISO/IEC 13818-2
> Is that correct? I think some references to the standard should be added
> were appropriate.

The structure does contain fields from the picture coding extension, but
also takes some bits from other parts of the bitstream. I have added
references to the specs and split out the different parts of the
structure for the next revision. I also took the occasion to add/rename
some fields to stick closer to the bitstream fields.

> > +
> > +.. tabularcolumns:: |p{2.0cm}|p{4.0cm}|p{11.0cm}|
> > +
> > +.. c:type:: v4l2_ctrl_mpeg2_slice_params
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table:: struct v4l2_ctrl_mpeg2_slice_params
> > + :header-rows: 0
> > + :stub-columns: 0
> > + :widths: 1 1 2
> > +
> > + * - __u32
> > + - ``slice_len``
> > + - Length (in bits) of the current slice data.
> > + * - __u32
> > + - ``slice_pos``
> > + - Position (in bits) of the current slice data, relative to the
> > + frame start.
> > + * - __u16
> > + - ``width``
> > + - Width of the corresponding output frame for the current slice.
> > + * - __u16
> > + - ``height``
> > + - Height of the corresponding output frame for the current slice.
> > + * - __u8
> > + - ``slice_type``
> > + - Picture coding type for the frame covered by the current slice
> > + (V4L2_MPEG2_SLICE_TYPE_I, V4L2_MPEG2_SLICE_TYPE_P or
> > + V4L2_MPEG2_SLICE_PCT_B).
> > + * - __u8
> > + - ``f_code[2][2]``
> > + - Motion vector codes.
> > + * - __u8
> > + - ``intra_dc_precision``
> > + - Precision of Discrete Cosine transform (0: 8 bits precision,
> > + 1: 9 bits precision, 2: 10 bits precision, 11: 11 bits precision).
> > + * - __u8
> > + - ``picture_structure``
> > + - Picture structure (1: interlaced top field,
> > + 2: interlaced bottom field, 3: progressive frame).
> > + * - __u8
> > + - ``top_field_first``
> > + - If set to 1 and interlaced stream, top field is output first.
> > + * - __u8
> > + - ``frame_pred_frame_dct``
> > + - If set to 1, only frame-DCT and frame prediction are used.
> > + * - __u8
> > + - ``concealment_motion_vectors``
> > + - If set to 1, motion vectors are coded for intra macroblocks.
> > + * - __u8
> > + - ``q_scale_type``
> > + - This flag affects the inverse quantisation process.
>
> quantization
>
> The american spelling appears to be the standard in our documentation, so
> let's stick to that.

I see, that makes sense.

> > + * - __u8
> > + - ``intra_vlc_format``
> > + - This flag affects the decoding of transform coefficient data.
> > + * - __u8
> > + - ``alternate_scan``
> > + - This flag affects the decoding of transform coefficient data.
> > + * - __u8
> > + - ``backward_ref_index``
> > + - Index for the V4L2 buffer to use as backward reference, used with
> > + B-coded and P-coded frames.
> > + * - __u8
> > + - ``forward_ref_index``
> > + - Index for the V4L2 buffer to use as forward reference, used with
> > + P-coded frames.
> > + * - :cspan:`2`
> > +
> > +``V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION (struct)``
> > + Specifies quantization matrices for the associated MPEG-2 slice data.
> > +
> > +.. tabularcolumns:: |p{2.0cm}|p{4.0cm}|p{11.0cm}|
> > +
> > +.. c:type:: v4l2_ctrl_mpeg2_quantization
> > +
> > +.. cssclass:: longtable
> > +
> > +.. flat-table:: struct v4l2_ctrl_mpeg2_quantization
> > + :header-rows: 0
> > + :stub-columns: 0
> > + :widths: 1 1 2
> > +
> > + * - __u8
> > + - ``load_intra_quantiser_matrix``
> > + - One bit to indicate whether to load the intra quantiser matrix.
>
> So the MPEG-2 spec appears to follow the British spelling, so for consistency
> with that spec we should stick to that for the field name.
>
> I'm not sure what is better in the description: stick to quantiser or change
> it to the US quantizer. I think we should keep quantiser since it looks weird
> otherwise.

Or I could just reformulate it to mention the ``intra_quantiser_matrix``
field directly, instead of breaking it down into words.

> > + * - __u32
> > + - ``load_non_intra_quantiser_matrix``
> > + - One bit to indicate whether to load the non-intra quantiser matrix.
> > + * - __u32
> > + - ``load_chroma_intra_quantiser_matrix``
> > + - One bit to indicate whether to load the chroma intra quantiser matrix,
> > + only relevant for non-4:2:0 YUV formats.
> > + * - __u32
> > + - ``load_chroma_non_intra_quantiser_matrix``
> > + - One bit to indicate whether to load the non-chroma intra quantiser
> > + matrix, only relevant for non-4:2:0 YUV formats.
> > + * - __u32
> > + - ``intra_quantiser_matrix[64]``
> > + - The intra quantiser matrix coefficients, in zigzag scanning order.
> > + It is relevant for both luma and chroma components, although it can be
> > + superseded by the chroma-specific matrix for non-4:2:0 YUV formats.
> > + * - __u32
> > + - ``non_intra_quantiser_matrix[64]``
> > + - The non-intra quantiser matrix coefficients, in zigzag scanning order.
> > + It is relevant for both luma and chroma components, although it can be
> > + superseded by the chroma-specific matrix for non-4:2:0 YUV formats.
> > + * - __u32
> > + - ``chroma_intra_quantiser_matrix[64]``
> > + - The intra quantiser matrix coefficients for the chroma YUV component,
> > + in zigzag scanning order. Only relevant for non-4:2:0 YUV formats.
> > + * - __u32
> > + - ``chroma_non_intra_quantiser_matrix[64]``
> > + - The non-intra quantiser matrix coefficients for the chroma YUV component,
> > + in zigzag scanning order. Only relevant for non-4:2:0 YUV formats.
>
> According to the MPEG-2 spec (6.3.11) these are all unsigned 8 bit values, so why
> use __u32?

Woops, that was purely a mistake. It's all __u8 indeed.

> > + * - :cspan:`2`
> >
> > MFC 5.1 MPEG Controls
> > ---------------------
> > diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > index abec03937bb3..4e73f62b5163 100644
> > --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > @@ -60,6 +60,11 @@ Compressed Formats
> > - ``V4L2_PIX_FMT_MPEG2``
> > - 'MPG2'
> > - MPEG2 video elementary stream.
> > + * .. _V4L2-PIX-FMT-MPEG2-SLICE:
> > +
> > + - ``V4L2_PIX_FMT_MPEG2_SLICE``
> > + - 'MG2S'
> > + - MPEG2 parsed slice data, as extracted from the MPEG2 bitstream.
> > * .. _V4L2-PIX-FMT-MPEG4:
> >
> > - ``V4L2_PIX_FMT_MPEG4``
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 3610dce3a4f8..22483d894259 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -844,6 +844,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return "Vertical MV Search Range";
> > case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER: return "Repeat Sequence Header";
> > case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME: return "Force Key Frame";
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS: return "MPEG2 Slice Header";
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION: return "MPEG2 Quantization Matrices";
> >
> > /* VPX controls */
> > case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS: return "VPX Number of Partitions";
> > @@ -1292,6 +1294,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_RDS_TX_ALT_FREQS:
> > *type = V4L2_CTRL_TYPE_U32;
> > break;
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS:
> > + *type = V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS;
> > + break;
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION:
> > + *type = V4L2_CTRL_TYPE_MPEG2_QUANTIZATION;
> > + break;
> > default:
> > *type = V4L2_CTRL_TYPE_INTEGER;
> > break;
> > @@ -1550,6 +1558,7 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> > static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
> > union v4l2_ctrl_ptr ptr)
> > {
> > + struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> > size_t len;
> > u64 offset;
> > s64 val;
> > @@ -1612,6 +1621,45 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
> > return -ERANGE;
> > return 0;
> >
> > + case V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS:
> > + p_mpeg2_slice_params = ptr.p;
> > +
> > + switch (p_mpeg2_slice_params->intra_dc_precision) {
> > + case 0: /* 8 bits */
> > + case 1: /* 9 bits */
> > + case 11: /* 11 bits */
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + switch (p_mpeg2_slice_params->picture_structure) {
> > + case 1: /* interlaced top field */
> > + case 2: /* interlaced bottom field */
> > + case 3: /* progressive */
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + switch (p_mpeg2_slice_params->slice_type) {
> > + case V4L2_MPEG2_SLICE_TYPE_I:
> > + case V4L2_MPEG2_SLICE_TYPE_P:
> > + case V4L2_MPEG2_SLICE_TYPE_B:
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (p_mpeg2_slice_params->backward_ref_index > VIDEO_MAX_FRAME ||
> > + p_mpeg2_slice_params->forward_ref_index > VIDEO_MAX_FRAME)
>
> Should be >=

Noted, thanks.

> > + return -EINVAL;
> > +
> > + return 0;
> > +
> > + case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
> > + return 0;
> > +
> > default:
> > return -EINVAL;
> > }
> > @@ -2186,6 +2234,12 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> > case V4L2_CTRL_TYPE_U32:
> > elem_size = sizeof(u32);
> > break;
> > + case V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS:
> > + elem_size = sizeof(struct v4l2_ctrl_mpeg2_slice_params);
> > + break;
> > + case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
> > + elem_size = sizeof(struct v4l2_ctrl_mpeg2_quantization);
> > + break;
> > default:
> > if (type < V4L2_CTRL_COMPOUND_TYPES)
> > elem_size = sizeof(s32);
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 44fc0102221f..68e914b83a03 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1304,6 +1304,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> > case V4L2_PIX_FMT_H263: descr = "H.263"; break;
> > case V4L2_PIX_FMT_MPEG1: descr = "MPEG-1 ES"; break;
> > case V4L2_PIX_FMT_MPEG2: descr = "MPEG-2 ES"; break;
> > + case V4L2_PIX_FMT_MPEG2_SLICE: descr = "MPEG-2 parsed slice data"; break;
>
> "MPEG-2 Parsed Slice Data"

Will do.

> > case V4L2_PIX_FMT_MPEG4: descr = "MPEG-4 part 2 ES"; break;
> > case V4L2_PIX_FMT_XVID: descr = "Xvid"; break;
> > case V4L2_PIX_FMT_VC1_ANNEX_G: descr = "VC-1 (SMPTE 412M Annex G)"; break;
> > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > index 34ee3167d7dd..83eff6f91ed2 100644
> > --- a/include/media/v4l2-ctrls.h
> > +++ b/include/media/v4l2-ctrls.h
> > @@ -35,13 +35,15 @@ struct poll_table_struct;
> >
> > /**
> > * union v4l2_ctrl_ptr - A pointer to a control value.
> > - * @p_s32: Pointer to a 32-bit signed value.
> > - * @p_s64: Pointer to a 64-bit signed value.
> > - * @p_u8: Pointer to a 8-bit unsigned value.
> > - * @p_u16: Pointer to a 16-bit unsigned value.
> > - * @p_u32: Pointer to a 32-bit unsigned value.
> > - * @p_char: Pointer to a string.
> > - * @p: Pointer to a compound value.
> > + * @p_s32: Pointer to a 32-bit signed value.
> > + * @p_s64: Pointer to a 64-bit signed value.
> > + * @p_u8: Pointer to a 8-bit unsigned value.
> > + * @p_u16: Pointer to a 16-bit unsigned value.
> > + * @p_u32: Pointer to a 32-bit unsigned value.
> > + * @p_char: Pointer to a string.
> > + * @p_mpeg2_slice_params: Pointer to a MPEG2 slice parameters structure.
> > + * @p_mpeg2_quantization: Pointer to a MPEG2 quantization data structure.
> > + * @p: Pointer to a compound value.
> > */
> > union v4l2_ctrl_ptr {
> > s32 *p_s32;
> > @@ -50,6 +52,8 @@ union v4l2_ctrl_ptr {
> > u16 *p_u16;
> > u32 *p_u32;
> > char *p_char;
> > + struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> > + struct v4l2_ctrl_mpeg2_quantization *p_mpeg2_quantization;
> > void *p;
> > };
> >
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index e4ee10ee917d..ce6de781037a 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -557,6 +557,9 @@ enum v4l2_mpeg_video_mpeg4_profile {
> > };
> > #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL (V4L2_CID_MPEG_BASE+407)
> >
> > +#define V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (V4L2_CID_MPEG_BASE+450)
>
> I would insert this at V4L2_CID_MPEG_BASE+250
>
> That is close to the existing MPEG2 controls, which makes sense.

Agreed, will change for the next version.

> > +#define V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION (V4L2_CID_MPEG_BASE+451)
>
> and this becomes +251 of course.
>
> > +
> > /* Control IDs for VP8 streams
> > * Although VP8 is not part of MPEG we add these controls to the MPEG class
> > * as that class is already handling other video compression standards
> > @@ -1092,4 +1095,44 @@ enum v4l2_detect_md_mode {
> > #define V4L2_CID_DETECT_MD_THRESHOLD_GRID (V4L2_CID_DETECT_CLASS_BASE + 3)
> > #define V4L2_CID_DETECT_MD_REGION_GRID (V4L2_CID_DETECT_CLASS_BASE + 4)
> >
> > +#define V4L2_MPEG2_SLICE_TYPE_I 1
> > +#define V4L2_MPEG2_SLICE_TYPE_P 2
> > +#define V4L2_MPEG2_SLICE_TYPE_B 3
> > +#define V4L2_MPEG2_SLICE_TYPE_D 4
> > +
> > +struct v4l2_ctrl_mpeg2_slice_params {
> > + __u32 slice_len;
> > + __u32 slice_pos;
> > +
> > + __u16 width;
> > + __u16 height;
> > +
> > + __u8 slice_type;
> > + __u8 f_code[2][2];
> > +
> > + __u8 intra_dc_precision;
> > + __u8 picture_structure;
> > + __u8 top_field_first;
> > + __u8 frame_pred_frame_dct;
> > + __u8 concealment_motion_vectors;
> > + __u8 q_scale_type;
> > + __u8 intra_vlc_format;
> > + __u8 alternate_scan;
> > +
> > + __u8 backward_ref_index;
> > + __u8 forward_ref_index;
>
> How certain are you that this is all that's needed? Should we add
> a reserved field?

There is a limited number of parameters that the bitstream offers and
only a subset of these parameters affects the decoding process.

VAAPI defines such a subset, so we used it as a base to figure out what
fields from the bitstream are involved. I also went through the
specification in detail to check whether we missed some relevant fields
(and I found a few while preparing the next revision of this series).

Still, I doubt that the result is perfect and it's hard to know whether
a future decoder driver will require some specific field not required by
Cedrus and that we missed, so adding reserved fields seems like the
safest approach.

On the other hand, I think that specific extensions should be added as
separate controls, like it's done for the quantization matrices, so this
should limit the relevant number of reserved fields to add. Something
like 4-6 __u8 values seems reasonable IMO.

> > +};
> > +
> > +struct v4l2_ctrl_mpeg2_quantization {
> > + __u8 load_intra_quantiser_matrix : 1;
> > + __u8 load_non_intra_quantiser_matrix : 1;
> > + __u8 load_chroma_intra_quantiser_matrix : 1;
> > + __u8 load_chroma_non_intra_quantiser_matrix : 1;
>
> I wouldn't use bitfields here. It doesn't add anything.

Okay, then I'll drop bitfields from future patches as well.

Cheers,

Paul

> > +
> > + __u8 intra_quantiser_matrix[64];
> > + __u8 non_intra_quantiser_matrix[64];
> > + __u8 chroma_intra_quantiser_matrix[64];
> > + __u8 chroma_non_intra_quantiser_matrix[64];
> > +};
> > +
> > #endif
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 1f6c4b52baae..d171361ed9b3 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -629,6 +629,7 @@ struct v4l2_pix_format {
> > #define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /* H263 */
> > #define V4L2_PIX_FMT_MPEG1 v4l2_fourcc('M', 'P', 'G', '1') /* MPEG-1 ES */
> > #define V4L2_PIX_FMT_MPEG2 v4l2_fourcc('M', 'P', 'G', '2') /* MPEG-2 ES */
> > +#define V4L2_PIX_FMT_MPEG2_SLICE v4l2_fourcc('M', 'G', '2', 'S') /* MPEG-2 parsed slice data */
> > #define V4L2_PIX_FMT_MPEG4 v4l2_fourcc('M', 'P', 'G', '4') /* MPEG-4 part 2 ES */
> > #define V4L2_PIX_FMT_XVID v4l2_fourcc('X', 'V', 'I', 'D') /* Xvid */
> > #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
> > @@ -1587,6 +1588,8 @@ struct v4l2_ext_control {
> > __u8 __user *p_u8;
> > __u16 __user *p_u16;
> > __u32 __user *p_u32;
> > + struct v4l2_ctrl_mpeg2_slice_params __user *p_mpeg2_slice_params;
> > + struct v4l2_ctrl_mpeg2_quantization __user *p_mpeg2_quantization;
> > void __user *ptr;
> > };
> > } __attribute__ ((packed));
> > @@ -1632,6 +1635,8 @@ enum v4l2_ctrl_type {
> > V4L2_CTRL_TYPE_U8 = 0x0100,
> > V4L2_CTRL_TYPE_U16 = 0x0101,
> > V4L2_CTRL_TYPE_U32 = 0x0102,
> > + V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
> > + V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
> > };
> >
> >

> Regards,
>
> Hans
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Attachments:

signature.asc (499.00 B)
This is a digitally signed message part

2018-08-08 12:07:03

by Paul Kocialkowski

[permalink] [raw]

Subject: Re: [PATCH v6 1/8] media: v4l: Add definitions for MPEG2 slice format and metadata

Hi,

On Sat, 2018-08-04 at 15:30 +0200, Hans Verkuil wrote:
> On 07/25/2018 12:02 PM, Paul Kocialkowski wrote:

[...]

> > diff --git a/Documentation/media/uapi/v4l/pixfmt-compressed.rst b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > index abec03937bb3..4e73f62b5163 100644
> > --- a/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > +++ b/Documentation/media/uapi/v4l/pixfmt-compressed.rst
> > @@ -60,6 +60,11 @@ Compressed Formats
> > - ``V4L2_PIX_FMT_MPEG2``
> > - 'MPG2'
> > - MPEG2 video elementary stream.
> > + * .. _V4L2-PIX-FMT-MPEG2-SLICE:
> > +
> > + - ``V4L2_PIX_FMT_MPEG2_SLICE``
> > + - 'MG2S'
> > + - MPEG2 parsed slice data, as extracted from the MPEG2 bitstream.
>
> This does not mention that this requires the use of the Request API and which controls
> are compulsory in the request.

Right, so I will add a more extensive description regarding the intended
use case for this format, covering what controls are required.

> > * .. _V4L2-PIX-FMT-MPEG4:
> >
> > - ``V4L2_PIX_FMT_MPEG4``
> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> > index 3610dce3a4f8..22483d894259 100644
> > --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> > @@ -844,6 +844,8 @@ const char *v4l2_ctrl_get_name(u32 id)
> > case V4L2_CID_MPEG_VIDEO_MV_V_SEARCH_RANGE: return "Vertical MV Search Range";
> > case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER: return "Repeat Sequence Header";
> > case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME: return "Force Key Frame";
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS: return "MPEG2 Slice Header";
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION: return "MPEG2 Quantization Matrices";
>
> Use MPEG-2 instead of MPEG2 in these two descriptions.

Will do!

Cheers,

Paul

> Regards,
>
> Hans
>
> >
> > /* VPX controls */
> > case V4L2_CID_MPEG_VIDEO_VPX_NUM_PARTITIONS: return "VPX Number of Partitions";
> > @@ -1292,6 +1294,12 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
> > case V4L2_CID_RDS_TX_ALT_FREQS:
> > *type = V4L2_CTRL_TYPE_U32;
> > break;
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS:
> > + *type = V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS;
> > + break;
> > + case V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION:
> > + *type = V4L2_CTRL_TYPE_MPEG2_QUANTIZATION;
> > + break;
> > default:
> > *type = V4L2_CTRL_TYPE_INTEGER;
> > break;
> > @@ -1550,6 +1558,7 @@ static void std_log(const struct v4l2_ctrl *ctrl)
> > static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
> > union v4l2_ctrl_ptr ptr)
> > {
> > + struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> > size_t len;
> > u64 offset;
> > s64 val;
> > @@ -1612,6 +1621,45 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
> > return -ERANGE;
> > return 0;
> >
> > + case V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS:
> > + p_mpeg2_slice_params = ptr.p;
> > +
> > + switch (p_mpeg2_slice_params->intra_dc_precision) {
> > + case 0: /* 8 bits */
> > + case 1: /* 9 bits */
> > + case 11: /* 11 bits */
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + switch (p_mpeg2_slice_params->picture_structure) {
> > + case 1: /* interlaced top field */
> > + case 2: /* interlaced bottom field */
> > + case 3: /* progressive */
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + switch (p_mpeg2_slice_params->slice_type) {
> > + case V4L2_MPEG2_SLICE_TYPE_I:
> > + case V4L2_MPEG2_SLICE_TYPE_P:
> > + case V4L2_MPEG2_SLICE_TYPE_B:
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + if (p_mpeg2_slice_params->backward_ref_index > VIDEO_MAX_FRAME ||
> > + p_mpeg2_slice_params->forward_ref_index > VIDEO_MAX_FRAME)
> > + return -EINVAL;
> > +
> > + return 0;
> > +
> > + case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
> > + return 0;
> > +
> > default:
> > return -EINVAL;
> > }
> > @@ -2186,6 +2234,12 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
> > case V4L2_CTRL_TYPE_U32:
> > elem_size = sizeof(u32);
> > break;
> > + case V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS:
> > + elem_size = sizeof(struct v4l2_ctrl_mpeg2_slice_params);
> > + break;
> > + case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
> > + elem_size = sizeof(struct v4l2_ctrl_mpeg2_quantization);
> > + break;
> > default:
> > if (type < V4L2_CTRL_COMPOUND_TYPES)
> > elem_size = sizeof(s32);
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 44fc0102221f..68e914b83a03 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1304,6 +1304,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> > case V4L2_PIX_FMT_H263: descr = "H.263"; break;
> > case V4L2_PIX_FMT_MPEG1: descr = "MPEG-1 ES"; break;
> > case V4L2_PIX_FMT_MPEG2: descr = "MPEG-2 ES"; break;
> > + case V4L2_PIX_FMT_MPEG2_SLICE: descr = "MPEG-2 parsed slice data"; break;
> > case V4L2_PIX_FMT_MPEG4: descr = "MPEG-4 part 2 ES"; break;
> > case V4L2_PIX_FMT_XVID: descr = "Xvid"; break;
> > case V4L2_PIX_FMT_VC1_ANNEX_G: descr = "VC-1 (SMPTE 412M Annex G)"; break;
> > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
> > index 34ee3167d7dd..83eff6f91ed2 100644
> > --- a/include/media/v4l2-ctrls.h
> > +++ b/include/media/v4l2-ctrls.h
> > @@ -35,13 +35,15 @@ struct poll_table_struct;
> >
> > /**
> > * union v4l2_ctrl_ptr - A pointer to a control value.
> > - * @p_s32: Pointer to a 32-bit signed value.
> > - * @p_s64: Pointer to a 64-bit signed value.
> > - * @p_u8: Pointer to a 8-bit unsigned value.
> > - * @p_u16: Pointer to a 16-bit unsigned value.
> > - * @p_u32: Pointer to a 32-bit unsigned value.
> > - * @p_char: Pointer to a string.
> > - * @p: Pointer to a compound value.
> > + * @p_s32: Pointer to a 32-bit signed value.
> > + * @p_s64: Pointer to a 64-bit signed value.
> > + * @p_u8: Pointer to a 8-bit unsigned value.
> > + * @p_u16: Pointer to a 16-bit unsigned value.
> > + * @p_u32: Pointer to a 32-bit unsigned value.
> > + * @p_char: Pointer to a string.
> > + * @p_mpeg2_slice_params: Pointer to a MPEG2 slice parameters structure.
> > + * @p_mpeg2_quantization: Pointer to a MPEG2 quantization data structure.
> > + * @p: Pointer to a compound value.
> > */
> > union v4l2_ctrl_ptr {
> > s32 *p_s32;
> > @@ -50,6 +52,8 @@ union v4l2_ctrl_ptr {
> > u16 *p_u16;
> > u32 *p_u32;
> > char *p_char;
> > + struct v4l2_ctrl_mpeg2_slice_params *p_mpeg2_slice_params;
> > + struct v4l2_ctrl_mpeg2_quantization *p_mpeg2_quantization;
> > void *p;
> > };
> >
> > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> > index e4ee10ee917d..ce6de781037a 100644
> > --- a/include/uapi/linux/v4l2-controls.h
> > +++ b/include/uapi/linux/v4l2-controls.h
> > @@ -557,6 +557,9 @@ enum v4l2_mpeg_video_mpeg4_profile {
> > };
> > #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL (V4L2_CID_MPEG_BASE+407)
> >
> > +#define V4L2_CID_MPEG_VIDEO_MPEG2_SLICE_PARAMS (V4L2_CID_MPEG_BASE+450)
> > +#define V4L2_CID_MPEG_VIDEO_MPEG2_QUANTIZATION (V4L2_CID_MPEG_BASE+451)
> > +
> > /* Control IDs for VP8 streams
> > * Although VP8 is not part of MPEG we add these controls to the MPEG class
> > * as that class is already handling other video compression standards
> > @@ -1092,4 +1095,44 @@ enum v4l2_detect_md_mode {
> > #define V4L2_CID_DETECT_MD_THRESHOLD_GRID (V4L2_CID_DETECT_CLASS_BASE + 3)
> > #define V4L2_CID_DETECT_MD_REGION_GRID (V4L2_CID_DETECT_CLASS_BASE + 4)
> >
> > +#define V4L2_MPEG2_SLICE_TYPE_I 1
> > +#define V4L2_MPEG2_SLICE_TYPE_P 2
> > +#define V4L2_MPEG2_SLICE_TYPE_B 3
> > +#define V4L2_MPEG2_SLICE_TYPE_D 4
> > +
> > +struct v4l2_ctrl_mpeg2_slice_params {
> > + __u32 slice_len;
> > + __u32 slice_pos;
> > +
> > + __u16 width;
> > + __u16 height;
> > +
> > + __u8 slice_type;
> > + __u8 f_code[2][2];
> > +
> > + __u8 intra_dc_precision;
> > + __u8 picture_structure;
> > + __u8 top_field_first;
> > + __u8 frame_pred_frame_dct;
> > + __u8 concealment_motion_vectors;
> > + __u8 q_scale_type;
> > + __u8 intra_vlc_format;
> > + __u8 alternate_scan;
> > +
> > + __u8 backward_ref_index;
> > + __u8 forward_ref_index;
> > +};
> > +
> > +struct v4l2_ctrl_mpeg2_quantization {
> > + __u8 load_intra_quantiser_matrix : 1;
> > + __u8 load_non_intra_quantiser_matrix : 1;
> > + __u8 load_chroma_intra_quantiser_matrix : 1;
> > + __u8 load_chroma_non_intra_quantiser_matrix : 1;
> > +
> > + __u8 intra_quantiser_matrix[64];
> > + __u8 non_intra_quantiser_matrix[64];
> > + __u8 chroma_intra_quantiser_matrix[64];
> > + __u8 chroma_non_intra_quantiser_matrix[64];
> > +};
> > +
> > #endif
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index 1f6c4b52baae..d171361ed9b3 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -629,6 +629,7 @@ struct v4l2_pix_format {
> > #define V4L2_PIX_FMT_H263 v4l2_fourcc('H', '2', '6', '3') /* H263 */
> > #define V4L2_PIX_FMT_MPEG1 v4l2_fourcc('M', 'P', 'G', '1') /* MPEG-1 ES */
> > #define V4L2_PIX_FMT_MPEG2 v4l2_fourcc('M', 'P', 'G', '2') /* MPEG-2 ES */
> > +#define V4L2_PIX_FMT_MPEG2_SLICE v4l2_fourcc('M', 'G', '2', 'S') /* MPEG-2 parsed slice data */
> > #define V4L2_PIX_FMT_MPEG4 v4l2_fourcc('M', 'P', 'G', '4') /* MPEG-4 part 2 ES */
> > #define V4L2_PIX_FMT_XVID v4l2_fourcc('X', 'V', 'I', 'D') /* Xvid */
> > #define V4L2_PIX_FMT_VC1_ANNEX_G v4l2_fourcc('V', 'C', '1', 'G') /* SMPTE 421M Annex G compliant stream */
> > @@ -1587,6 +1588,8 @@ struct v4l2_ext_control {
> > __u8 __user *p_u8;
> > __u16 __user *p_u16;
> > __u32 __user *p_u32;
> > + struct v4l2_ctrl_mpeg2_slice_params __user *p_mpeg2_slice_params;
> > + struct v4l2_ctrl_mpeg2_quantization __user *p_mpeg2_quantization;
> > void __user *ptr;
> > };
> > } __attribute__ ((packed));
> > @@ -1632,6 +1635,8 @@ enum v4l2_ctrl_type {
> > V4L2_CTRL_TYPE_U8 = 0x0100,
> > V4L2_CTRL_TYPE_U16 = 0x0101,
> > V4L2_CTRL_TYPE_U32 = 0x0102,
> > + V4L2_CTRL_TYPE_MPEG2_SLICE_PARAMS = 0x0103,
> > + V4L2_CTRL_TYPE_MPEG2_QUANTIZATION = 0x0104,
> > };
> >
> > /* Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
> >
>
>
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com

Attachments:

signature.asc (499.00 B)
This is a digitally signed message part