Click here to Skip to main content
15,880,503 members
Please Sign up or sign in to vote.
3.00/5 (1 vote)
See more:
C#
public unsafe Bitmap  MedianFilter(Bitmap Img)
{
    int Size =2;
    List<byte> R = new List<byte>();
    List<byte> G = new List<byte>();
    List<byte> B = new List<byte>();
    int ApetureMin = -(Size / 2);
    int ApetureMax = (Size / 2);
    BitmapData imageData = Img.LockBits(new Rectangle(0, 0, Img.Width, Img.Height), ImageLockMode.ReadOnly, PixelFormat.Format32bppRgb);
    byte* start = (byte*)imageData.Scan0.ToPointer ();
    for (int x = 0; x < imageData.Width; x++)
    {
        for (int y = 0; y < imageData.Height; y++)
        {
            for (int x1 = ApetureMin; x1 < ApetureMax; x1++)
            {
                int valx = x + x1;
                if (valx >= 0 && valx < imageData.Width)
                {
                    for (int y1 = ApetureMin; y1 < ApetureMax; y1++)
                    {
                        int valy = y + y1;
                        if (valy >= 0 && valy < imageData.Height)
                        {
                            // error come from here
                            Color tempColor = Img.GetPixel(valx, valy);

                            R.Add(tempColor.R);
                            G.Add(tempColor.G);
                            B.Add(tempColor.B);
                        }
                    }
                }
            }
        }
    }
    R.Sort();
    G.Sort();
    B.Sort();
    Img.UnlockBits(imageData);
    return Img;
}

I tried to do this. but i got an error call "Bitmap region is already locked" can anyone help how to solve this. (error position is highlighted)
Posted
Updated 11-May-11 2:02am
v3

If you're going to use the unsafe approach, don't use GetPixel. The whole point of the unsafe approach is to bypass the slow managed API. You should be using *scan++ (if you are reading whole rows from the rectangle you selected, which you seem to be).

As for the error, I guess Get/SetPixel lock the image internally (possibly a part of why they're so slow), and since you are (correctly) locking it yourself, that fails. But your problem is that you have a horrible mashing together of the safe, slow way and the unsafe, fast way.
 
Share this answer
 
Comments
Olivier Levrey 11-May-11 8:05am    
You are completely right. Have my 5.
Steve Roziro 11-May-11 8:43am    
can you show me how to make this done.I'm new to image processing and this is median filtering. i got an code which provided by the codeproject but it is not unsafe method. :(
BobJanova 11-May-11 10:33am    
There are actually other serious problems with your code: you return Img without ever modifying it or doing anything with the R/G/B lists. I think you need to understand the algorithm you're trying to do better. I don't know what a median filter is myself so I can't help with that.

The link Oliver posted in his solution shows you how to do this the 'safe' way. You want something like


byte* start = (byte*)imageData.Scan0.ToPointer ();
for (int x = 0; x < imageData.Width; x++)
{
for (int y = 0; y < imageData.Height; y++)
{
for (int x1 = ApetureMin; x1 < ApetureMax; x1++)
{
int valx = x + x1;
if (valx >= 0 && valx < imageData.Width)
{
for (int y1 = ApetureMin; y1 < ApetureMax; y1++)
int valy = y + y1;
if (valy >= 0 && valy < imageData.Height)
{
byte a = *(start + valy * stride + valx);
byte r = *(1 + start + valy * stride + valx);
byte g = *(2 + start + valy * stride + valx);
byte b = *(3 + start + valy * stride + valx);

// OR: uint col = *(uint*)(start + valy * stride + valx)
// if you want to work with all 32 bits

R.add(r); G.add(g); B.add(b);
}
}

Color output = SomeCleverFunction(R, G, B);
byte* rp = *(1 + start + y * stride + x);
(*rp) = output.R;
// etc
}
}
}

disclaimer: not tested, I've not done this stuff for real. Always double check pointer related code off the internet before trying to use it!
Sergey Alexandrovich Kryukov 11-May-11 13:56pm    
Absolutely right, my 5.

Also, GetPixel/SetPixel is a real performance killer. Please see my answer.
--SA
BobJanova is right.

GetPixel and SetPixel call LockBits and UnlockBits internally. This is why these functions are so slow and shouldn't be used for image processing.

Follow this link to learn how to use LockBits and access internal data:
http://msdn.microsoft.com/en-us/library/5ey6h79d.aspx[^]
 
Share this answer
 
v2
Comments
Sergey Alexandrovich Kryukov 11-May-11 13:55pm    
This is correct answer, my 5.

Also, GetPixel/SetPixel presents performance problem. Please see my answer.
--SA
Never use GetPixel/SetPixel unless you need just one or two (which is pretty rare) or ready to kill your performance. When you use LockBits, its more than enough to access individual pixels.

—SA
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900