movie2serial: image2data changes running very slowly

BuffaloFan32

I am running an LED array using a Teensy 3.1 and I am currently using movie2serial (a processing script) to tell the Teensy what to output. I changed movie2serial so that it reads a bunch of pixel locations from a text file and then grabs those locations from an image and outputs them to my array. My problem is that my edits are causing the program to run very slowly. The biggest changes I made were to image2data:

// image2data converts an image to OctoWS2811's raw data format.
// The number of vertical pixels in the image must be a multiple
// of 8.  The data array must be the proper size for the image.
void image2data(PImage image, byte[] data) {
  int offset = 3;
  int i, j, counter, mask;
  int pixel[] = new int[8];

  for (i = 0; i < strLines.length/8; i++) {             //strLines is a list of pixels to find
    for (j = 0; j < 8; j++) {
      pixel[j] = image.pixels[int(strLines[i * 8 + j])];
      pixel[j] = colorWiring(pixel[j]);
    }
    // convert 8 pixels to 24 bytes                   
      for (mask = 0x800000; mask != 0; mask >>= 1) {
        byte b = 0;
        for (j=0; j < 8; j++) {
          if ((pixel[j] & mask) != 0) b |= (1 << j);
        }
        data[offset++] = b;
      }
  }
}

I also pass the entire screenshot to image2data now (I am only running one Teensy at the moment). I cannot figure out why it is running so much slower now when it seems to be running the same number of iterations on each frame as it was in the original version.

The Teensy forum sent me here as this is more of a processing question. https://forum.pjrc.com/threads/33067-movie2serial-image2data-changes-running-very-slowly

BuffaloFan32

I guess the question boils down to: why is the function above so much slower than the original:

// image2data converts an image to OctoWS2811's raw data format.
 // The number of vertical pixels in the image must be a multiple
 // of 8. The data array must be the proper size for the image.
 void image2data(PImage image, byte[] data, boolean layout) {
   int offset = 3;
   int x, y, xbegin, xend, xinc, mask;
   int linesPerPin = image.height / 8;
   int pixel[] = new int[8];

   for (y = 0; y < linesPerPin; y++) {
     if ((y & 1) == (layout ? 0 : 1)) {
       // even numbered rows are left to right
       xbegin = 0;
       xend = image.width;
       xinc = 1;
     } else {
       // odd numbered rows are right to left
       xbegin = image.width - 1;
       xend = -1;
       xinc = -1;
     }
     for (x = xbegin; x != xend; x += xinc) {
       for (int i=0; i < 8; i++) {
         // fetch 8 pixels from the image, 1 for each pin
         pixel[i] = image.pixels[x + (y + linesPerPin * i) * image.width];
         pixel[i] = colorWiring(pixel[i]);
       }
       // convert 8 pixels to 24 bytes
       for (mask = 0x800000; mask != 0; mask >>= 1) {
         byte b = 0;
         for (int i=0; i < 8; i++) {
           if ((pixel[i] & mask) != 0) b |= (1 << i);
         }
         data[offset++] = b;
       }
     }
   } 
 }

BuffaloFan32

Anybody out there?

GoToLoop

... why is the function above so much slower than the original:

Just made a quick glance, but 1 thing I've noticed in the original is the use of cache for calculations.

For example, while the original caches: int linesPerPin = image.height / 8;
Yours repeats the following inside the loop: for (i = 0; i < strLines.length/8; i++) {

Is the expression strLines.length/8 necessary to be recalculated over & over? What about this 1?

for (int i = 0; i < strLines.length; i += 8) {
  for (int j = 0; j < 8; j++) {
    pixel[j] = image.pixels[int(strLines[i + j])];

We can also cache image.pixels[] in order to avoid the extra image reference indirection:

for (int p[] = image.pixels, bits[] = new int[8], i = 0; i < strLines.length; i += 8) {
  for (int j = 0; j < 8; ++j)  bits[j] = colorWiring(p[int(strLines[i + j])]);

Another important issue is strLines[]. Where does it come from?
Utility functions should strive to get everything they need to do their job from their own parameters.
So why not instead?: void image2data(PImage image, String[] strLines, byte... data) {

But even after that, the worst offender is still strLines[].
Why? B/c it's a String[]; but you actually want an int[]!
The conversion from String to int 1 by 1 here: int(strLines[i * 8 + j])
is killing off the whole performance for sure!

Rather than 1 by 1, what about all at once?: final int[] intLines = int(strLines);
Thus we get now: for (int j = 0; j < 8; ++j) bits[j] = colorWiring(p[intLines[i + j]]);

There are more optimizations to spot. But for now I'm gonna leave to you the task to re-post your code here after applying those tips. Good luck! O:-)

BuffaloFan32

@GoToLoop thank you! I will implement those changes and get back to you.

BuffaloFan32

@GoToLoop I made the edits to this function but the program does not seem to be running any faster. Here is my new function:

void image2data(PImage image, int[] intLines, byte... data) {
  int offset = 3;
  int j, mask;

  for (int p[] = image.pixels, bits[] = new int[8], i = 0; i < intLines.length; i += 8) {
   for (j = 0; j < 8; ++j)  bits[j] = colorWiring(p[intLines[i + j]]);

    // convert 8 pixels to 24 bytes                    this is it
      for (mask = 0x800000; mask != 0; mask >>= 1) {
        byte b = 0;
        for (j=0; j < 8; j++) {
          if ((bits[j] & mask) != 0) b |= (1 << j);
        }
        data[offset++] = b;
      }
  }
}

I thought all of you suggestions were pretty good so I was surprised not to notice any increase in speed.

koogs

I changed movie2serial so that it reads a bunch of pixel locations from a text file

Please tell me it's not doing this in draw()

BuffaloFan32

@koogs no, not in draw(). I think I figured out where the problem was. When I call image2data, I pass an empty array called ledData. ledData has a blank space for each byte created by image2data. Originally, the image was downsized by 8 before passing to image2data. Now, the image full image is being passed but we only take every 8th pixel. This means ledData is way to big. When I divide its size by 8, in combination with @GoToLoop's suggestions, it works a lot faster.