1
\$\begingroup\$

To convert JPEG file, C function, bgraToGrayscale is called from Swift.

Is there a better way to

1.allocate memory and

2.access pointers

than the ways they have been done in code below?

And maybe things that have been completely missed, like freeing memory?

converter.swift

func BGRAToGrayscale(srcImg: CGImage) -> CGImage {
    let BytesPerPixel = 4

    guard let srcProv = srcImg.dataProvider else { fatalError("dataProvider is nil") }
    guard let srcProvData = srcProv.data else { fatalError("dataProvider.data is nil") }

    let srcPtr = CFDataGetBytePtr(srcProvData)
    let srcSize = srcImg.width * srcImg.height * BytesPerPixel

    var destPtr = [CUnsignedChar](repeating: 0, count: srcSize)
    bgraToGrayscale(&destPtr, srcPtr, Int32(srcImg.width), Int32(srcImg.height))

    let destCgContext = CGContext(data: &destPtr,
                                  width: srcImg.width, height: srcImg.height,
                                  bitsPerComponent: srcImg.bitsPerComponent,
                                  bytesPerRow: srcImg.bytesPerRow,
                                  space: srcImg.colorSpace!,
                                  bitmapInfo: srcImg.bitmapInfo.rawValue)

    guard let destImg = destCgContext?.makeImage() else {
      fatalError("Destination Image is nil")
    }

    return destImg
}

converter.c (slow way of converting to grayscale, will write assembly code for this, just for the sake of learning :-) )

void bgraToGrayscale( unsigned char * __restrict dest, const unsigned char* src, int width, int height ) {
  int BYTES_PER_PIXEL = 4;
  int numPixels = width * height;

  for( int idx=0; idx < numPixels; idx++ ) {

    float weight = ((float)src[0]) * 0.11 +
                  ((float)src[1]) * 0.59 +
                  ((float)src[2]) * 0.3;

    dest[0] = (int)weight;
    dest[1] = (int)weight;
    dest[2] = (int)weight;
    dest[3] = src[3];
    dest += BYTES_PER_PIXEL;
    src += BYTES_PER_PIXEL;
  }
}
\$\endgroup\$
  • \$\begingroup\$ You avoided doing any memory allocation in bgraToGrayScale(), so there's nothing to worry about. Unless you want this function to allocate memory for the converted image and return a pointer to that new image? \$\endgroup\$ – G. Sliepen Dec 21 '19 at 21:13
  • 2
    \$\begingroup\$ Before writing assembly code by hand, check what assembly code the compiler generates and then decide if you can do better. These days they are pretty good at vectorizing code like this. Of course it's fine for learning :) \$\endgroup\$ – G. Sliepen Dec 21 '19 at 21:15
  • \$\begingroup\$ (Beware that besides coding and the points you mentioned explicitly, there's floating point arithmetic. I'd drop an assert(int(255f*.11+255f*.59+255f*.3) < 256). (hm, ==255?)) \$\endgroup\$ – greybeard Dec 22 '19 at 0:33
  • \$\begingroup\$ Great idea to inspect generated assembly code. thanks @G.Sliepen \$\endgroup\$ – Don Ch Dec 22 '19 at 4:27
  • \$\begingroup\$ noted @greybeard \$\endgroup\$ – Don Ch Dec 22 '19 at 4:29
1
\$\begingroup\$
 // Not following Swift naming conventions, should be bytesPerPixel
let BytesPerPixel = 4

// Better to make function throwable and throw specific error.
guard let srcProv = srcImg.dataProvider else { fatalError("dataProvider is nil") }
guard let srcProvData = srcProv.data else { fatalError("dataProvider.data is nil") }

For unwrap of srcImg.colorSpace! is not good solution, I would recommend to use guard ... else and throw an exception if value is nil.

Overall: why you are using abbreviations and shortcuts for every variable/constant name? It is much harder to read such code

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ trying to write relatively concise code, which why there are some abbreviations like src and dest, and prov. It is harder to read for the first time, i agree. \$\endgroup\$ – Don Ch Jan 9 at 9:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.