Re: [PATCH] for the file gspca/mr97310a.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





kilgota@xxxxxxxxxxxxxxxxxxxxxx wrote:


On Thu, 5 Mar 2009, Kyle Guinn wrote:

On Thursday 05 March 2009 20:34:27 kilgota@xxxxxxxxxxxxxxxxxxxxxx wrote:
Signed-off-by: Theodore Kilgore <kilgota@xxxxxxxxxx>
----------------------------------------------------------------------
--- mr97310a.c.old    2009-02-23 23:59:07.000000000 -0600
+++ mr97310a.c    2009-03-05 19:14:13.000000000 -0600
@@ -29,9 +29,7 @@ MODULE_LICENSE("GPL");
  /* specific webcam descriptor */
  struct sd {
      struct gspca_dev gspca_dev;  /* !! must be the first item */
-
      u8 sof_read;
-    u8 header_read;
  };

  /* V4L2 controls supported by the driver */
@@ -100,12 +98,9 @@ static int sd_init(struct gspca_dev *gsp

  static int sd_start(struct gspca_dev *gspca_dev)
  {
-    struct sd *sd = (struct sd *) gspca_dev;
      __u8 *data = gspca_dev->usb_buf;
      int err_code;

-    sd->sof_read = 0;
-

Good catch, I didn't realize this was kzalloc'd.

Hmmm. Perhaps I cut too much and _that_ should go back in. What if one stops the streaming and then restarts it? OTOH, one only risks losing one frame. OTTH, one might really want that frame. I will put it back.


Ack I was about to make a comment along the same lines, please put it back in.


      /* Note:  register descriptions guessed from MR97113A driver */

      data[0] = 0x01;
@@ -285,40 +280,29 @@ static void sd_pkt_scan(struct gspca_dev
              __u8 *data,                   /* isoc packet */
              int len)                      /* iso packet length */
  {
-    struct sd *sd = (struct sd *) gspca_dev;
      unsigned char *sof;

      sof = pac_find_sof(gspca_dev, data, len);
      if (sof) {
          int n;
-
+        int marker_len = sizeof pac_sof_marker;

The value doesn't change; there's no need to use a variable for this.

True. I was just working for legibility, and trying to substitute a shorter symbol for something which is long and cumbersome and screws up 80-character lines. If it is bad to do that, then I can take it right back out, of course.


          /* finish decoding current frame */
          n = sof - data;
-        if (n > sizeof pac_sof_marker)
-            n -= sizeof pac_sof_marker;
+        if (n > marker_len)
+            n -= marker_len;
          else
              n = 0;
          frame = gspca_frame_add(gspca_dev, LAST_PACKET, frame,
                      data, n);
-        sd->header_read = 0;
-        gspca_frame_add(gspca_dev, FIRST_PACKET, frame, NULL, 0);
-        len -= sof - data;
+        /* Start next frame. */
+        gspca_frame_add(gspca_dev, FIRST_PACKET, frame,
+            pac_sof_marker, marker_len);
+        len -= n;
+        len -= marker_len;
+        if (len < 0)
+            len = 0;

len -= sof - data; is a shorter way to find the remaining length.

Now, why did I try that and it did not work, but now it does? Weird. OK, you are right.


Actually, that is the right thing to do, as in cases where the frame only contained
the last part of a sof sequence, your code from the patch as send won't work.

<snip>

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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux