On 12/12/2014 06:14 AM, Mauro Carvalho Chehab wrote: > Em Fri, 12 Dec 2014 13:55:14 +0100 > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > >> On 12/12/2014 01:49 PM, Mauro Carvalho Chehab wrote: >>> Em Fri, 12 Dec 2014 11:16:01 +0100 >>> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: >>> >>>> Hi Shuah, >>>> >>>> This is the video.c review with your patch applied. >>>> >>>>> /* >>>>> * Auvitek AU0828 USB Bridge (Analog video support) >>>>> * >>>>> * Copyright (C) 2009 Devin Heitmueller <dheitmueller@xxxxxxxxxxx> >>>>> * Copyright (C) 2005-2008 Auvitek International, Ltd. >>>>> * >>>>> * This program is free software; you can redistribute it and/or >>>>> * modify it under the terms of the GNU General Public License >>>>> * As published by the Free Software Foundation; either version 2 >>>>> * of the License, or (at your option) any later version. >>>>> * >>>>> * This program is distributed in the hope that it will be useful, >>>>> * but WITHOUT ANY WARRANTY; without even the implied warranty of >>>>> * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>> * GNU General Public License for more details. >>>>> * >>>>> * You should have received a copy of the GNU General Public License >>>>> * along with this program; if not, write to the Free Software >>>>> * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >>>>> * 02110-1301, USA. >>>>> */ >>>>> >>>>> /* Developer Notes: >>>>> * >>>>> * VBI support is not yet working >>>> >>>> I'll see if I can get this to work quickly. If not, then we should >>>> probably just strip the VBI support from this driver. It's pointless to >>>> have non-functioning VBI support. >>> >>> This is a left-over. VBI support works on this driver. I tested. >> >> Oh wait, now I get it. You are only capturing line 21, not the whole vbi area. >> That's why vbi_height = 1. Never mind then. Although that comment should indeed >> be removed. Want me to remove the comment with this work or as a separate patch?? >> >>> >>> Probably, the patches that added VBI support forgot to remove the >>> above notice. >>> >>>>> /* This function ensures that video frames continue to be delivered even if >>>>> the ITU-656 input isn't receiving any data (thereby preventing applications >>>>> such as tvtime from hanging) */ >>>> >>>> Why would tvtime be hanging? Make a separate patch that just removes all this >>>> timeout nonsense. If there are no frames, then tvtime (and any other app) should >>>> just wait for frames to arrive. And ctrl-C should always be able to break the app >>>> (or they can timeout themselves). >>>> >>>> It's not the driver's responsibility to do this and it only makes the code overly >>>> complex. >>> >>> Well, we should not cause regressions on userspace. If removing this >>> check will cause tvtime to hang, we should keep it. >> >> Obviously if it hangs (i.e. tvtime can't be killed anymore) it is a bug in the driver. >> But the driver shouldn't start generating bogus frames just because no new frames are >> arriving, that's just nuts. > > If I remember the bug well, what used to happen is that tvtime would wait > for a certain amount of time for a frame. If nothing arrives, it stops > capturing. > > The net effect is that tvtime shows no picture. This used to be so bad > that tvtime didn't work with vivi at all. > > The bug used also to manifest there if lots of frames got dropped > when, for example, changing from one channel to another. > > Btw, on a quick look, I'm not seeing any patch at tvtime since we took > it over that would be fixing it. So, it was either a VB bug or the > bug is still there. > >> >>> Btw, the same kind of test used to be at vivi and other drivers. >>> I think we removed it there some time ago, so maybe either it was a >>> VB1 bug or this got fixed at tvtime. >> I take it that we decided to keep the timeout handling for now. thanks, -- Shuah -- Shuah Khan Sr. Linux Kernel Developer Samsung Open Source Group Samsung Research America (Silicon Valley) shuahkh@xxxxxxxxxxxxxxx | (970) 217-8978 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
