* [PATCH] xawtv: release buffer if it can't be displayed @ 2013-03-30 9:47 Hans Verkuil 2013-03-31 12:48 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Hans Verkuil @ 2013-03-30 9:47 UTC (permalink / raw) To: linux-media; +Cc: Hans de Goede This patch for xawtv3 releases the buffer if it can't be displayed because the resolution of the current format is larger than the size of the window. This will happen if the hardware cannot scale down to the initially quite small xawtv window. For example the au0828 driver has a fixed size of 720x480, so it will not display anything until the window is large enough for that resolution. The problem is that xawtv never releases (== calls QBUF) the buffer in that case, and it will of course run out of buffers and stall. The only way to kill it is to issue a 'kill -9' since ctrl-C won't work either. By releasing the buffer xawtv at least remains responsive and a picture will appear after resizing the window. Ideally of course xawtv should resize itself to the minimum supported resolution, but that's left as an exercise for the reader... Hans, the xawtv issues I reported off-list are all caused by this bug and by by the scaling bug introduced recently in em28xx. They had nothing to do with the alsa streaming, that was a red herring. Regards, Hans diff --git a/x11/x11.c b/x11/x11.c index 5324521..b203d84 100644 --- a/x11/x11.c +++ b/x11/x11.c @@ -152,8 +152,10 @@ int video_gd_blitframe(struct video_handle *h, struct ng_video_buf *buf) { if (buf->fmt.width > cur_tv_width || - buf->fmt.height > cur_tv_height) + buf->fmt.height > cur_tv_height) { + ng_release_video_buf(buf); return -1; + } if (cur_filter) buf = video_gd_filter(h,buf); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xawtv: release buffer if it can't be displayed 2013-03-30 9:47 [PATCH] xawtv: release buffer if it can't be displayed Hans Verkuil @ 2013-03-31 12:48 ` Hans de Goede 2013-04-01 10:19 ` Hans Verkuil 0 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2013-03-31 12:48 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media Hi, On 03/30/2013 10:47 AM, Hans Verkuil wrote: > This patch for xawtv3 releases the buffer if it can't be displayed because > the resolution of the current format is larger than the size of the window. > > This will happen if the hardware cannot scale down to the initially quite > small xawtv window. For example the au0828 driver has a fixed size of 720x480, > so it will not display anything until the window is large enough for that > resolution. > > The problem is that xawtv never releases (== calls QBUF) the buffer in that > case, and it will of course run out of buffers and stall. The only way to > kill it is to issue a 'kill -9' since ctrl-C won't work either. > > By releasing the buffer xawtv at least remains responsive and a picture will > appear after resizing the window. Ideally of course xawtv should resize itself > to the minimum supported resolution, but that's left as an exercise for the > reader... > > Hans, the xawtv issues I reported off-list are all caused by this bug and by > by the scaling bug introduced recently in em28xx. They had nothing to do with > the alsa streaming, that was a red herring. Thanks for the debugging and for the patch. I've pushed the patch to xawtv3.git. I've a 2 patch follow up set which should fix the issue with being able to resize the window to a too small size. I'll send this patch set right after this mail, can you test it with the au0828 please? Thanks, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xawtv: release buffer if it can't be displayed 2013-03-31 12:48 ` Hans de Goede @ 2013-04-01 10:19 ` Hans Verkuil 2013-04-01 14:23 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Hans Verkuil @ 2013-04-01 10:19 UTC (permalink / raw) To: Hans de Goede; +Cc: linux-media Hi Hans, On Sun March 31 2013 14:48:01 Hans de Goede wrote: > Hi, > > On 03/30/2013 10:47 AM, Hans Verkuil wrote: > > This patch for xawtv3 releases the buffer if it can't be displayed because > > the resolution of the current format is larger than the size of the window. > > > > This will happen if the hardware cannot scale down to the initially quite > > small xawtv window. For example the au0828 driver has a fixed size of 720x480, > > so it will not display anything until the window is large enough for that > > resolution. > > > > The problem is that xawtv never releases (== calls QBUF) the buffer in that > > case, and it will of course run out of buffers and stall. The only way to > > kill it is to issue a 'kill -9' since ctrl-C won't work either. > > > > By releasing the buffer xawtv at least remains responsive and a picture will > > appear after resizing the window. Ideally of course xawtv should resize itself > > to the minimum supported resolution, but that's left as an exercise for the > > reader... > > > > Hans, the xawtv issues I reported off-list are all caused by this bug and by > > by the scaling bug introduced recently in em28xx. They had nothing to do with > > the alsa streaming, that was a red herring. > > Thanks for the debugging and for the patch. I've pushed the patch to > xawtv3.git. I've a 2 patch follow up set which should fix the issue with being > able to resize the window to a too small size. > > I'll send this patch set right after this mail, can you test it with the au0828 > please? I've tested it and it is not yet working. I've tracked it down to video_gd_configure where it calls ng_ratio_fixup() which changes the cur_tv_width of 736 to 640. The height remains the same at 480. Regards, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xawtv: release buffer if it can't be displayed 2013-04-01 10:19 ` Hans Verkuil @ 2013-04-01 14:23 ` Hans de Goede 2013-04-01 14:39 ` Hans Verkuil 0 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2013-04-01 14:23 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media [-- Attachment #1: Type: text/plain, Size: 1972 bytes --] Hi, On 04/01/2013 12:19 PM, Hans Verkuil wrote: > Hi Hans, > > On Sun March 31 2013 14:48:01 Hans de Goede wrote: >> Hi, >> >> On 03/30/2013 10:47 AM, Hans Verkuil wrote: >>> This patch for xawtv3 releases the buffer if it can't be displayed because >>> the resolution of the current format is larger than the size of the window. >>> >>> This will happen if the hardware cannot scale down to the initially quite >>> small xawtv window. For example the au0828 driver has a fixed size of 720x480, >>> so it will not display anything until the window is large enough for that >>> resolution. >>> >>> The problem is that xawtv never releases (== calls QBUF) the buffer in that >>> case, and it will of course run out of buffers and stall. The only way to >>> kill it is to issue a 'kill -9' since ctrl-C won't work either. >>> >>> By releasing the buffer xawtv at least remains responsive and a picture will >>> appear after resizing the window. Ideally of course xawtv should resize itself >>> to the minimum supported resolution, but that's left as an exercise for the >>> reader... >>> >>> Hans, the xawtv issues I reported off-list are all caused by this bug and by >>> by the scaling bug introduced recently in em28xx. They had nothing to do with >>> the alsa streaming, that was a red herring. >> >> Thanks for the debugging and for the patch. I've pushed the patch to >> xawtv3.git. I've a 2 patch follow up set which should fix the issue with being >> able to resize the window to a too small size. >> >> I'll send this patch set right after this mail, can you test it with the au0828 >> please? > > I've tested it and it is not yet working. I've tracked it down to video_gd_configure > where it calls ng_ratio_fixup() which changes the cur_tv_width of 736 to 640. The > height remains the same at 480. Thanks for testing and for figuring out where the problem lies. I've attached a second version of the second patch, can you give that a try please? Thanks, Hans [-- Attachment #2: 0001-xawtv-Limit-minimum-window-size-to-minimum-capture-r.patch --] [-- Type: text/x-patch, Size: 1536 bytes --] >From 1628ca8cddcd213abd16c20ad847072e6446976c Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@redhat.com> Date: Sun, 31 Mar 2013 14:41:17 +0200 Subject: [PATCH] xawtv: Limit minimum window size to minimum capture resolution Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- x11/xawtv.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/x11/xawtv.c b/x11/xawtv.c index bade35a..37afcb6 100644 --- a/x11/xawtv.c +++ b/x11/xawtv.c @@ -1636,7 +1636,7 @@ create_launchwin(void) int main(int argc, char *argv[]) { - int i; + int i, min_width, min_height; unsigned long freq; hello_world("xawtv"); @@ -1784,11 +1784,18 @@ main(int argc, char *argv[]) XSetWMProtocols(XtDisplay(app_shell), XtWindow(app_shell), &WM_DELETE_WINDOW, 1); + drv->get_min_size(h_drv, &min_width, &min_height); + ng_ratio_fixup2(&min_width, &min_height, NULL, NULL, + ng_ratio_x, ng_ratio_y, True); + min_width = ((min_width + (WIDTH_INC - 1)) / WIDTH_INC) * WIDTH_INC; + min_height = ((min_height + (HEIGHT_INC - 1)) / HEIGHT_INC) * HEIGHT_INC; + if (debug) + fprintf(stderr,"main: window min size %dx%d\n", min_width, min_height); XtVaSetValues(app_shell, XtNwidthInc, WIDTH_INC, XtNheightInc, HEIGHT_INC, - XtNminWidth, WIDTH_INC, - XtNminHeight, HEIGHT_INC, + XtNminWidth, min_width, + XtNminHeight, min_height, NULL); if (f_drv & CAN_TUNE) XtVaSetValues(chan_shell, -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xawtv: release buffer if it can't be displayed 2013-04-01 14:23 ` Hans de Goede @ 2013-04-01 14:39 ` Hans Verkuil 2013-04-02 9:02 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Hans Verkuil @ 2013-04-01 14:39 UTC (permalink / raw) To: Hans de Goede; +Cc: linux-media On Mon April 1 2013 16:23:51 Hans de Goede wrote: > Hi, > > On 04/01/2013 12:19 PM, Hans Verkuil wrote: > > Hi Hans, > > > > On Sun March 31 2013 14:48:01 Hans de Goede wrote: > >> Hi, > >> > >> On 03/30/2013 10:47 AM, Hans Verkuil wrote: > >>> This patch for xawtv3 releases the buffer if it can't be displayed because > >>> the resolution of the current format is larger than the size of the window. > >>> > >>> This will happen if the hardware cannot scale down to the initially quite > >>> small xawtv window. For example the au0828 driver has a fixed size of 720x480, > >>> so it will not display anything until the window is large enough for that > >>> resolution. > >>> > >>> The problem is that xawtv never releases (== calls QBUF) the buffer in that > >>> case, and it will of course run out of buffers and stall. The only way to > >>> kill it is to issue a 'kill -9' since ctrl-C won't work either. > >>> > >>> By releasing the buffer xawtv at least remains responsive and a picture will > >>> appear after resizing the window. Ideally of course xawtv should resize itself > >>> to the minimum supported resolution, but that's left as an exercise for the > >>> reader... > >>> > >>> Hans, the xawtv issues I reported off-list are all caused by this bug and by > >>> by the scaling bug introduced recently in em28xx. They had nothing to do with > >>> the alsa streaming, that was a red herring. > >> > >> Thanks for the debugging and for the patch. I've pushed the patch to > >> xawtv3.git. I've a 2 patch follow up set which should fix the issue with being > >> able to resize the window to a too small size. > >> > >> I'll send this patch set right after this mail, can you test it with the au0828 > >> please? > > > > I've tested it and it is not yet working. I've tracked it down to video_gd_configure > > where it calls ng_ratio_fixup() which changes the cur_tv_width of 736 to 640. The > > height remains the same at 480. > > Thanks for testing and for figuring out where the problem lies. I've attached a > second version of the second patch, can you give that a try please? This is now working for au0828, but now vivi is broken... That worked fine with your previous patch. I'm getting: $ xawtv This is xawtv-3.102, running on Linux/x86_64 (3.9.0-rc1-tschai) ioctl: VIDIOC_QUERYMENU(id=134217731;index=2;name="Menu Item 1";reserved=0): Invalid argument vid-open-auto: using grabber/webcam device /dev/video0 libv4l2: error setting pixformat: Device or resource busy ioctl: VIDIOC_S_FMT(type=VIDEO_CAPTURE;fmt.pix.width=384;fmt.pix.height=288;fmt.pix.pixelformat=0x34524742 [BGR4];fmt.pix.field=INTERLACED;fmt.pix.bytesperline=1536;fmt.pix.sizeimage=442368;fmt.pix.colorspace=SRGB;fmt.pix.priv=0): Device or resource busy Note that the QUERYMENU error is harmless, although it would be nice if xawtv would understand menu controls with 'holes' in the menu list. The 'Device or resource busy' errors are new and I didn't have them in your previous version. Regards, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xawtv: release buffer if it can't be displayed 2013-04-01 14:39 ` Hans Verkuil @ 2013-04-02 9:02 ` Hans de Goede 0 siblings, 0 replies; 6+ messages in thread From: Hans de Goede @ 2013-04-02 9:02 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media Hi, On 04/01/2013 04:39 PM, Hans Verkuil wrote: > On Mon April 1 2013 16:23:51 Hans de Goede wrote: >> Hi, >> >> On 04/01/2013 12:19 PM, Hans Verkuil wrote: >>> Hi Hans, >>> >>> On Sun March 31 2013 14:48:01 Hans de Goede wrote: >>>> Hi, >>>> >>>> On 03/30/2013 10:47 AM, Hans Verkuil wrote: >>>>> This patch for xawtv3 releases the buffer if it can't be displayed because >>>>> the resolution of the current format is larger than the size of the window. >>>>> >>>>> This will happen if the hardware cannot scale down to the initially quite >>>>> small xawtv window. For example the au0828 driver has a fixed size of 720x480, >>>>> so it will not display anything until the window is large enough for that >>>>> resolution. >>>>> >>>>> The problem is that xawtv never releases (== calls QBUF) the buffer in that >>>>> case, and it will of course run out of buffers and stall. The only way to >>>>> kill it is to issue a 'kill -9' since ctrl-C won't work either. >>>>> >>>>> By releasing the buffer xawtv at least remains responsive and a picture will >>>>> appear after resizing the window. Ideally of course xawtv should resize itself >>>>> to the minimum supported resolution, but that's left as an exercise for the >>>>> reader... >>>>> >>>>> Hans, the xawtv issues I reported off-list are all caused by this bug and by >>>>> by the scaling bug introduced recently in em28xx. They had nothing to do with >>>>> the alsa streaming, that was a red herring. >>>> >>>> Thanks for the debugging and for the patch. I've pushed the patch to >>>> xawtv3.git. I've a 2 patch follow up set which should fix the issue with being >>>> able to resize the window to a too small size. >>>> >>>> I'll send this patch set right after this mail, can you test it with the au0828 >>>> please? >>> >>> I've tested it and it is not yet working. I've tracked it down to video_gd_configure >>> where it calls ng_ratio_fixup() which changes the cur_tv_width of 736 to 640. The >>> height remains the same at 480. >> >> Thanks for testing and for figuring out where the problem lies. I've attached a >> second version of the second patch, can you give that a try please? > > This is now working for au0828, but now vivi is broken... That worked fine with your > previous patch. > > I'm getting: > > $ xawtv > This is xawtv-3.102, running on Linux/x86_64 (3.9.0-rc1-tschai) > ioctl: VIDIOC_QUERYMENU(id=134217731;index=2;name="Menu Item 1";reserved=0): Invalid argument > vid-open-auto: using grabber/webcam device /dev/video0 > libv4l2: error setting pixformat: Device or resource busy > ioctl: VIDIOC_S_FMT(type=VIDEO_CAPTURE;fmt.pix.width=384;fmt.pix.height=288;fmt.pix.pixelformat=0x34524742 [BGR4];fmt.pix.field=INTERLACED;fmt.pix.bytesperline=1536;fmt.pix.sizeimage=442368;fmt.pix.colorspace=SRGB;fmt.pix.priv=0): Device or resource busy > > Note that the QUERYMENU error is harmless, although it would be nice if xawtv > would understand menu controls with 'holes' in the menu list. > > The 'Device or resource busy' errors are new and I didn't have them in your > previous version. After much debugging I feel it is safe to say, that these errors are not new, and not caused by my patch. But still a good catch. They are caused by a pre-existing timing dependent bug. I can trigger the problem both with / without my patch by simply starting xawtv with the vivi driver a number of times, and 1 in every 2-5 times it triggers. I've also debugged and fixed this, see the commit message for details: http://git.linuxtv.org/xawtv3.git/commit/f0d84401dfa392ad86d11a76dda1f722269f3eaa I'm going to work on fixing the snapshot function with videobuf2 based drivers next, and when that is fixed I'll do a new xawtv3 release, unless someone yells NOOO before that time. Regards, Hans ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-02 8:59 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-30 9:47 [PATCH] xawtv: release buffer if it can't be displayed Hans Verkuil 2013-03-31 12:48 ` Hans de Goede 2013-04-01 10:19 ` Hans Verkuil 2013-04-01 14:23 ` Hans de Goede 2013-04-01 14:39 ` Hans Verkuil 2013-04-02 9:02 ` Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).