linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ondrej Zary <linux@rainbow-software.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Joerg Heckenbach <joerg@heckenbach-aw.de>,
	Dwaine Garden <dwainegarden@rogers.com>,
	linux-media@vger.kernel.org,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usbvision: remove (broken) image format conversion
Date: Tue, 26 Apr 2011 10:30:21 +0200	[thread overview]
Message-ID: <201104261030.21681.linux@rainbow-software.org> (raw)
In-Reply-To: <201104260832.11150.hverkuil@xs4all.nl>

On Tuesday 26 April 2011, you wrote:
> On Monday, April 25, 2011 23:23:17 Ondrej Zary wrote:
> > The YVU420 and YUV422P formats are broken and cause kernel panic on use.
> > (YVU420 does not work and sometimes causes "unable to handle paging
> > request" panic, YUV422P always causes "NULL pointer dereference").
> >
> > As V4L2 spec says that drivers shouldn't do any in-kernel image format
> > conversion, remove it completely (except YUYV).
>
> What really should happen is that the conversion is moved to libv4lconvert.
> I've never had the time to tackle that, but it would improve this driver a
> lot.

Depending on isoc_mode module parameter, the device uses different image 
formats: YUV 4:2:2 interleaved, YUV 4:2:0 planar or compressed format.

Maybe the parameter should go away and these three formats exposed to 
userspace? Hopefully the non-compressed formats could be used directly 
without any conversion. But the compressed format (with new V4L2_PIX_FMT_ 
assigned?) should be preferred (as it provides much higher frame rates). The 
code moved into libv4lconvert would decompress the format and convert into 
something standard (YUV420?).

> Would you perhaps be interested in doing that work?

I can try it. But the hardware isn't mine so my time is limited.

> > The removal also reveals an off-by-one bug in enum_fmt ioctl - it misses
> > the last format, so this patch fixes it too.
>
> Good. But why are the GREY/RGB formats also removed? Are those broken as
> well?

GREY, RGB24 and RGB32 seem to work (at least with mplayer). RGB565 and RGB555 
have wrong colors. GREY is implemented only in compressed mode but can be 
selected in other modes too. Can't userspace do the conversion better?

> Regards,
>
> 	Hans

-- 
Ondrej Zary

  reply	other threads:[~2011-04-26  8:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-25 21:23 [PATCH] usbvision: remove (broken) image format conversion Ondrej Zary
2011-04-26  6:32 ` Hans Verkuil
2011-04-26  8:30   ` Ondrej Zary [this message]
2011-04-26 11:54     ` Hans de Goede
2011-04-26 12:33       ` Hans Verkuil
2011-04-26 20:40         ` Ondrej Zary
2011-05-03 10:29           ` Mauro Carvalho Chehab
2011-05-03 16:37             ` Ondrej Zary
2011-06-11 12:13               ` Mauro Carvalho Chehab
2011-06-16 14:28                 ` Hans de Goede
2011-04-26 11:11   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201104261030.21681.linux@rainbow-software.org \
    --to=linux@rainbow-software.org \
    --cc=dwainegarden@rogers.com \
    --cc=hverkuil@xs4all.nl \
    --cc=joerg@heckenbach-aw.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).