[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 6 changesets: 01/06: uvcvideo: Fix controls blacklisting http://linuxtv.org/hg/~pinchartl/uvcvideo/?cmd=changeset;node=0826158f044c 02/06: uvcvideo: Switch to a monotonic clock for V4L2 buffers timestamps http://linuxtv.org/hg/~pinchartl/uvcvideo/?cmd=changeset;node=101e9906760b 03/06: uvcvideo: Make the quirks module parameter override the built-in quirks http://linuxtv.org/hg/~pinchartl/uvcvideo/?cmd=changeset;node=655336840534 04/06: uvcvideo: Fix alternate setting selection in isochronous mode http://linuxtv.org/hg/~pinchartl/uvcvideo/?cmd=changeset;node=033b5968aa1a 05/06: uvcvideo: add another YUYV format GUID for iSight cameras http://linuxtv.org/hg/~pinchartl/uvcvideo/?cmd=changeset;node=c1f376eae978 06/06: uvcvideo: Fix oops caused by a race condition in buffer dequeuing http://linuxtv.org/hg/~pinchartl/uvcvideo/?cmd=changeset;node=fc9c55827738 The last changeset is a high-prio one, and should probably be backported to the stable kernel line. uvc_ctrl.c |9 +++- uvc_driver.c | 65 ++- uvc_queue.c | 14 uvc_video.c | 55 ++--- uvcvideo.h |9 ++-- 5 files changed, 113 insertions(+), 39 deletions(-) -- Thanks, Laurent Pinchart -- 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 5 changesets: uvcvideo: Add support for Genius eFace 2025 webcams uvcvideo: Merge iterms, oterms and units linked lists uvcvideo: Fix extension units parsing uvcvideo: Refactor chain scan uvcvideo: Factorize common field in uvc_entity structure uvc_ctrl.c | 29 +- uvc_driver.c | 257 +-- uvc_v4l2.c | 16 ++- uvcvideo.h | 14 +-- 4 files changed, 148 insertions(+), 168 deletions(-) -- Thanks, Laurent Pinchart -- 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 4 changesets: uvcvideo: Add support for MSI StarCam 370i webcams uvcvideo: Ignore the FIX_BANDWIDTH for compressed video uvcvideo: Return -EINVAL instead of -ENODEV in read() uvcvideo: Fix compilation warning with 2.6.32 due to type mismatch with abs() Two of them are marked as "Priority: high" and should go to 2.6.32 if possible. uvc_ctrl.c |2 +- uvc_driver.c |9 + uvc_v4l2.c |2 +- uvc_video.c |3 ++- 4 files changed, 13 insertions(+), 3 deletions(-) -- Thanks, Laurent Pinchart -- 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 8 changesets: uvcvideo: Handle V4L2_CTRL_TYPE_BUTTON control type in VIDIOC_QUERYCTRL uvcvideo: Add a new UVC_TRACE_VIDEO trace level uvcvideo: Don't acquire privileges in VIDIOC_TRY_FMT uvcvideo: Dismiss privileges when freeing video buffers uvcvideo: Rely on videodev to reference-count the device uvcvideo: Fix uvc_alloc_urb_buffers() uvcvideo: Handle garbage at the end of streaming interface descriptors uvcvideo: Add a module parameter to set the streaming control timeout uvc_ctrl.c |7 ++ uvc_driver.c | 172 +++ uvc_v4l2.c | 42 -- uvc_video.c | 26 ++-- uvcvideo.h |9 +-- 5 files changed, 135 insertions(+), 121 deletions(-) -- Thanks, Laurent Pinchart -- 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 3 changesets: uvcvideo: Restructure the driver to support multiple simultaneous streams. uvcvideo: Multiple streaming interfaces support uvcvideo: Avoid flooding the kernel log with "unknown event type" messages The 3rd one is a bugfix (marked as Priority: high) that should go to 2.6.31 if still possible. uvc_ctrl.c | 82 +++ uvc_driver.c | 654 +++ uvc_isight.c |7 uvc_status.c |4 uvc_v4l2.c | 275 uvc_video.c | 417 +++-- uvcvideo.h | 149 ++--- 7 files changed, 878 insertions(+), 710 deletions(-) Thanks, Laurent Pinchart -- 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 2 changesets: uvcvideo: Add PROBE_DEF quirk and enable it for the MT6227 device uvcvideo: Don't apply the FIX_BANDWIDTH quirk to all ViMicro devices uvc_driver.c | 25 ++--- uvc_video.c |3 +++ uvcvideo.h |1 + 3 files changed, 26 insertions(+), 3 deletions(-) The second changeset fixes a regression introduced in 2.6.30. Is it still time to get it in 2.6.31 ? Thanks, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Em Fri, 3 Jul 2009 17:37:35 +0200 Laurent Pinchart escreveu: > Mauro, > > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ > > for the following 5 changesets: > > uvcvideo: Use class-specific descriptor types from usb/ch9.h > uvcvideo: Prefix all UVC constants with UVC_ > uvcvideo: Remove unused Logitech-specific constants > uvcvideo: Move UVC definitions to linux/usb/video.h > uvcvideo: Set PROBE_MINMAX quirk for Aveo Technology webcams > > b/linux/include/linux/usb/video.h | 164 > linux/drivers/media/video/uvc/uvc_ctrl.c | 205 --- > linux/drivers/media/video/uvc/uvc_driver.c | 139 +- > linux/drivers/media/video/uvc/uvc_v4l2.c | 14 - > linux/drivers/media/video/uvc/uvc_video.c | 30 +- > linux/drivers/media/video/uvc/uvcvideo.h | 374 > - > 6 files changed, 474 insertions(+), 452 deletions(-) > > The fourth patch (Move UVC definitions to linux/usb/video.h) has been > submitted to the linux-usb mailing list and acked by Greh KH. Ok. I'm adding it on my linux-next tree. Cheers, 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 5 changesets: uvcvideo: Use class-specific descriptor types from usb/ch9.h uvcvideo: Prefix all UVC constants with UVC_ uvcvideo: Remove unused Logitech-specific constants uvcvideo: Move UVC definitions to linux/usb/video.h uvcvideo: Set PROBE_MINMAX quirk for Aveo Technology webcams b/linux/include/linux/usb/video.h | 164 linux/drivers/media/video/uvc/uvc_ctrl.c | 205 --- linux/drivers/media/video/uvc/uvc_driver.c | 139 +- linux/drivers/media/video/uvc/uvc_v4l2.c | 14 - linux/drivers/media/video/uvc/uvc_video.c | 30 +- linux/drivers/media/video/uvc/uvcvideo.h | 374 - 6 files changed, 474 insertions(+), 452 deletions(-) The fourth patch (Move UVC definitions to linux/usb/video.h) has been submitted to the linux-usb mailing list and acked by Greh KH. Thanks, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
On Tue, 9 Jun 2009, Mauro Carvalho Chehab wrote: > Em Tue, 9 Jun 2009 18:08:38 +0200 > Hans Verkuil escreveu: > > > > > > Should I submit a patch that implement VIDIOC_S_JPEGCOMP support in the > > > > UVC driver and implement a JPEG compression quality control later, or > > > > would you prefer the driver not to implement VIDIOC_S_JPEGCOMP at all ? > > > > As there are existing applications using that ioctl a few users are > > > > pushing for VIDIOC_S_JPEGCOMP support. > > > > > > I prefer the later. Adding a new ioctl support just to deprecate it on > > > the next kernel doesn't seem nice. Let's add directly the newer controls > > > and add a patch marking this as deprecated. > > > > I'm not sure whether we can deprecate JPEGCOMP. It is used in too many > > places. Perhaps we should create a set of JPEG controls that match what is > > in v4l2_jpegcompression and convert all the drivers that use JPEGCOMP to > > these new controls. Then the v4l core can map S/G_JPEGCOMP ioctls to a set > > of control read/writes. I'm working on string control support, so that will > > allow us to handle the APP_data and COM_data fields. > > This seems to be the correct approach. Implement it as controls, and let > video_ioctl2 to handle the calls to the legacy ioctls, while marking it as > deprecate. One problem is that COM and APP segment actually have a string of bytes associated with them. Right now we only have boolean and interger controls. There is no way to set a control to the 32-bytes you want put into a APP segment. Now, if we added NUL terminated string and fixed length byte arrays as control types this could be done. I know that's something I've mentioned before. It would also be a step to allowing drivers to export metadata about captured images (exposured info, focus tracking, etc.) via a control-like interface. -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Em Tue, 9 Jun 2009 18:08:38 +0200 Hans Verkuil escreveu: > > > Should I submit a patch that implement VIDIOC_S_JPEGCOMP support in the > > > UVC driver and implement a JPEG compression quality control later, or > > > would you prefer the driver not to implement VIDIOC_S_JPEGCOMP at all ? > > > As there are existing applications using that ioctl a few users are > > > pushing for VIDIOC_S_JPEGCOMP support. > > > > I prefer the later. Adding a new ioctl support just to deprecate it on > > the next kernel doesn't seem nice. Let's add directly the newer controls > > and add a patch marking this as deprecated. > > I'm not sure whether we can deprecate JPEGCOMP. It is used in too many > places. Perhaps we should create a set of JPEG controls that match what is > in v4l2_jpegcompression and convert all the drivers that use JPEGCOMP to > these new controls. Then the v4l core can map S/G_JPEGCOMP ioctls to a set > of control read/writes. I'm working on string control support, so that will > allow us to handle the APP_data and COM_data fields. This seems to be the correct approach. Implement it as controls, and let video_ioctl2 to handle the calls to the legacy ioctls, while marking it as deprecate. Cheers, 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 5 changesets: uvcvideo: Add generic control blacklist. uvcvideo: Don't accept to change the format when buffers are allocated. uvcvideo: Add support for Aveo Technology webcams uvcvideo: Add support for FSC V30S webcams uvcvideo: Ignore non-UVC trailing interface descriptors. uvc_ctrl.c | 35 +++ uvc_driver.c | 25 + uvc_queue.c | 14 ++ uvc_v4l2.c |2 +- uvcvideo.h |2 +- 5 files changed, 52 insertions(+), 26 deletions(-) I've removed the VIDIOC_[GS]_JPEGCOMP patch and I'll implement JPEG compression quality setting through a V4L2 control. Thanks, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
On Tuesday 09 June 2009 17:56:01 Mauro Carvalho Chehab wrote:
> Em Tue, 9 Jun 2009 17:29:58 +0200
>
> Laurent Pinchart escreveu:
> > > Since uvc is part of the kernel, in fact, it is uvcvideo that is
> > > adding more exceptions to the Kernel style.
> >
> > Damn, I hoped you wouldn't notice that ;-)
> >
> :)
> :
> > I find the
> >
> > if ((ret = function()) < 0)
> >
> > more compact (which can't be debated) and as easy to read as the
> > alternative, if not easier (now this can be debated). As this point of
> > view seems to be minority I'll try to adjust my coding style
> > accordingly. Of course I'll blame you personally for any bug that it
> > would introduce ;-)
>
> It is more compact, and I used to write codes like above in the past.
> Yet, we should stick on Kernel style at the kernelspace stuff.
>
> About the readability, a code like:
>
> ret = function;
> if (ret < 0)
>
> is just a little more readable than on a single line. However, if codes
> like the above are accepted, it should be accepted things like:
>
> if ((ret = functionX() * functionY() + 3) < 5 + 2 * functionZ())
>
> that would be more complex to read than when broken into two separate
> statements:
>
> ret = functionX() * functionY() + 3;
> if (ret < 5 + 2 * functionZ())
>
> Anyway, since Coding Style is global to the kernel as a hole, it is out
> of topic to discuss about the rationale behind each rule, or proposing
> improvements here. The proper forum for it is LKML.
>
> > > > > Also, I have a few comments, from API POV.
> > > > >
> > > > > It doesn't seem that those ioctls are properly implemented. There
> > > > > are some things there that sounded obfuscated for me, and it you
> > > > > aren't implementing. Probably because it is not clear enough.
> > > >
> > > > The UVC standard doesn't specify a way to add/remove specific JPEG
> > > > segments. As MJPEG has not COM, DRI and DHT segments, I've
> > > > hardcoded flags to DQT.
> > > >
> > > > > Also, we used to have a similar set of ioctls for MPEG, that were
> > > > > removed, in favor of the usage of extended controls, with a
> > > > > cleaner interface.
> > > > >
> > > > > That's said, instead of adding more support for this obfuscated
> > > > > API, maybe we could deprecate those in favor of some controls
> > > > > that could make more sense, and let vidio_ioctl2 provide backward
> > > > > compat for it by calling the proper controls, just to preserve
> > > > > binary compatibility with legacy applications.
> > > >
> > > > I have mixed feelings about this. On one hand I agree that
> > > > VIDIOC_S_JPEGCOMP is under-specified and should be either properly
> > > > document, or deprecate it in favor of a control. On the other hand,
> > > > the uvcvideo driver assumes that controls can all be read and
> > > > written while streaming is in progress. I suppose other drivers
> > > > (probably non-MPEG ones) were written with the same assumption in
> > > > mind. Using a control to set JPEG compression would mean that a
> > > > proper infrastructure would need to be put in place in those
> > > > drivers to handle controls that influence video streaming. I'd
> > > > appreciate your opinion on that.
> > >
> > > I'm in favor of deprecate VIDIOC_S_JPEGCOMP.
> > >
> > > The issue you've described of changing the format while streaming
> > > exists, being it a control, or an ioctl. So, it deserves a separate
> > > discussion.
> > >
> > > -
> > >
> > > The MPEG drivers block trials of changing the MPEG format that can't
> > > be done while streaming. On the other hand, user preference controls,
> > > like bright, contrast, (auto)gain, exposure, etc can be done anytime.
> > > As most drivers just exposes such controls, they don't need a logic
> > > to block a control while streaming.
> > >
> > > The case of JPEG/MJPEG is similar to MPEG: there are some changes
> > > that may not be possible while streaming. So, such changes should, in
> > > thesis, be blocked.
> > >
> > > On the other hand, from users perspective, it seems interesting for
> > > they to start an application and, for example, change the JPEG
> > > quality while streaming, to see what better fits his needs.
> > >
> > > Since JPEG is just a set of independent jpeg images, in thesis, it is
> > > possible to change the encoding while streaming, without breaking for
> > > userspace, assuming that the allocated buffers are large enough to
> > > support such changes.
> > >
> > > So, IMO, we should try to allow such changes where possible, even by
> > > doing something like this, at the loop that fills the video buffer:
> > >
> > > if (streaming && frame_is_complete && quality_changed) {
> > > stop_streaming();
> > > change_quality();
> > > start_streaming();
> > > }
> >
> > This can only be done if the allocated buffers are big enough. As the
> > device reports the required buffer size the driver would have to query
> > the device before deciding if it allows to change the quality during
> > streaming. Given the usefulness of cha
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Em Tue, 9 Jun 2009 17:29:58 +0200
Laurent Pinchart escreveu:
>
> > Since uvc is part of the kernel, in fact, it is uvcvideo that is adding
> > more exceptions to the Kernel style.
>
> Damn, I hoped you wouldn't notice that ;-)
:)
>
> I find the
>
> if ((ret = function()) < 0)
>
> more compact (which can't be debated) and as easy to read as the alternative,
> if not easier (now this can be debated). As this point of view seems to be
> minority I'll try to adjust my coding style accordingly. Of course I'll blame
> you personally for any bug that it would introduce ;-)
It is more compact, and I used to write codes like above in the past. Yet, we
should stick on Kernel style at the kernelspace stuff.
About the readability, a code like:
ret = function;
if (ret < 0)
is just a little more readable than on a single line. However, if codes like
the above are accepted, it should be accepted things like:
if ((ret = functionX() * functionY() + 3) < 5 + 2 * functionZ())
that would be more complex to read than when broken into two separate
statements:
ret = functionX() * functionY() + 3;
if (ret < 5 + 2 * functionZ())
Anyway, since Coding Style is global to the kernel as a hole, it is out of
topic to discuss about the rationale behind each rule, or proposing
improvements here. The proper forum for it is LKML.
> > > > Also, I have a few comments, from API POV.
> > > >
> > > > It doesn't seem that those ioctls are properly implemented. There are
> > > > some things there that sounded obfuscated for me, and it you aren't
> > > > implementing. Probably because it is not clear enough.
> > >
> > > The UVC standard doesn't specify a way to add/remove specific JPEG
> > > segments. As MJPEG has not COM, DRI and DHT segments, I've hardcoded
> > > flags to DQT.
> > >
> > > > Also, we used to have a similar set of ioctls for MPEG, that were
> > > > removed, in favor of the usage of extended controls, with a cleaner
> > > > interface.
> > > >
> > > > That's said, instead of adding more support for this obfuscated API,
> > > > maybe we could deprecate those in favor of some controls that could
> > > > make more sense, and let vidio_ioctl2 provide backward compat for it by
> > > > calling the proper controls, just to preserve binary compatibility with
> > > > legacy applications.
> > >
> > > I have mixed feelings about this. On one hand I agree that
> > > VIDIOC_S_JPEGCOMP is under-specified and should be either properly
> > > document, or deprecate it in favor of a control. On the other hand, the
> > > uvcvideo driver assumes that controls can all be read and written while
> > > streaming is in progress. I suppose other drivers (probably non-MPEG
> > > ones) were written with the same assumption in mind. Using a control to
> > > set JPEG compression would mean that a proper infrastructure would need
> > > to be put in place in those drivers to handle controls that influence
> > > video streaming. I'd appreciate your opinion on that.
> >
> > I'm in favor of deprecate VIDIOC_S_JPEGCOMP.
> >
> > The issue you've described of changing the format while streaming exists,
> > being it a control, or an ioctl. So, it deserves a separate discussion.
> >
> > -
> >
> > The MPEG drivers block trials of changing the MPEG format that can't be
> > done while streaming. On the other hand, user preference controls, like
> > bright, contrast, (auto)gain, exposure, etc can be done anytime. As most
> > drivers just exposes such controls, they don't need a logic to block a
> > control while streaming.
> >
> > The case of JPEG/MJPEG is similar to MPEG: there are some changes that may
> > not be possible while streaming. So, such changes should, in thesis, be
> > blocked.
> >
> > On the other hand, from users perspective, it seems interesting for they to
> > start an application and, for example, change the JPEG quality while
> > streaming, to see what better fits his needs.
> >
> > Since JPEG is just a set of independent jpeg images, in thesis, it is
> > possible to change the encoding while streaming, without breaking for
> > userspace, assuming that the allocated buffers are large enough to support
> > such changes.
> >
> > So, IMO, we should try to allow such changes where possible, even by doing
> > something like this, at the loop that fills the video buffer:
> >
> > if (streaming && frame_is_complete && quality_changed) {
> > stop_streaming();
> > change_quality();
> > start_streaming();
> > }
>
> This can only be done if the allocated buffers are big enough. As the device
> reports the required buffer size the driver would have to query the device
> before deciding if it allows to change the quality during streaming. Given
> the
> usefulness of changing MJPEG compression quality during streaming, and given
> how most webcams seem to ignore the quality anyway, I don't think it would be
> worth it at the moment.
Ok.
> Should I submit a patch that implement VIDIOC_S_JPEGCOMP support in the UVC
> driver and imp
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Hi Mauro,
On Tuesday 09 June 2009 17:01:14 Mauro Carvalho Chehab wrote:
> Em Mon, 8 Jun 2009 16:15:57 +0200
>
> Laurent Pinchart escreveu:
> > On Monday 08 June 2009 03:52:12 Mauro Carvalho Chehab wrote:
> > > ERROR: do not use assignment in if condition
> > > #64: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:376:
> > > + if ((ret = uvc_probe_video(video, &probe)) < 0)
> > >
> > > ERROR: do not use assignment in if condition
> > > #80: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:872:
> > > + if ((ret = uvc_acquire_privileges(handle)) < 0)
> > >
> > > total: 2 errors, 0 warnings, 81 lines checked
> > >
> > > Please, one statement per line.
> >
> > Is there any way I can convince you to accept the
> >
> > if ((ret = function(...) < 0)
>
> I prefer if you fix it, although I won't nack your pull just due to that.
I'll fix the syntax.
> > syntax on the basis that it's used throughout the rest of the driver ?
> > :-)
> >
> > $ find . -type f -name \*.c -exec grep -l 'if (([a-zA-Z]\+ \?= \?[a-zA-
> > Z_]\+([a-zA-Z0-9 ,*]*)) \?[<>=]\+ \?[a-zA-Z0-9]\+)' {} \; | wc
> > 307 3079204
> >
> > I might have missed a few cases. Not that many occurrences.
>
> Try to count the proper coding style over the kernel source code. I bet
> they you'll find much more cases of:
>
> ret = function();
> if (ret < 0) /* and other similar tests, like ret == NULL, ret,
> !ret, etc
> */ foo;
>
> Since uvc is part of the kernel, in fact, it is uvcvideo that is adding
> more exceptions to the Kernel style.
Damn, I hoped you wouldn't notice that ;-)
I find the
if ((ret = function()) < 0)
more compact (which can't be debated) and as easy to read as the alternative,
if not easier (now this can be debated). As this point of view seems to be
minority I'll try to adjust my coding style accordingly. Of course I'll blame
you personally for any bug that it would introduce ;-)
> > > Also, I have a few comments, from API POV.
> > >
> > > It doesn't seem that those ioctls are properly implemented. There are
> > > some things there that sounded obfuscated for me, and it you aren't
> > > implementing. Probably because it is not clear enough.
> >
> > The UVC standard doesn't specify a way to add/remove specific JPEG
> > segments. As MJPEG has not COM, DRI and DHT segments, I've hardcoded
> > flags to DQT.
> >
> > > Also, we used to have a similar set of ioctls for MPEG, that were
> > > removed, in favor of the usage of extended controls, with a cleaner
> > > interface.
> > >
> > > That's said, instead of adding more support for this obfuscated API,
> > > maybe we could deprecate those in favor of some controls that could
> > > make more sense, and let vidio_ioctl2 provide backward compat for it by
> > > calling the proper controls, just to preserve binary compatibility with
> > > legacy applications.
> >
> > I have mixed feelings about this. On one hand I agree that
> > VIDIOC_S_JPEGCOMP is under-specified and should be either properly
> > document, or deprecate it in favor of a control. On the other hand, the
> > uvcvideo driver assumes that controls can all be read and written while
> > streaming is in progress. I suppose other drivers (probably non-MPEG
> > ones) were written with the same assumption in mind. Using a control to
> > set JPEG compression would mean that a proper infrastructure would need
> > to be put in place in those drivers to handle controls that influence
> > video streaming. I'd appreciate your opinion on that.
>
> I'm in favor of deprecate VIDIOC_S_JPEGCOMP.
>
> The issue you've described of changing the format while streaming exists,
> being it a control, or an ioctl. So, it deserves a separate discussion.
>
> -
>
> The MPEG drivers block trials of changing the MPEG format that can't be
> done while streaming. On the other hand, user preference controls, like
> bright, contrast, (auto)gain, exposure, etc can be done anytime. As most
> drivers just exposes such controls, they don't need a logic to block a
> control while streaming.
>
> The case of JPEG/MJPEG is similar to MPEG: there are some changes that may
> not be possible while streaming. So, such changes should, in thesis, be
> blocked.
>
> On the other hand, from users perspective, it seems interesting for they to
> start an application and, for example, change the JPEG quality while
> streaming, to see what better fits his needs.
>
> Since JPEG is just a set of independent jpeg images, in thesis, it is
> possible to change the encoding while streaming, without breaking for
> userspace, assuming that the allocated buffers are large enough to support
> such changes.
>
> So, IMO, we should try to allow such changes where possible, even by doing
> something like this, at the loop that fills the video buffer:
>
> if (streaming && frame_is_complete && quality_changed) {
> stop_streaming();
> change_quality();
> start_streaming();
> }
This can only be done if the allocated buffers are big enough. As the
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Em Mon, 8 Jun 2009 16:15:57 +0200
Laurent Pinchart escreveu:
> Hi Mauro,
>
> On Monday 08 June 2009 03:52:12 Mauro Carvalho Chehab wrote:
> > Em Thu, 4 Jun 2009 14:39:52 +0200
> >
> > Laurent Pinchart escreveu:
> > > Mauro,
> > >
> > > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/
> > >
> > > for the following 3 changesets:
> > >
> > > uvcvideo: Add generic control blacklist.
> > > uvcvideo: Don't accept to change the format when buffers are allocated.
> > > uvcvideo: Implement VIDIOC_[GS]_JPEGCOMP
> >
> > I have a few comments about the code of the last patch:
>
> I'll have to drop the tree, clone v4l-dvb and re-apply patches. git ? :-)
> (don't take this too seriously, I would love to switch to git, but it doesn't
> bother me *that* much for now as I don't have to rework patches too often).
With hg, there aren't any alternatives, AFAIK, since "hg push -f" won't delete
an object that were removed locally with hg strip (or changed via hg mq).
>
> > + jpeg->quality = video->streaming->ctrl.wCompQuality / 100;
> >
> > Wouldn't be better to round it to the closest value instead of just
> > truncating?
>
> Ok. I don't think it will really make a difference given how webcams handle
> JPEG compression quality, but better being correct anyway.
Ok.
>
> > + memcpy(&probe, &video->streaming->ctrl, sizeof probe);
> >
> > Please use sizeof(probe) instead.
>
> Oops sorry. I'm used to the 'sizeof variable' syntax. Seems checkpatch
> doesn't
> catch this. I'll fix that.
Checkpatch doesn't catch all problems (and, sometimes, a new version "forgets"
to report some errors that previous versions used to report).
>
> > ERROR: do not use assignment in if condition
> > #64: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:376:
> > + if ((ret = uvc_probe_video(video, &probe)) < 0)
> >
> > ERROR: do not use assignment in if condition
> > #80: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:872:
> > + if ((ret = uvc_acquire_privileges(handle)) < 0)
> >
> > total: 2 errors, 0 warnings, 81 lines checked
> >
> > Please, one statement per line.
>
> Is there any way I can convince you to accept the
>
> if ((ret = function(...) < 0)
I prefer if you fix it, although I won't nack your pull just due to that.
>
> syntax on the basis that it's used throughout the rest of the driver ? :-)
>
> $ find . -type f -name \*.c -exec grep -l 'if (([a-zA-Z]\+ \?= \?[a-zA-
> Z_]\+([a-zA-Z0-9 ,*]*)) \?[<>=]\+ \?[a-zA-Z0-9]\+)' {} \; | wc
> 307 3079204
>
> I might have missed a few cases. Not that many occurrences.
Try to count the proper coding style over the kernel source code. I bet
they you'll find much more cases of:
ret = function();
if (ret < 0)/* and other similar tests, like ret == NULL, ret,
!ret, etc */
foo;
Since uvc is part of the kernel, in fact, it is uvcvideo that is adding more
exceptions to the Kernel style.
> > Also, I have a few comments, from API POV.
> >
> > It doesn't seem that those ioctls are properly implemented. There are some
> > things there that sounded obfuscated for me, and it you aren't
> > implementing. Probably because it is not clear enough.
>
> The UVC standard doesn't specify a way to add/remove specific JPEG segments.
> As MJPEG has not COM, DRI and DHT segments, I've hardcoded flags to DQT.
>
> > Also, we used to have a similar set of ioctls for MPEG, that were removed,
> > in favor of the usage of extended controls, with a cleaner interface.
> >
> > That's said, instead of adding more support for this obfuscated API, maybe
> > we could deprecate those in favor of some controls that could make more
> > sense, and let vidio_ioctl2 provide backward compat for it by calling the
> > proper controls, just to preserve binary compatibility with legacy
> > applications.
>
> I have mixed feelings about this. On one hand I agree that VIDIOC_S_JPEGCOMP
> is under-specified and should be either properly document, or deprecate it in
> favor of a control. On the other hand, the uvcvideo driver assumes that
> controls can all be read and written while streaming is in progress. I
> suppose
> other drivers (probably non-MPEG ones) were written with the same assumption
> in mind. Using a control to set JPEG compression would mean that a proper
> infrastructure would need to be put in place in those drivers to handle
> controls that influence video streaming. I'd appreciate your opinion on that.
I'm in favor of deprecate VIDIOC_S_JPEGCOMP.
The issue you've described of changing the format while streaming exists, being
it a control, or an ioctl. So, it deserves a separate discussion.
-
The MPEG drivers block trials of changing the MPEG format that can't be done
while
streaming. On the other hand, user preference controls, like bright, contrast,
(auto)gain, exposure, etc can be done anytime. As most drivers just exposes
such controls, they don't need a logic to block a control while streaming.
The case of JPEG/MJ
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Hi Mauro,
On Monday 08 June 2009 03:52:12 Mauro Carvalho Chehab wrote:
> Em Thu, 4 Jun 2009 14:39:52 +0200
>
> Laurent Pinchart escreveu:
> > Mauro,
> >
> > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/
> >
> > for the following 3 changesets:
> >
> > uvcvideo: Add generic control blacklist.
> > uvcvideo: Don't accept to change the format when buffers are allocated.
> > uvcvideo: Implement VIDIOC_[GS]_JPEGCOMP
>
> I have a few comments about the code of the last patch:
I'll have to drop the tree, clone v4l-dvb and re-apply patches. git ? :-)
(don't take this too seriously, I would love to switch to git, but it doesn't
bother me *that* much for now as I don't have to rework patches too often).
> + jpeg->quality = video->streaming->ctrl.wCompQuality / 100;
>
> Wouldn't be better to round it to the closest value instead of just
> truncating?
Ok. I don't think it will really make a difference given how webcams handle
JPEG compression quality, but better being correct anyway.
> + memcpy(&probe, &video->streaming->ctrl, sizeof probe);
>
> Please use sizeof(probe) instead.
Oops sorry. I'm used to the 'sizeof variable' syntax. Seems checkpatch doesn't
catch this. I'll fix that.
> ERROR: do not use assignment in if condition
> #64: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:376:
> + if ((ret = uvc_probe_video(video, &probe)) < 0)
>
> ERROR: do not use assignment in if condition
> #80: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:872:
> + if ((ret = uvc_acquire_privileges(handle)) < 0)
>
> total: 2 errors, 0 warnings, 81 lines checked
>
> Please, one statement per line.
Is there any way I can convince you to accept the
if ((ret = function(...) < 0)
syntax on the basis that it's used throughout the rest of the driver ? :-)
$ find . -type f -name \*.c -exec grep -l 'if (([a-zA-Z]\+ \?= \?[a-zA-
Z_]\+([a-zA-Z0-9 ,*]*)) \?[<>=]\+ \?[a-zA-Z0-9]\+)' {} \; | wc
307 3079204
I might have missed a few cases. Not that many occurrences.
> Also, I have a few comments, from API POV.
>
> It doesn't seem that those ioctls are properly implemented. There are some
> things there that sounded obfuscated for me, and it you aren't
> implementing. Probably because it is not clear enough.
The UVC standard doesn't specify a way to add/remove specific JPEG segments.
As MJPEG has not COM, DRI and DHT segments, I've hardcoded flags to DQT.
> Also, we used to have a similar set of ioctls for MPEG, that were removed,
> in favor of the usage of extended controls, with a cleaner interface.
>
> That's said, instead of adding more support for this obfuscated API, maybe
> we could deprecate those in favor of some controls that could make more
> sense, and let vidio_ioctl2 provide backward compat for it by calling the
> proper controls, just to preserve binary compatibility with legacy
> applications.
I have mixed feelings about this. On one hand I agree that VIDIOC_S_JPEGCOMP
is under-specified and should be either properly document, or deprecate it in
favor of a control. On the other hand, the uvcvideo driver assumes that
controls can all be read and written while streaming is in progress. I suppose
other drivers (probably non-MPEG ones) were written with the same assumption
in mind. Using a control to set JPEG compression would mean that a proper
infrastructure would need to be put in place in those drivers to handle
controls that influence video streaming. I'd appreciate your opinion on that.
Cheers,
Laurent Pinchart
--
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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Em Thu, 4 Jun 2009 14:39:52 +0200 Laurent Pinchart escreveu: > Mauro, > > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ > > for the following 3 changesets: > > uvcvideo: Add generic control blacklist. > uvcvideo: Don't accept to change the format when buffers are allocated. > uvcvideo: Implement VIDIOC_[GS]_JPEGCOMP I have a few comments about the code of the last patch: + jpeg->quality = video->streaming->ctrl.wCompQuality / 100; Wouldn't be better to round it to the closest value instead of just truncating? + memcpy(&probe, &video->streaming->ctrl, sizeof probe); Please use sizeof(probe) instead. ERROR: do not use assignment in if condition #64: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:376: + if ((ret = uvc_probe_video(video, &probe)) < 0) ERROR: do not use assignment in if condition #80: FILE: linux/drivers/media/video/uvc/uvc_v4l2.c:872: + if ((ret = uvc_acquire_privileges(handle)) < 0) total: 2 errors, 0 warnings, 81 lines checked Please, one statement per line. - Also, I have a few comments, from API POV. It doesn't seem that those ioctls are properly implemented. There are some things there that sounded obfuscated for me, and it you aren't implementing. Probably because it is not clear enough. Also, we used to have a similar set of ioctls for MPEG, that were removed, in favor of the usage of extended controls, with a cleaner interface. That's said, instead of adding more support for this obfuscated API, maybe we could deprecate those in favor of some controls that could make more sense, and let vidio_ioctl2 provide backward compat for it by calling the proper controls, just to preserve binary compatibility with legacy applications. Cheers, 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 3 changesets: uvcvideo: Add generic control blacklist. uvcvideo: Don't accept to change the format when buffers are allocated. uvcvideo: Implement VIDIOC_[GS]_JPEGCOMP uvc_ctrl.c | 35 +++ uvc_driver.c |3 +-- uvc_queue.c | 14 ++ uvc_v4l2.c | 47 ++- uvc_video.c |6 -- uvcvideo.h |2 +- 6 files changed, 81 insertions(+), 26 deletions(-) Thanks, Laurent Pinchart -- 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 4 changesets: uvcvideo: Parse frame descriptors with non-continuous indexes. uvcvideo: Add missing whitespaces to multi-line format strings. uvcvideo: Start status polling on device open uvcvideo: Add Lenovo Thinkpad SL400 to device list comments uvc_driver.c | 43 --- uvc_status.c | 21 ++--- uvc_v4l2.c | 14 ++ uvc_video.c | 17 +++-- uvcvideo.h |3 +++ 5 files changed, 58 insertions(+), 40 deletions(-) Thanks, Laurent Pinchart -- 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 3 changesets: uvcvideo: Prevent invormation loss with removing implicit casting uvcvideo: fill reserved fields with zero of VIDIOC_QUERYMENU uvcvideo: fix uvc resume failed uvc_v4l2.c | 10 -- uvc_video.c |2 +- 2 files changed, 9 insertions(+), 3 deletions(-) Thanks, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
On Mon, 30 Mar 2009 23:16:09 +0200 Laurent Pinchart wrote: > Hi Mauro, > > On Monday 30 March 2009 05:49:36 Mauro Carvalho Chehab wrote: > > Hi Laurent, > > > > On Sun, 29 Mar 2009 14:45:07 +0200 > > > > Laurent Pinchart wrote: > > > uvcvideo: Add support for Syntek cameras found in JAOtech Smart Terminals > > > > Please prefer using "make commit" to commit your patches. It will run some > > sanity scripts that will help you to not forget something. > > Sorry. I wanted to commit only a subset of modified files and I used hg > commit > directly for that purpose. Ah, ok. Here, I have a hgeditor script for such cases (enclosed). You'll need to copy or symlink the v4l hg_prep_msg.pl as /usr/local/bin/hg_prep_msg.pl in order to use it, and properly setup the hgrc for using it to generate the hg commit msgs. > I'll be more careful next time. No problem. > > This time, you forgot the "From:" on the above patch. In order to avoid > > attributing a patch to the wrong author at -git, I need to manually fix the > > lack of the "From:" meta-tag. > > > > Since I saw only your SOB, I've credited this patch to you. Please let me > > know ASAP if my hint were wrong. > > Nothing wrong, thanks for fixing this. Good! Cheers, Mauro hgeditor Description: Binary data
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Hi Mauro, On Monday 30 March 2009 05:49:36 Mauro Carvalho Chehab wrote: > Hi Laurent, > > On Sun, 29 Mar 2009 14:45:07 +0200 > > Laurent Pinchart wrote: > > uvcvideo: Add support for Syntek cameras found in JAOtech Smart Terminals > > Please prefer using "make commit" to commit your patches. It will run some > sanity scripts that will help you to not forget something. Sorry. I wanted to commit only a subset of modified files and I used hg commit directly for that purpose. I'll be more careful next time. > This time, you forgot the "From:" on the above patch. In order to avoid > attributing a patch to the wrong author at -git, I need to manually fix the > lack of the "From:" meta-tag. > > Since I saw only your SOB, I've credited this patch to you. Please let me > know ASAP if my hint were wrong. Nothing wrong, thanks for fixing this. Cheers, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Hi Laurent, On Sun, 29 Mar 2009 14:45:07 +0200 Laurent Pinchart wrote: > uvcvideo: Add support for Syntek cameras found in JAOtech Smart Terminals Please prefer using "make commit" to commit your patches. It will run some sanity scripts that will help you to not forget something. This time, you forgot the "From:" on the above patch. In order to avoid attributing a patch to the wrong author at -git, I need to manually fix the lack of the "From:" meta-tag. Since I saw only your SOB, I've credited this patch to you. Please let me know ASAP if my hint were wrong. Cheers, 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 2 changesets: uvcvideo: Add support for Syntek cameras found in JAOtech Smart Terminals uvcvideo: Add zero fill for VIDIOC_ENUM_FMT uvc_driver.c |9 + uvc_v4l2.c |6 ++ 2 files changed, 15 insertions(+) Thanks, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Hi Mauro, On Tuesday 17 February 2009 03:54:48 Mauro Carvalho Chehab wrote: > On Mon, 16 Feb 2009 21:44:58 +0100 > > Laurent Pinchart wrote: > > Mauro, > > > > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ > > > > for the following 3 changesets: > > > > uvcvideo: Initialize streaming parameters with the probe control value > > Hmm... I suspect that this is not part of this changeset... > > --- a/linux/drivers/media/video/uvc/Kconfig Thu Jan 22 16:45:10 2009 > +0100 +++ b/linux/drivers/media/video/uvc/Kconfig Sat Feb 14 23:26:56 > 2009 +0100 @@ -9,7 +9,7 @@ config USB_VIDEO_CLASS_INPUT_EVDEV > config USB_VIDEO_CLASS_INPUT_EVDEV > bool "UVC input events device support" > default y > - depends on USB_VIDEO_CLASS && INPUT > + depends on USB_VIDEO_CLASS && (INPUT = y || INPUT = > USB_VIDEO_CLASS) ---help--- > This option makes USB Video Class devices register an input > device to report button events. Oops, my bad, sorry. Thanks for catching it. > Could you please re-generate your tree? Done. Please pull. (I'll be very happy when/if decide to switch to git, reworking patches would be much easier :-)). Cheers, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
On Mon, 16 Feb 2009 21:44:58 +0100 Laurent Pinchart wrote: > Mauro, > > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ > > for the following 3 changesets: > > uvcvideo: Initialize streaming parameters with the probe control value Hmm... I suspect that this is not part of this changeset... --- a/linux/drivers/media/video/uvc/Kconfig Thu Jan 22 16:45:10 2009 +0100 +++ b/linux/drivers/media/video/uvc/Kconfig Sat Feb 14 23:26:56 2009 +0100 @@ -9,7 +9,7 @@ config USB_VIDEO_CLASS_INPUT_EVDEV config USB_VIDEO_CLASS_INPUT_EVDEV bool "UVC input events device support" default y - depends on USB_VIDEO_CLASS && INPUT + depends on USB_VIDEO_CLASS && (INPUT = y || INPUT = USB_VIDEO_CLASS) ---help--- This option makes USB Video Class devices register an input device to report button events. Could you please re-generate your tree? > uvcvideo: Ignore empty bulk URBs > uvcvideo: Add quirk to override wrong bandwidth value for Vimicro devices > > Kconfig |2 +- > uvc_driver.c |9 + > uvc_video.c | 54 ++ > uvcvideo.h |1 + > 4 files changed, 57 insertions(+), 9 deletions(-) > > Thanks, > > Laurent Pinchart > > -- > 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 -- Cheers, 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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Mauro, Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/ for the following 3 changesets: uvcvideo: Initialize streaming parameters with the probe control value uvcvideo: Ignore empty bulk URBs uvcvideo: Add quirk to override wrong bandwidth value for Vimicro devices Kconfig |2 +- uvc_driver.c |9 + uvc_video.c | 54 ++ uvcvideo.h |1 + 4 files changed, 57 insertions(+), 9 deletions(-) Thanks, Laurent Pinchart -- 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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo
On Mon, 19 Jan 2009 10:38:45 +0100
Laurent Pinchart wrote:
> On Monday 19 January 2009, Mauro Carvalho Chehab wrote:
> > On Sun, 18 Jan 2009 21:49:13 +0100
> >
> > Laurent Pinchart wrote:
> > > Mauro,
> > >
> > > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/
> > >
> > > for the following 3 changesets:
> > >
> > > uvcvideo: replace strn{cpy,cat} with strl{cpy,cat}.
> >
> > Hmm... instead of this:
> >
> > + phys = kasprintf(GFP_KERNEL, "usb-%s-%s", udev->bus->bus_name,
> > +udev->devpath);
> >
> > You should use, instead:
> >
> > usb_make_path(udev, phys, sizeof(phys));
> >
> > This is easier to read and it should become a standard to fill bus_name on
> > usb drivers, since it produces a canonical name.
>
> input->name isn't a fixed-size buffer but a dynamically allocated one, so I
> can't use usb_make_path as-is. Reading the code, the phys buffer is currently
> leaked, so I'll have to fix it anyway.
>
> Switching to a fixed-size buffer is possible and 64 bytes seems to be a
> sensible value. Most USB input devices seem to set phys to usb_make_path()
> + "/input0". Is there an authoritative source of information regarding how
> the phys field should be formatted ?
No. I've seen some drivers adding /input0 as well. I guess we should discuss
this issue with linux event guys.
Cheers,
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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo
Mauro,
Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/
for the following 4 changesets:
uvcvideo: replace strn{cpy,cat} with strl{cpy,cat}.
uvcvideo: Add support for the Alcor Micro AU3820 chipset.
uvcvideo: Retry URB buffers allocation when the system is low on memory.
uvcvideo: Fix memory leak in input device handling
uvc_ctrl.c |2 -
uvc_driver.c | 27 +---
uvc_status.c | 16 ++-
uvc_v4l2.c | 12
uvc_video.c | 79 +++
uvcvideo.h |7 ++---
6 files changed, 74 insertions(+), 69 deletions(-)
Thanks,
Laurent Pinchart
--
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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo
On Monday 19 January 2009, Mauro Carvalho Chehab wrote:
> On Sun, 18 Jan 2009 21:49:13 +0100
>
> Laurent Pinchart wrote:
> > Mauro,
> >
> > Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/
> >
> > for the following 3 changesets:
> >
> > uvcvideo: replace strn{cpy,cat} with strl{cpy,cat}.
>
> Hmm... instead of this:
>
> + phys = kasprintf(GFP_KERNEL, "usb-%s-%s", udev->bus->bus_name,
> +udev->devpath);
>
> You should use, instead:
>
> usb_make_path(udev, phys, sizeof(phys));
>
> This is easier to read and it should become a standard to fill bus_name on
> usb drivers, since it produces a canonical name.
input->name isn't a fixed-size buffer but a dynamically allocated one, so I
can't use usb_make_path as-is. Reading the code, the phys buffer is currently
leaked, so I'll have to fix it anyway.
Switching to a fixed-size buffer is possible and 64 bytes seems to be a
sensible value. Most USB input devices seem to set phys to usb_make_path()
+ "/input0". Is there an authoritative source of information regarding how
the phys field should be formatted ?
Cheers,
Laurent Pinchart
--
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: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo
On Sun, 18 Jan 2009 21:49:13 +0100
Laurent Pinchart wrote:
> Mauro,
>
> Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/
>
> for the following 3 changesets:
>
> uvcvideo: replace strn{cpy,cat} with strl{cpy,cat}.
Hmm... instead of this:
+ phys = kasprintf(GFP_KERNEL, "usb-%s-%s", udev->bus->bus_name,
+udev->devpath);
You should use, instead:
usb_make_path(udev, phys, sizeof(phys));
This is easier to read and it should become a standard to fill bus_name on usb
drivers, since it produces a canonical name.
Cheers,
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
[PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo
Mauro,
Please pull from http://linuxtv.org/hg/~pinchartl/uvcvideo/
for the following 3 changesets:
uvcvideo: replace strn{cpy,cat} with strl{cpy,cat}.
uvcvideo: Add support for the Alcor Micro AU3820 chipset.
uvcvideo: Retry URB buffers allocation when the system is low on memory.
uvc_ctrl.c |2 -
uvc_driver.c | 27 +---
uvc_status.c |5 +--
uvc_v4l2.c | 12
uvc_video.c | 79 +++
uvcvideo.h |6 +---
6 files changed, 71 insertions(+), 60 deletions(-)
Thanks,
Laurent Pinchart
--
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
