Re: [PATCH] usbvision: remove (broken) image format conversion

From: Ondrej Zary
Date: Tue Apr 26 2011 - 16:41:01 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/