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. > > 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. > 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. Most likely. Regards, Hans -- 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
