On Thu, 5 Mar 2009, Hans de Goede wrote:
kilgota@xxxxxxxxxxxxxxxxxxxxxx wrote:
On Thu, 5 Mar 2009, Hans de Goede wrote:
kilgota@xxxxxxxxxxxxxxxxxxxxxx wrote:
On Thu, 5 Mar 2009, Hans de Goede wrote:
Kyle Guinn wrote:
On Wednesday 04 March 2009 22:34:13 kilgota@xxxxxxxxxxxxxxxxxxxxxx
wrote:
On Wed, 4 Mar 2009, Kyle Guinn wrote:
On Tuesday 03 March 2009 18:12:33 kilgota@xxxxxxxxxxxxxxxxxxxxxx
wrote:
contents of file mr97310a.patch follow, for gspca/mr97310a.c
--------------------------------------------------------
--- mr97310a.c.old 2009-02-23 23:59:07.000000000 -0600
+++ mr97310a.c.new 2009-03-03 17:19:06.000000000 -0600
@@ -302,21 +302,9 @@ static void sd_pkt_scan(struct gspca_dev
data, n);
sd->header_read = 0;
gspca_frame_add(gspca_dev, FIRST_PACKET, frame, NULL, 0);
- len -= sof - data;
- data = sof;
- }
- if (sd->header_read < 7) {
- int needed;
-
- /* skip the rest of the header */
- needed = 7 - sd->header_read;
- if (len <= needed) {
- sd->header_read += len;
- return;
- }
- data += needed;
- len -= needed;
- sd->header_read = 7;
+ /* keep the header, including sof marker, for coming frame
*/
+ len -= n;
+ data = sof - sizeof pac_sof_marker;;
}
gspca_frame_add(gspca_dev, INTER_PACKET, frame, data, len);
A few notes:
1. There is an extra semicolon on that last added line.
Oops. My bifocals.
2. sd->header_read no longer seems necessary.
This is very likely true.
3. If the SOF marker is split over two transfers then everything
falls
apart.
Are you sure about that?
Simple example: One transfer ends with FF FF 00 and the next begins
with FF 96 64. pac_find_sof() returns a pointer to 64, n is set to 0,
len stays the same, data now points at 3 bytes _before_ the transfer
buffer, and we will most likely get undefined behavior when trying to
copy the data out of the transfer buffer. Not only that, but the FF FF
00 portion of the SOF won't get copied to the frame buffer.
Good point, since we will always pass frames to userspace which start
with the
sof, maybe we should just only pass the variable part of the header to
userspace?
That sure feels like the easiest solution to me.
Regards,
Hans
Hans, that would not solve the problem. In fact, it appears to me that
this problem was already inherent in the driver code before I proposed
any patches at all.
Erm, if I understood correctly (haven't looked yet) the driver is working
with the sof detection from pac_common, which does work with a SOF split
over multiple frames.
That is not my impression of what the code in pac_common is doing. That
code, as I understand, is totally neutral about such things. What is does
is to parse some data and search for an SOF marker, and if it finds such a
thing then it declares the next byte after to be what it calls "sof".
Specifically, there is the function
static unsigned char *pac_find_sof(struct gspca_dev *gspca_dev,
unsigned char *m, int len)
and what it does is that it searches through unsigned char *m up to the
extent declared in int len, looking for an SOF marker. If it finds one,
then it returns the location of the next byte after the SOF marker has been
successfully read.
Check that function again, more carefully, if it fails, but finds a part of
the sof at the end of m it remembers how much of the sof it has already seen
and continues where it left of the next time it is called.
Well, here is the code in the function. I don't see. So can you explain?
Perhaps I am dense.
{
struct sd *sd = (struct sd *) gspca_dev;
int i;
/* Search for the SOF marker (fixed part) in the header */
for (i = 0; i < len; i++) {
if (m[i] == pac_sof_marker[sd->sof_read]) {
sd->sof_read++;
if (sd->sof_read == sizeof(pac_sof_marker)) {
PDEBUG(D_FRAM,
"SOF found, bytes to analyze: %u."
" Frame starts at byte #%u",
len, i + 1);
sd->sof_read = 0;
return m + i + 1;
}
} else {
sd->sof_read = 0;
}
}
return NULL;
}
--
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
