[Images] [3/N] Add image decoding for uint8 images. #981
Conversation
| @@ -58,17 +58,7 @@ impl<'a> DaftImageBuffer<'a> { | |||
| with_method_on_image_buffer!(self, width) | |||
| } | |||
|
|
|||
| pub fn channels(&self) -> u16 { | |||
There was a problem hiding this comment.
We already have ImageMode::num_chanels(), trying to keep a single source of truth there.
| @@ -136,6 +130,44 @@ where | |||
| ImageBuffer::from_raw(w, h, owned).unwrap() | |||
| } | |||
|
|
|||
| impl<'a> From<DynamicImage> for DaftImageBuffer<'a> { | |||
There was a problem hiding this comment.
Are you intending to take ownership of the buffer of DynamicImage or Borrow the buffer? Right now it's taking ownership.
There was a problem hiding this comment.
Ownership, since we should only be doing this conversion on ingest (decoding), right? I can change this to borrow the buffer if we think that will be needed.
There was a problem hiding this comment.
This is fine then, we can implement a From<&'a DynamicImage> down the road when/if we need to borrow.
| @@ -227,7 +259,6 @@ impl ImageArray { | |||
|
|
|||
| assert_eq!(result.height(), h); | |||
| assert_eq!(result.width(), w); | |||
| assert_eq!(result.channels(), c); | |||
There was a problem hiding this comment.
This DaftImageBuffer::channels() method was duplicating the ImageMode::num_channels() logic, so I'd like to consolidate that to one place if possible. We're also no longer pulling the channels from the DaftImageBuffer in from_daft_image_buffer() (we're pulling from ImageMode::num_channels() instead), so I don't see motivation for continuing to have a DaftImageBuffer::channels() API.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #981 +/- ##
==========================================
- Coverage 85.04% 84.96% -0.08%
==========================================
Files 184 184
Lines 15835 15906 +71
==========================================
+ Hits 13467 13515 +48
- Misses 2368 2391 +23
|
| pub modes: Vec<u8>, | ||
| pub offsets: Vec<i64>, | ||
| pub validity: Option<arrow2::bitmap::Bitmap>, | ||
| } |
There was a problem hiding this comment.
Added this to keep the ImageArray::from_vecs() function from having a lot of arguments.
| @@ -179,6 +226,77 @@ impl ImageArray { | |||
| array.as_ref().as_any().downcast_ref().unwrap() | |||
| } | |||
|
|
|||
| pub fn from_vecs<T: arrow2::types::NativeType>( | |||
There was a problem hiding this comment.
@samster25 I consolidated the kernels and casting vecs --> ImageArray construction to this single function, to make sure that those paths don't diverge.
…ng; always return an ImageArray
clarkzinzow
force-pushed
the
clark/full-image-decoding
branch
from
d59dedf to
cd47dac
Compare
This PR adds basic support for decoding uint8 images.