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

2011-06-16 Thread Hans de Goede 

Hi,

On 06/11/2011 02:13 PM, Mauro Carvalho Chehab wrote:

Hi Hans de G.,

Em 26-04-2011 08:54, Hans de Goede escreveu:

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.


As Ondrej couldn't work on that while he was with the hardware, could you
please try to address this issue?


I've put it on my to do list, not sure when I'll get around to it though.

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-06-11 Thread Mauro Carvalho Chehab 
Hi Hans de G.,

Em 26-04-2011 08:54, Hans de Goede escreveu:
 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.

As Ondrej couldn't work on that while he was with the hardware, could you
please try to address this issue?

Thanks,
Mauro


Em 03-05-2011 13:37, Ondrej Zary escreveu:
 On Tuesday 03 May 2011 12:29:45 Mauro Carvalho Chehab wrote:
 Em 26-04-2011 17:40, Ondrej Zary escreveu:
 On Tuesday 26 April 2011 14:33:20 Hans Verkuil wrote:

 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.

 While in a perfect world, this should be moved to userspace, I'm ok on
 keeping it there, but the OOPS/Panic conditions should be fixed.

 Could you please work on a patch fixing the broken stuff, without removing
 the conversions?
 
 I've already returned the hardware so I can't test the driver anymore.
 
 Did the YUV422P conversion ever work? The initialization of u and v pointers
 is missing in usbvision_parse_compress() function for YUV422P case.
 
 The following oops was captured when trying to fix YVU420. Seems to be
 related to the u pointer too...
 
 [  181.169233] usbvision_parse_compress before conversion
 [  181.169233] usbvision_parse_compress idx=0, format=842094169
 [  181.169233] usbvision_parse_compress YVU420 f=e088d000, Y=e0a3e000
 [  181.169233] usbvision_parse_compress YVU420 frame=df016b68
 [  181.169233] usbvision_parse_compress YVU420 frame-curline=0
 [  181.169233] usbvision_parse_compress YVU420 u=e08a4700, U=e0a50c00, 
 v=e089fc00, V=e0a55700
 [  181.169233] BUG: unable to handle kernel paging request at e08a4700
 [  181.169233] IP: [e097fd57] usbvision_parse_compress+0x599/0x76e 
 [usbvision]
 [  181.169233] *pde = 1f0c2067 *pte = 
 [  181.169233] Oops: 0002 [#1] SMP
 [  181.169233] last sysfs file: 
 /sys/devices/pci:00/:00:1d.0/usb2/2-1/i2c-0/uevent
 [  181.169233] Modules linked in: savage drm loop usbvision v4l2_common 
 snd_fm801 snd_tea575x_tuner videodev snd_intel8x0 e
 [  181.169233]
 [  181.169233] Pid: 0, comm: swapper Not tainted 2.6.39-rc2 #2/848P-ICH5
 [  181.169233] EIP: 0060:[e097fd57] EFLAGS: 00010046 CPU: 0
 [  181.169233] EIP is at usbvision_parse_compress+0x599/0x76e [usbvision]
 [  181.169233] EAX:  EBX: df016b68 ECX: e08a4700 EDX: e0a50c81
 [  181.169233] ESI:  EDI: e088d001 EBP: 00a0 ESP: df021cb4
 [  181.169233]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
 [  181.169233] Process swapper (pid: 0, ti=df02 task=c12dde60 
 task.ti=c12b)
 [  181.169233] Stack:
 [  181.169233]  e0986ed3 e09861a4 e08a4700 e0a50c00 e089fc00 e0a55700 
 0003 e089fc00
 [  181.169233]  e08a4700 00c0 0140 df021f14 df021d52 0506 
 00060027 0009
 [  181.169233]  00c0 3293 00605a25 ff00605a fff0 55d501fc 
 82995568 a0060124
 [  181.169233] Call Trace:
 [  181.169233]  [e0c9c69d] ? uhci_urb_enqueue+0x712/0x725 [uhci_hcd]
 [  181.169233]  [e08f4155] ? usb_hcd_submit_urb+0x4be/0x53d [usbcore]
 [  181.169233]  [c11e9840] ? printk+0xe/0x16
 [  181.169233]  [e0980486] ? usbvision_isoc_irq+0x55a/0x17a0 [usbvision]
 [  181.169233]  [e097f5f9] ? usbvision_write_reg_irq+0xd5/0x10f [usbvision]
 [  181.169233]  [e0981505] ? usbvision_isoc_irq+0x15d9/0x17a0 [usbvision]
 [  181.169233]  [c1026eb2] ? try_to_wake_up+0x13b/0x13b
 [  181.169233]  [c117057c] ? ata_scsi_qc_complete+0x2b4/0x2c2
 [  181.169233]  [c101e899] ? __wake_up+0x2c/0x3b
 [  181.169233]  [e08f33e5] ? usb_hcd_giveback_urb+0x46/0x71 [usbcore]
 [  181.169233]  [e0c9a98d] ? uhci_giveback_urb+0xea/0x15d [uhci_hcd]
 [  181.169233]  [e0c9aff7] ? uhci_scan_schedule+0x526/0x777 [uhci_hcd]
 [  181.169233]  [c117951b] ? __ata_sff_port_intr+0x97/0xa2
 [  181.169233]  [e0c9c9f8] ? uhci_irq+0xbf/0xcd [uhci_hcd]
 [  181.169233]  [e08f2d2e] ? usb_hcd_irq+0x1e/0x5f [usbcore]
 [  181.169233]  [c1058581] ? handle_irq_event_percpu+0x1e/0x106
 [  181.169233]  [c1059edc] ? handle_edge_irq+0xa0/0xa0
 [  181.169233]  [c105868a] ? handle_irq_event+0x21/0x37
 [  181.169233]  [c1059edc] ?

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

2011-05-03 Thread Mauro Carvalho Chehab 
Em 26-04-2011 17:40, Ondrej Zary escreveu:
 On Tuesday 26 April 2011 14:33:20 Hans Verkuil wrote:

 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.

While in a perfect world, this should be moved to userspace, I'm ok on keeping
it there, but the OOPS/Panic conditions should be fixed.

Could you please work on a patch fixing the broken stuff, without removing the
conversions?

Thanks,
Mauro
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-05-03 Thread Ondrej Zary 
On Tuesday 03 May 2011 12:29:45 Mauro Carvalho Chehab wrote:
 Em 26-04-2011 17:40, Ondrej Zary escreveu:
  On Tuesday 26 April 2011 14:33:20 Hans Verkuil wrote:
 
  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.

 While in a perfect world, this should be moved to userspace, I'm ok on
 keeping it there, but the OOPS/Panic conditions should be fixed.

 Could you please work on a patch fixing the broken stuff, without removing
 the conversions?

I've already returned the hardware so I can't test the driver anymore.

Did the YUV422P conversion ever work? The initialization of u and v pointers
is missing in usbvision_parse_compress() function for YUV422P case.

The following oops was captured when trying to fix YVU420. Seems to be
related to the u pointer too...

[  181.169233] usbvision_parse_compress before conversion
[  181.169233] usbvision_parse_compress idx=0, format=842094169
[  181.169233] usbvision_parse_compress YVU420 f=e088d000, Y=e0a3e000
[  181.169233] usbvision_parse_compress YVU420 frame=df016b68
[  181.169233] usbvision_parse_compress YVU420 frame-curline=0
[  181.169233] usbvision_parse_compress YVU420 u=e08a4700, U=e0a50c00, 
v=e089fc00, V=e0a55700
[  181.169233] BUG: unable to handle kernel paging request at e08a4700
[  181.169233] IP: [e097fd57] usbvision_parse_compress+0x599/0x76e [usbvision]
[  181.169233] *pde = 1f0c2067 *pte = 
[  181.169233] Oops: 0002 [#1] SMP
[  181.169233] last sysfs file: 
/sys/devices/pci:00/:00:1d.0/usb2/2-1/i2c-0/uevent
[  181.169233] Modules linked in: savage drm loop usbvision v4l2_common 
snd_fm801 snd_tea575x_tuner videodev snd_intel8x0 e
[  181.169233]
[  181.169233] Pid: 0, comm: swapper Not tainted 2.6.39-rc2 #2/848P-ICH5
[  181.169233] EIP: 0060:[e097fd57] EFLAGS: 00010046 CPU: 0
[  181.169233] EIP is at usbvision_parse_compress+0x599/0x76e [usbvision]
[  181.169233] EAX:  EBX: df016b68 ECX: e08a4700 EDX: e0a50c81
[  181.169233] ESI:  EDI: e088d001 EBP: 00a0 ESP: df021cb4
[  181.169233]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  181.169233] Process swapper (pid: 0, ti=df02 task=c12dde60 
task.ti=c12b)
[  181.169233] Stack:
[  181.169233]  e0986ed3 e09861a4 e08a4700 e0a50c00 e089fc00 e0a55700 0003 
e089fc00
[  181.169233]  e08a4700 00c0 0140 df021f14 df021d52 0506 00060027 
0009
[  181.169233]  00c0 3293 00605a25 ff00605a fff0 55d501fc 82995568 
a0060124
[  181.169233] Call Trace:
[  181.169233]  [e0c9c69d] ? uhci_urb_enqueue+0x712/0x725 [uhci_hcd]
[  181.169233]  [e08f4155] ? usb_hcd_submit_urb+0x4be/0x53d [usbcore]
[  181.169233]  [c11e9840] ? printk+0xe/0x16
[  181.169233]  [e0980486] ? usbvision_isoc_irq+0x55a/0x17a0 [usbvision]
[  181.169233]  [e097f5f9] ? usbvision_write_reg_irq+0xd5/0x10f [usbvision]
[  181.169233]  [e0981505] ? usbvision_isoc_irq+0x15d9/0x17a0 [usbvision]
[  181.169233]  [c1026eb2] ? try_to_wake_up+0x13b/0x13b
[  181.169233]  [c117057c] ? ata_scsi_qc_complete+0x2b4/0x2c2
[  181.169233]  [c101e899] ? __wake_up+0x2c/0x3b
[  181.169233]  [e08f33e5] ? usb_hcd_giveback_urb+0x46/0x71 [usbcore]
[  181.169233]  [e0c9a98d] ? uhci_giveback_urb+0xea/0x15d [uhci_hcd]
[  181.169233]  [e0c9aff7] ? uhci_scan_schedule+0x526/0x777 [uhci_hcd]
[  181.169233]  [c117951b] ? __ata_sff_port_intr+0x97/0xa2
[  181.169233]  [e0c9c9f8] ? uhci_irq+0xbf/0xcd [uhci_hcd]
[  181.169233]  [e08f2d2e] ? usb_hcd_irq+0x1e/0x5f [usbcore]
[  181.169233]  [c1058581] ? handle_irq_event_percpu+0x1e/0x106
[  181.169233]  [c1059edc] ? handle_edge_irq+0xa0/0xa0
[  181.169233]  [c105868a] ? handle_irq_event+0x21/0x37
[  181.169233]  [c1059edc] ? handle_edge_irq+0xa0/0xa0
[  181.169233]  [c1059f42] ? handle_fasteoi_irq+0x66/0x7e
[  181.169233]  IRQ
[  181.169233]  [c10035c7] ? do_IRQ+0x2e/0x84
[  181.169233]  [c11ec370] ? common_interrupt+0x30/0x38
[  181.169233]  [c1007699] ? mwait_idle+0x4f/0x54
[  181.169233]  [c1001a86] ? cpu_idle+0x91/0xab
[  181.169233]  [c12f96d4] ? start_kernel+0x2a3/0x2a8
[  181.169233] Code: 08 ff 35 8c a1 98 e0 ff 74 24 14 68 a4 61 98 e0 68 d3 6e 
98 e0 e8 ec 9a 86 e0 8b 15 8c a1 98 e0 89 f0
[  181.169233]  11 8b 15 88 a1 98 e0 41 89 4c 24

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

2011-04-26 Thread Hans Verkuil 
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.

Would you perhaps be interested in doing that work?

 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?

Regards,

Hans

 
 This allows the driver to work with mplayer without need to manually specify a
 format and also to work with VLC without causing kernel panic.
 
 Signed-off-by: Ondrej Zary li...@rainbow-software.org
 
 diff -up linux-2.6.39-rc2-/drivers/media/video/usbvision/usbvision-core.c 
 linux-2.6.39-rc2/drivers/media/video/usbvision/usbvision-core.c
 --- linux-2.6.39-rc2-/drivers/media/video/usbvision/usbvision-core.c  
 2011-04-25 22:30:09.0 +0200
 +++ linux-2.6.39-rc2/drivers/media/video/usbvision/usbvision-core.c   
 2011-04-25 23:07:46.0 +0200
 @@ -586,9 +586,8 @@ static enum parse_state usbvision_parse_
   int len;
   int i;
   unsigned char yuyv[4] = { 180, 128, 10, 128 }; /* YUV components */
 - unsigned char rv, gv, bv;   /* RGB components */
 - int clipmask_index, bytes_per_pixel;
 - int stretch_bytes, clipmask_add;
 + int bytes_per_pixel;
 + int stretch_bytes;
  
   frame  = usbvision-cur_frame;
   f = frame-data + (frame-v4l2_linesize * frame-curline);
 @@ -605,78 +604,16 @@ static enum parse_state usbvision_parse_
  
   bytes_per_pixel = frame-v4l2_format.bytes_per_pixel;
   stretch_bytes = (usbvision-stretch_width - 1) * bytes_per_pixel;
 - clipmask_index = frame-curline * MAX_FRAME_WIDTH;
 - clipmask_add = usbvision-stretch_width;
  
   for (i = 0; i  frame-frmwidth; i += (2 * usbvision-stretch_width)) {
   scratch_get(usbvision, yuyv[0], 4);
  
 - if (frame-v4l2_format.format == V4L2_PIX_FMT_YUYV) {
 - *f++ = yuyv[0]; /* Y */
 - *f++ = yuyv[3]; /* U */
 - } else {
 - YUV_TO_RGB_BY_THE_BOOK(yuyv[0], yuyv[1], yuyv[3], rv, 
 gv, bv);
 - switch (frame-v4l2_format.format) {
 - case V4L2_PIX_FMT_RGB565:
 - *f++ = (0x1F  rv) |
 - (0xE0  (gv  5));
 - *f++ = (0x07  (gv  3)) |
 - (0xF8   bv);
 - break;
 - case V4L2_PIX_FMT_RGB24:
 - *f++ = rv;
 - *f++ = gv;
 - *f++ = bv;
 - break;
 - case V4L2_PIX_FMT_RGB32:
 - *f++ = rv;
 - *f++ = gv;
 - *f++ = bv;
 - f++;
 - break;
 - case V4L2_PIX_FMT_RGB555:
 - *f++ = (0x1F  rv) |
 - (0xE0  (gv  5));
 - *f++ = (0x03  (gv  3)) |
 - (0x7C  (bv  2));
 - break;
 - }
 - }
 - clipmask_index += clipmask_add;
 + *f++ = yuyv[0]; /* Y */
 + *f++ = yuyv[3]; /* U */
   f += stretch_bytes;
  
 - if (frame-v4l2_format.format == V4L2_PIX_FMT_YUYV) {
 - *f++ = yuyv[2]; /* Y */
 - *f++ = yuyv[1]; /* V */
 - } else {
 - YUV_TO_RGB_BY_THE_BOOK(yuyv[2], yuyv[1], yuyv[3], rv, 
 gv, bv);
 - switch (frame-v4l2_format.format) {
 - case V4L2_PIX_FMT_RGB565:
 - *f++ = (0x1F  rv) |
 - (0xE0  (gv  5));
 - *f++ = (0x07  (gv  3)) |
 - (0xF8   bv);
 - break;
 - case V4L2_PIX_FMT_RGB24:
 - *f++ = rv;
 - *f++ = gv;
 - *f++ = bv;
 - break;
 - case V4L2_PIX_FMT_RGB32:
 - *f++ = rv;
 - *f++ = gv;
 - *f++ = bv;
 - f++;
 - break;
 -

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

2011-04-26 Thread Ondrej Zary 
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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-04-26 Thread Hans de Goede 

Hi,

On 04/26/2011 08:32 AM, Hans Verkuil 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.

Would you perhaps be interested in doing that work?



Seconded! I would be more then happy to help with the libv4l part (in the form
of review + merging + advice).

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-04-26 Thread Hans de Goede 

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.

Thanks  Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-04-26 Thread Hans Verkuil 
 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.

Regards,

   Hans


 Thanks  Regards,

 Hans



--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-04-26 Thread Ondrej Zary 
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-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2011-04-25 Thread Ondrej Zary 
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).

The removal also reveals an off-by-one bug in enum_fmt ioctl - it misses the
last format, so this patch fixes it too.

This allows the driver to work with mplayer without need to manually specify a
format and also to work with VLC without causing kernel panic.

Signed-off-by: Ondrej Zary li...@rainbow-software.org

diff -up linux-2.6.39-rc2-/drivers/media/video/usbvision/usbvision-core.c 
linux-2.6.39-rc2/drivers/media/video/usbvision/usbvision-core.c
--- linux-2.6.39-rc2-/drivers/media/video/usbvision/usbvision-core.c
2011-04-25 22:30:09.0 +0200
+++ linux-2.6.39-rc2/drivers/media/video/usbvision/usbvision-core.c 
2011-04-25 23:07:46.0 +0200
@@ -586,9 +586,8 @@ static enum parse_state usbvision_parse_
int len;
int i;
unsigned char yuyv[4] = { 180, 128, 10, 128 }; /* YUV components */
-   unsigned char rv, gv, bv;   /* RGB components */
-   int clipmask_index, bytes_per_pixel;
-   int stretch_bytes, clipmask_add;
+   int bytes_per_pixel;
+   int stretch_bytes;
 
frame  = usbvision-cur_frame;
f = frame-data + (frame-v4l2_linesize * frame-curline);
@@ -605,78 +604,16 @@ static enum parse_state usbvision_parse_
 
bytes_per_pixel = frame-v4l2_format.bytes_per_pixel;
stretch_bytes = (usbvision-stretch_width - 1) * bytes_per_pixel;
-   clipmask_index = frame-curline * MAX_FRAME_WIDTH;
-   clipmask_add = usbvision-stretch_width;
 
for (i = 0; i  frame-frmwidth; i += (2 * usbvision-stretch_width)) {
scratch_get(usbvision, yuyv[0], 4);
 
-   if (frame-v4l2_format.format == V4L2_PIX_FMT_YUYV) {
-   *f++ = yuyv[0]; /* Y */
-   *f++ = yuyv[3]; /* U */
-   } else {
-   YUV_TO_RGB_BY_THE_BOOK(yuyv[0], yuyv[1], yuyv[3], rv, 
gv, bv);
-   switch (frame-v4l2_format.format) {
-   case V4L2_PIX_FMT_RGB565:
-   *f++ = (0x1F  rv) |
-   (0xE0  (gv  5));
-   *f++ = (0x07  (gv  3)) |
-   (0xF8   bv);
-   break;
-   case V4L2_PIX_FMT_RGB24:
-   *f++ = rv;
-   *f++ = gv;
-   *f++ = bv;
-   break;
-   case V4L2_PIX_FMT_RGB32:
-   *f++ = rv;
-   *f++ = gv;
-   *f++ = bv;
-   f++;
-   break;
-   case V4L2_PIX_FMT_RGB555:
-   *f++ = (0x1F  rv) |
-   (0xE0  (gv  5));
-   *f++ = (0x03  (gv  3)) |
-   (0x7C  (bv  2));
-   break;
-   }
-   }
-   clipmask_index += clipmask_add;
+   *f++ = yuyv[0]; /* Y */
+   *f++ = yuyv[3]; /* U */
f += stretch_bytes;
 
-   if (frame-v4l2_format.format == V4L2_PIX_FMT_YUYV) {
-   *f++ = yuyv[2]; /* Y */
-   *f++ = yuyv[1]; /* V */
-   } else {
-   YUV_TO_RGB_BY_THE_BOOK(yuyv[2], yuyv[1], yuyv[3], rv, 
gv, bv);
-   switch (frame-v4l2_format.format) {
-   case V4L2_PIX_FMT_RGB565:
-   *f++ = (0x1F  rv) |
-   (0xE0  (gv  5));
-   *f++ = (0x07  (gv  3)) |
-   (0xF8   bv);
-   break;
-   case V4L2_PIX_FMT_RGB24:
-   *f++ = rv;
-   *f++ = gv;
-   *f++ = bv;
-   break;
-   case V4L2_PIX_FMT_RGB32:
-   *f++ = rv;
-   *f++ = gv;
-   *f++ = bv;
-   f++;
-   break;
-   case V4L2_PIX_FMT_RGB555:
-   *f++ = (0x1F  rv) |
-   (0xE0  (gv  5));
-   *f++ = (0x03  (gv  3)) |
-   (0x7C  (bv  2));
-   break;
-