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: "Hans de Goede" <hdegoede@redhat.com>,
	"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 22:40:35 +0200	[thread overview]
Message-ID: <201104262240.40497.linux@rainbow-software.org> (raw)
In-Reply-To: <f2291b622da20d240c4ebe0ae72beb8c.squirrel@webmail.xs4all.nl>

On Tuesday 26 April 2011 14:33:20 Hans Verkuil wrote:
> > Hi,
> >
> > On 04/26/2011 10:30 AM, Ondrej Zary wrote:
> >> 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?
> >
> > That sounds right,
> >
> >> 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?).
> >
> > Correct.
> >
> >>> Would you perhaps be interested in doing that work?
> >>
> >> I can try it. But the hardware isn't mine so my time is limited.
> >
> > If you could give it a shot that would be great. I've some hardware to
> > test this with (although I've never actually tested that hardware), so
> > I can hopefully pick things up if you cannot finish things before you
> > need to return the hardware.
>
> I can also test this.

After digging in the code for hours, I'm giving this up. It's not worth it.

The ISOC_MODE_YUV422 mode works as V4L2_PIX_FMT_YVYU with VLC and 
mplayer+libv4lconvert, reducing the loop (and dropping strech_*) in 
usbvision_parse_lines_422() to:
 scratch_get(usbvision, frame->data + (frame->v4l2_linesize * frame->curline),
 2 * frame->frmwidth);

The ISOC_MODE_YUV420 is some weird custom format with 64-byte lines of YYUV. 
usbvision_parse_lines_420() is real mess with that scratch_* crap everywhere.

ISOC_MODE_COMPRESS: There are callbacks to usbvision_request_intra() and also 
usbvision_adjust_compression(). This is not going to work outside the kernel.


So I can redo the conversion removal patch to keep the RGB formats and also 
provide another one to remove the testpattern (it oopses too). But I'm not 
going to do any major changes in the driver.

-- 
Ondrej Zary

  reply	other threads:[~2011-04-26 20:40 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
2011-04-26 11:54     ` Hans de Goede
2011-04-26 12:33       ` Hans Verkuil
2011-04-26 20:40         ` Ondrej Zary [this message]
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=201104262240.40497.linux@rainbow-software.org \
    --to=linux@rainbow-software.org \
    --cc=dwainegarden@rogers.com \
    --cc=hdegoede@redhat.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).