From patchwork Mon Sep 14 15:17:57 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Blanford X-Patchwork-Id: 47321 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n8EFI23g006219 for ; Mon, 14 Sep 2009 15:18:02 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751066AbZINPR6 (ORCPT ); Mon, 14 Sep 2009 11:17:58 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751250AbZINPR5 (ORCPT ); Mon, 14 Sep 2009 11:17:57 -0400 Received: from mail-yw0-f174.google.com ([209.85.211.174]:38501 "EHLO mail-yw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751063AbZINPR5 (ORCPT ); Mon, 14 Sep 2009 11:17:57 -0400 Received: by ywh4 with SMTP id 4so4348777ywh.5 for ; Mon, 14 Sep 2009 08:18:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:subject :message-id:x-mailer:mime-version:content-type :content-transfer-encoding; bh=Pwf2UoLfC/f4Mg0EJzOHVLZiaG6CXWeKKWMI7L0yrUU=; b=dglGfmmz0mW7kfF4czqRNP29dEfKWDr5z39hWJ7RXrqMUglpLPfY1hpvz6iAcos9pH Tx2tm9N2yansnTPfzJG05O81kU35oWODEMP9dJNsFtWefZ6LLE29PdVVJjXY0+fX43dv ej1PPFTJ6/vkblIVuorQO+X3OVUibJPoTJ0iU= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:subject:message-id:x-mailer:mime-version:content-type :content-transfer-encoding; b=Fc9SJlnTuSU+2NJH0Hx9X7xZMe1k8Kwsp9hjQ9i9QFWhYmgUEfW9Xzab0G+KQdJ/QA 8I9nh9bhHNCYLoDnJ0/mzewZyvfVYJZETpjSvsM9kyEfkaag8SY83q6J24qXdIN2ALtT obAXVh7uDm08vo/HP38iWcMzGsnOt2HfR5pc8= Received: by 10.90.217.11 with SMTP id p11mr3912765agg.82.1252941479762; Mon, 14 Sep 2009 08:17:59 -0700 (PDT) Received: from blackbart.localnet.prv (adsl-99-56-122-183.dsl.klmzmi.sbcglobal.net [99.56.122.183]) by mx.google.com with ESMTPS id 8sm2967469agd.37.2009.09.14.08.17.58 (version=TLSv1/SSLv3 cipher=RC4-MD5); Mon, 14 Sep 2009 08:17:59 -0700 (PDT) Date: Mon, 14 Sep 2009 11:17:57 -0400 From: James Blanford To: linux-media@vger.kernel.org, moinejf@free.fr Subject: Race in gspca main or missing lock in stv06xx subdriver? Message-ID: <20090914111757.543c7e77@blackbart.localnet.prv> X-Mailer: Claws Mail 3.5.0 (GTK+ 2.16.5; i486-pc-linux-gnu) Mime-Version: 1.0 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Howdy folks, I have my old quickcam express webcam working, with HDCS1000 sensor, 046d:840. It's clearly throwing away every other frame. What seems to be happening is, while the last packet of the previous frame is being analyzed by the subdriver, the first packet of the next frame is assigned to the current frame buffer. By the time that packet is analysed and sent back to the main driver, it's frame buffer has been completely filled and marked as "DONE." The entire frame is then marked for "DISCARD." This does _not_ happen with all cams using this subdriver. Here's a little patch, supplied only to help illustrate the problem, that allows for the full, expected frame rate of the webcam. What it does is wait until the very last moment to assign a frame buffer to any packet, but the last. I also threw in a few printks so I can see where failure takes place without wading through a swamp of debug output. It works, at least until there is any disruption to the stream, such as an exposure change, and then something gets out of sync and it starts throwing out every other frame again. It shows that the driver framework and USB bus is capable of handling the full frame rate. I'll keep looking for an actual solution, but there is a lot I don't understand. Any suggestions or ideas would be appreciated. Several questions come to mind. Why bother assigning a frame buffer with get_i_frame, before it's needed? What purpose has frame_wait, when it's not called until the frame is completed and the buffer is marked as "DONE." Why are there five, fr_i fr_q fr_o fr_queue index , buffer indexing counters? I'm sure I don't understand all the different tasks this driver has to handle and all the different hardware it has to deal with. But I would be surprised if my cam is the only one this is happening with. Thanks, - Jim --- gspca/gspca.c.orig 2009-09-11 22:43:54.000000000 -0400 +++ gspca/gspca.c 2009-09-11 23:04:34.000000000 -0400 @@ -114,8 +114,10 @@ struct gspca_frame *gspca_get_i_frame(st i = gspca_dev->fr_queue[i]; frame = &gspca_dev->frame[i]; if ((frame->v4l2_buf.flags & BUF_ALL_FLAGS) - != V4L2_BUF_FLAG_QUEUED) - return NULL; + != V4L2_BUF_FLAG_QUEUED){ + printk(KERN_DEBUG "Shout NULL NULL NULL\n"); + return frame; + } return frame; } EXPORT_SYMBOL(gspca_get_i_frame); @@ -146,6 +148,7 @@ static void fill_frame(struct gspca_dev /* check the availability of the frame buffer */ frame = gspca_get_i_frame(gspca_dev); if (!frame) { + printk(KERN_DEBUG "get_i_frame fails\n"); gspca_dev->last_packet_type = DISCARD_PACKET; break; } @@ -268,9 +271,12 @@ struct gspca_frame *gspca_frame_add(stru /* when start of a new frame, if the current frame buffer * is not queued, discard the whole frame */ if (packet_type == FIRST_PACKET) { + i = gspca_dev->fr_i; + frame = &gspca_dev->frame[i]; if ((frame->v4l2_buf.flags & BUF_ALL_FLAGS) != V4L2_BUF_FLAG_QUEUED) { gspca_dev->last_packet_type = DISCARD_PACKET; + printk(KERN_DEBUG "Frame marked for discard\n"); return frame; } frame->data_end = frame->data; @@ -285,8 +291,11 @@ struct gspca_frame *gspca_frame_add(stru /* append the packet to the frame buffer */ if (len > 0) { + i = gspca_dev->fr_i; + frame = &gspca_dev->frame[i]; if (frame->data_end - frame->data + len > frame->v4l2_buf.length) { + printk(KERN_DEBUG "Frame overflow\n"); PDEBUG(D_ERR|D_PACK, "frame overflow %zd > %d", frame->data_end - frame->data + len, frame->v4l2_buf.length);