3
\$\begingroup\$

This code is working and was tested for over 10000 images. I have many headerless images, i.e images which are binary files with known dimensions. Some of the images have a visible area which is different from the size of the binary files which have padding.

For example, the image is 100x100, however the file size is 100x164, since padding was added to each line. So the user inputs tells me that and I remove that part and return a 100x100 image. The image has different sizes and bits per pixels.

Please comment about my style of code in C#, and performance issues I might have.

public static byte[] FromHeaderless(string path, int width, int height, int bitsPerPixel, bool isBigEndian, bool isPerformShift = true, int extraColsLeft = 0, int extraColsRight = 0, int extraLinesTop = 0, int extraLinesBottom = 0)
{
 byte[] fileData = null;
 if (!File.Exists(path))
 {
 throw new FileNotFoundException(path);
 }
 fileData = File.ReadAllBytes(path);
 return DecodeHeaderless(fileData, width, height, bitsPerPixel, isBigEndian,isPerformShift, extraColsLeft, 
 extraColsRight, extraLinesTop, extraLinesBottom);
}
public static byte[] DecodeHeaderless(byte[] fileData, int width, int height, int bitsPerPixel,
 bool isBigEndian, bool isPerformShift = true, int extraColsLeft = 0, int extraColsRight = 0, 
 int extraLinesTop = 0, int extraLinesBottom = 0)
{
 var bytesPerPixel = (bitsPerPixel + 7) / 8;
 int headerLength = fileData.Length - ((width + extraColsLeft + extraColsRight) * (height + extraLinesBottom + extraLinesTop) * bytesPerPixel);
 if (bytesPerPixel == 1)
 {
 byte[] newImage = new byte[width * height * bytesPerPixel];
 if (extraColsLeft == 0 && extraColsRight == 0 && extraLinesBottom == 0 && extraLinesTop == 0)
 {
 Buffer.BlockCopy(fileData, headerLength, newImage, 0, fileData.Length - headerLength);
 }
 else
 {
 int currentBufferIndex = 0;
 int currentDataIndex = headerLength + extraColsLeft + extraLinesTop * (width + extraColsLeft + extraColsRight);
 Parallel.For(0, height, i =>
 {
 Buffer.BlockCopy(fileData, currentDataIndex, newImage, currentBufferIndex, width);
 currentDataIndex = currentDataIndex + width + extraColsRight + extraColsLeft;
 currentBufferIndex = currentBufferIndex + width;
 }
 );
 }
 return newImage;
 }
 if (bytesPerPixel == 2)
 {
 var dataSize = (width + extraColsRight + extraColsLeft) * (height + extraLinesBottom + extraLinesTop) * bytesPerPixel;
 byte[] newImage = new byte[width * height * bytesPerPixel];
 var sdata = new short[dataSize / 2];
 if (isBigEndian == false)
 {
 for (int i = headerLength, shortIndex = 0; i < dataSize; i += 2, shortIndex++)
 {
 CopyBytesToShort(fileData[i], fileData[i + 1], out sdata[shortIndex], bitsPerPixel, isPerformShift);
 }
 }
 else
 {
 for (int i = headerLength, shortIndex = 0; i < dataSize; i += 2, shortIndex++)
 {
 CopyBytesToShort(fileData[i + 1], fileData[i], out sdata[shortIndex], bitsPerPixel, isPerformShift);
 }
 }
 CopyRelevantImageData(sdata, newImage, bytesPerPixel, width, height, extraLinesTop, extraLinesBottom, extraColsLeft, extraColsRight);
 return newImage;
 }
 return null;
}
private static void CopyBytesToShort(byte byte1, byte byte2, out short retShort, int bitsPerPixel, bool isPerformShift)
{
 short lsb, msb;
 lsb = byte1;
 msb = byte2;
 if (isPerformShift)
 {
 lsb <<= 16 - bitsPerPixel;
 msb <<= (16 - (bitsPerPixel - 8));
 }
 else
 {
 msb <<= 8;
 }
 retShort = (short)(msb | lsb);
}
private static void CopyRelevantImageData(short[] sourceData, byte[] destData, int bytesPerPixel, int width, int height, int extraLinesTop, int extraLinesBottom, int extraColsLeft, int extraColsRight)
{
 if (extraColsLeft == 0 && extraColsRight == 0 && extraLinesBottom == 0 && extraLinesTop == 0)
 {
 Buffer.BlockCopy(sourceData, 0, destData, 0, sourceData.Length * bytesPerPixel);
 }
 else
 {
 int currentBufferIndex = 0;
 int currentDataIndex = extraLinesTop * (width + extraColsLeft + extraColsRight) + extraColsLeft;
 for (int i = 0; i < height; i++)
 {
 Buffer.BlockCopy(sourceData, currentDataIndex * bytesPerPixel, destData, currentBufferIndex, width * bytesPerPixel);
 currentDataIndex += (width + extraColsRight + extraColsLeft);
 currentBufferIndex += (width * bytesPerPixel);
 }
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Sep 17, 2015 at 20:45
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

FromHeaderless has ten parameters, ditto DecodeHeaderless. That's waaaay too many. Once you go beyond four or so, you need to consider creating a specific class where each of those parameters is a property. That way you only need to pass this class, and adding/removing parameters is much easier. Also, you don't risk to mix up parameters of the same type e.g. width and height, or extraColsLeft, extraColsRight, extraLinesTop and extraLinesBottom.


I would move all of the logic inside if (bytesPerPixel == 1) and if (bytesPerPixel == 2) to a separate method each. Each of these cases follows a completely different logic and to me it just makes sense to "de-clutter" DecodeHeaderless.


I would move the if (isBigEndian == false) logic to CopyBytesToShort, to avoid repeating the same thing with only a parameter change as you're doing here:

 if (isBigEndian == false)
 {
 for (int i = headerLength, shortIndex = 0; i < dataSize; i += 2, shortIndex++)
 {
 CopyBytesToShort(fileData[i], fileData[i + 1], out sdata[shortIndex], bitsPerPixel, isPerformShift);
 }
 }
 else
 {
 for (int i = headerLength, shortIndex = 0; i < dataSize; i += 2, shortIndex++)
 {
 CopyBytesToShort(fileData[i + 1], fileData[i], out sdata[shortIndex], bitsPerPixel, isPerformShift);
 }
 }
answered Sep 18, 2015 at 7:37
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.