[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 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 laurent.pinch...@skynet.be 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/
Em Mon, 8 Jun 2009 16:15:57 +0200
Laurent Pinchart laurent.pinch...@skynet.be 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 laurent.pinch...@skynet.be 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/MJPEG is similar to MPEG: there are some changes that may not
be possible while streaming. So, such changes
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 laurent.pinch...@skynet.be 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 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
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Em Tue, 9 Jun 2009 17:29:58 +0200
Laurent Pinchart laurent.pinch...@skynet.be 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 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
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 laurent.pinch...@skynet.be 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 implement a JPEG compression
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo/
Em Tue, 9 Jun 2009 18:08:38 +0200 Hans Verkuil hverk...@xs4all.nl 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
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 hverk...@xs4all.nl 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 Thu, 4 Jun 2009 14:39:52 +0200 Laurent Pinchart laurent.pinch...@skynet.be 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 laurent.pinch...@skynet.be 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 laurent.pinch...@skynet.be 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
[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 Laurent, On Sun, 29 Mar 2009 14:45:07 +0200 Laurent Pinchart laurent.pinch...@skynet.be 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
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 laurent.pinch...@skynet.be 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 laurent.pinch...@skynet.be 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
Re: [PULL] http://linuxtv.org/hg/~pinchartl/uvcvideo
On Mon, 19 Jan 2009 10:38:45 +0100
Laurent Pinchart laurent.pinch...@skynet.be wrote:
On Monday 19 January 2009, Mauro Carvalho Chehab wrote:
On Sun, 18 Jan 2009 21:49:13 +0100
Laurent Pinchart laurent.pinch...@skynet.be 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
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 laurent.pinch...@skynet.be 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
