-
Notifications
You must be signed in to change notification settings - Fork 3
conv: Support buffer strides on unpacked raw buffers #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
pixutils/conv/raw.py
Outdated
| data = data.reshape((height, len(data) // height)) | ||
|
|
||
| # Remove padding if present | ||
| padded_width = width * bits_per_pixel // 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did you test this?
The unpacked data is in 16-bit containers for 10/12/16. The above calculates a packed width.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, so most of my current test on this only displays the top left corner of a 20Megapixel image - so I could easily have got this wrong in fact.
This patch does however now let me /see/ the top left corner which is what I needed ;-) but yes - it probably needs retesting without my artificial crops added to verify the whole image.
I'm testing on a Raspberry Pi 5 capturing SRGGB16.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok - so perhaps this only worked for me because I was fortunately using SRGGB16 to match the 16 bit alignement of the data coming in by chance.
So perhaps regardless of bits_per_pixel, we should always do padded_width = width * 2; /* 16 bit data */
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope - now I see it. Particularly retesting on a system with 12 bit data instead of 16 bit data ;-)
# Remove padding if present
- padded_width = width * bits_per_pixel // 8
+ # The unpacked data is stored in 8 bits for 8bpp, and 16 bits for 10/12/16bpp.
+ bytes_per_pixel = 1 if bits_per_pixel == 8 else 2
+ padded_width = width * bytes_per_pixel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or more generically to support future higher bit depths:
# The unpacked data is stored in 8 bits for 8bpp, and 16 bits for 10/12/16bpp
bytes_per_pixel = (bits_per_pixel + 7) // 8
padded_width = width * bytes_per_pixel
Platforms may configure buffers that have a larger buffer stride than the image width to support hardware padding restrictions. The raw_to_bgr888() function already accounts for this when operating on prepare_packed_raw(), however prepare_unpacked_raw() raw is missing the handling for buffer strides. Reshape the data, and delete the padding accordingly. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
a395394 to
548f27b
Compare
|
@tomba Any happier with this now ? |
Platforms may configure buffers that have a larger buffer stride than the image width to support hardware padding restrictions.
The raw_to_bgr888() function already accounts for this when operating on prepare_packed_raw(), however prepare_unpacked_raw() raw is missing the handling for buffer strides.
Reshape the data, and delete the padding accordingly.