.Net getting RGB values from a bitmap using Lockbi

I am using the below code to extract RGB values from images, sometimes this works, however on certain files (seemingly where the Stride is not divisible by the width of the bitmap) it is returning mixed up values:

Dim rect As New Rectangle(0, 0, bmp.Width, bmp.Height)
Dim bmpData As System.Drawing.Imaging.BitmapData = bmp.LockBits(rect, Imaging.ImageLockMode.ReadOnly, Imaging.PixelFormat.Format24bppRgb)
Dim ptr As IntPtr = bmpData.Scan0
Dim cols As New List(Of Color)
Dim bytes As Integer = Math.Abs(bmpData.Stride) * bmp.Height
Dim rgbValues(bytes - 1) As Byte
System.Runtime.InteropServices.Marshal.Copy(ptr, rgbValues, 0, bytes)

' Retrieve RGB values
For i = modByte To rgbValues.Length Step 3
     cols.Add(Color.FromArgb(rgbValues(i + 2), rgbValues(i + 1), rgbValues(i)))
Next

bmp.UnlockBits(bmpData)
bmp.Dispose()
Dim colsCnt As List(Of RgbPixels) = cols.GroupBy(Function(g) New With {Key .R = g.R, Key .G = g.G, Key .B = g.B}).Select(Function(s) New RgbPixels With {.Colour = Color.FromArgb(s.Key.R, s.Key.G, s.Key.B), .Amount = s.Count()}).ToList()

After grouping the resulting colours, the values are something like:

R    G    B
255  255  255
255  255  0
255  0    0
0    0    255
0    255  255

Or some variation of that, when they should just be:

R    G    B
255  255  255
0    0    0

Please point me in the right direction, BTW my source bmp is in PixelFormat.Format24bppRgb too, so I don't believe that is the problem. Also if you can only answer in C# that is not a problem.

The problem is that you're not considering the stride value. Stride is always padded so that the width of the byte-array per image row is dividable by 4. This is an optimization related to memory copy and how the CPU works, that goes decades back and still is useful.

F.ex, if one image has a width of 13 pixels, the stride would be like this (simplified to one component):

=============    (width 13 pixels = 13 bytes when using RGB)
================ (stride would be 16)

for an image of 14 pixels it would look like this:

==============   (width 14 pixels = 14 bytes when using RGB)
================ (stride would still be 16)

So in your code you need to handle a stride row instead of a byte array, unless you are using fixed and defined widths of the images.

I modified your code so it skips rows by stride:

Dim rect As New Rectangle(0, 0, bmp.Width, bmp.Height)
Dim bmpData As System.Drawing.Imaging.BitmapData = bmp.LockBits(rect, Imaging.ImageLockMode.ReadOnly, Imaging.PixelFormat.Format24bppRgb)
Dim ptr As IntPtr = bmpData.Scan0
Dim cols As New List(Of Color)
Dim bytes As Integer = Math.Abs(bmpData.Stride) * bmp.Height
Dim rgbValues(bytes - 1) As Byte
System.Runtime.InteropServices.Marshal.Copy(ptr, rgbValues, 0, bytes)

Dim x, y, dx, l as Integer

For y = 0 To rect.Height - 1

    l = y * bmpData.Stride 'calulate line based on stride

    For x = 0 To rect.Width - 1

        dx = l + x * 3  '3 for RGB, 4 for ARGB, notice l is used as offset

        cols.Add(Color.FromArgb(rgbValues(dx + 2), _
                                rgbValues(dx + 1), _
                                rgbValues(dx)))
    Next
Next

' Retrieve RGB values
'For i = modByte To rgbValues.Length Step 3
'     cols.Add(Color.FromArgb(rgbValues(i + 2), rgbValues(i + 1), rgbValues(i)))
'Next

bmp.UnlockBits(bmpData)
bmp.Dispose()