Stb #3823

shrit · 2024-11-02T14:51:47Z

Make STB part of mlpack.

Just a draft, to get an idea if this works with the CI

Signed-off-by: Omar Shrit <[email protected]>

src/mlpack/config.hpp

src/mlpack/core/data/stb_image.h

rcurtin · 2024-11-02T15:12:14Z

It could also be a good idea to update LICENSE.txt: https://github.com/mlpack/mlpack/blob/master/LICENSE.txt#L35

STB is dual-licensed... both under the MIT and also it's in the public domain (which means we wouldn't have to list it in the license). But I think it might be a good idea to point it out in the license file, both to give credit to the STB authors and also so that people coming through the project have more ways to learn that we are bundling STB.

Signed-off-by: Omar Shrit <[email protected]>

rcurtin

Nice, it will be super useful to add some extra image support. My main comment is that we should use the same formats for images that data::Load gives.

Also, up to you, but do you want to separate this into two separate PRs for simplicity? One could be including STB as part of mlpack and making those CMake configuration changes, and the other one could be the resize/crop functionality that you added in the latest commit.

src/mlpack/core/data/image_resize_crop.hpp

Signed-off-by: Omar Shrit <[email protected]>

rcurtin

Some comments on the STB CMake code---hopefully it's helpful. I'm looking forward to having STB bundled; it'll make life easier 😄

CMakeLists.txt

LICENSE.txt

src/mlpack/config.hpp

src/mlpack/core/stb/stb.hpp

src/mlpack/tests/data/umbrella.jpg

src/mlpack/core/data/image_resize_crop.hpp

Co-authored-by: Ryan Curtin <[email protected]>

Signed-off-by: Omar Shrit <[email protected]>

Co-authored-by: Ryan Curtin <[email protected]>

Signed-off-by: Omar Shrit <[email protected]>

src/mlpack/tests/image_test.cpp

Signed-off-by: Omar Shrit <[email protected]>

rcurtin · 2025-01-30T19:09:07Z

@mlpack-jenkins test this please

shrit · 2025-02-02T09:35:54Z

@mlpack-jenkins test this please

Signed-off-by: Omar Shrit <[email protected]>

rcurtin · 2025-02-03T19:54:55Z

@mlpack-jenkins test this please

rcurtin

It looks good to me. There is a bug with the conv_to in the image resize functionality (take a look, I think I have the fix in the comments), and there are a few simple optimizations I think we can make. It's late now so I won't try it, but I will do it tomorrow when I get up unless you get to it first (I think all my suggested fixes should work directly, take a look and let me know what you think). Glad to get this merged, sorry I have had so many tedious comments throughout 😄

HISTORY.md

src/mlpack/config.hpp

rcurtin · 2025-02-04T03:52:32Z

src/mlpack/core/data/image_resize_crop.hpp

+      arma::fmat tempDestFloat(newDimension, 1);
+      stbir_resize_float_linear(images.colptr(i), info.Width(), info.Height(),
+          0, tempDestFloat.memptr(), newWidth, newHeight, 0, channels);
+      resizedFloatImages.col(i) = std::move(tempDestFloat);


Suggested change

arma::fmat tempDestFloat(newDimension, 1);

stbir_resize_float_linear(images.colptr(i), info.Width(), info.Height(),

0, tempDestFloat.memptr(), newWidth, newHeight, 0, channels);

resizedFloatImages.col(i) = std::move(tempDestFloat);

stbir_resize_float_linear(images.colptr(i), info.Width(), info.Height(),

0, resizedFloatImages.colptr(i), newWidth, newHeight, 0, channels);

rcurtin · 2025-02-04T03:53:05Z

src/mlpack/core/data/image_resize_crop.hpp

+      arma::Mat<unsigned char> tempDest(newDimension, 1);
+      stbir_resize_uint8_linear(images.colptr(i), info.Width(), info.Height(),
+          0, tempDest.memptr(), newWidth, newHeight, 0, channels);
+
+      resizedImages.col(i) = std::move(tempDest);


Suggested change

arma::Mat<unsigned char> tempDest(newDimension, 1);

stbir_resize_uint8_linear(images.colptr(i), info.Width(), info.Height(),

0, tempDest.memptr(), newWidth, newHeight, 0, channels);

resizedImages.col(i) = std::move(tempDest);

stbir_resize_uint8_linear(images.colptr(i), info.Width(), info.Height(),

0, resizedImages.colptr(i), newWidth, newHeight, 0, channels);

I am pretty sure that this should work. I will try it locally tomorrow, but you will wake up earlier than me so if you want you can try it too 😄

rcurtin · 2025-02-04T03:54:49Z

src/mlpack/core/data/image_resize_crop.hpp

+    }
+  }
+
+  images = std::move(resizedImages);


Suggested change

images = std::move(resizedImages);

images = arma::conv_to<Mat<eT>>::from(std::move(resizedImages));

We need to put the conv_to here, because resizedImages has type arma::Mat<unsigned char> and images has type arma::Mat<eT>.

rcurtin · 2025-02-04T03:55:06Z

src/mlpack/core/data/image_resize_crop.hpp

+      arma::Mat<unsigned char> tempDest(newDimension, 1);
+
+      stbir_resize_uint8_linear(tempSrc.colptr(i), info.Width(), info.Height(),
+          0, tempDest.memptr(), newWidth, newHeight, 0, channels);
+
+      resizedImages.col(i) = arma::conv_to<arma::Mat<eT>>::from(tempDest);


Suggested change

arma::Mat<unsigned char> tempDest(newDimension, 1);

stbir_resize_uint8_linear(tempSrc.colptr(i), info.Width(), info.Height(),

0, tempDest.memptr(), newWidth, newHeight, 0, channels);

resizedImages.col(i) = arma::conv_to<arma::Mat<eT>>::from(tempDest);

stbir_resize_uint8_linear(tempSrc.colptr(i), info.Width(), info.Height(),

0, resizedImages.colptr(i), newWidth, newHeight, 0, channels);

github-actions

Second approval provided automatically after 24 hours. 👍

Co-authored-by: Ryan Curtin <[email protected]>

shrit added 3 commits November 2, 2024 15:31

Move STB internally, comment functions if not

20ae504

Signed-off-by: Omar Shrit <[email protected]>

Comment for now STB including macros

79cb8ee

Signed-off-by: Omar Shrit <[email protected]>

Modify CMake and tests accordingly

d0df8bb

Signed-off-by: Omar Shrit <[email protected]>

shrit requested a review from rcurtin November 2, 2024 14:51

rcurtin reviewed Nov 2, 2024

View reviewed changes

src/mlpack/config.hpp Outdated Show resolved Hide resolved

rcurtin reviewed Nov 2, 2024

View reviewed changes

src/mlpack/core/data/stb_image.h Show resolved Hide resolved

shrit added 3 commits November 11, 2024 19:48

Fix the license and update the date.

8d0d665

Signed-off-by: Omar Shrit <[email protected]>

Move STB to its own directory

cb93f2d

Signed-off-by: Omar Shrit <[email protected]>

Adding the functrions of image resize and crop

03e9866

Signed-off-by: Omar Shrit <[email protected]>

rcurtin reviewed Nov 14, 2024

View reviewed changes

src/mlpack/core/data/image_resize_crop.hpp Outdated Show resolved Hide resolved

src/mlpack/core/data/image_resize_crop.hpp Outdated Show resolved Hide resolved

shrit added 11 commits November 15, 2024 15:19

Update the API not finished yet

557484f

Signed-off-by: Omar Shrit <[email protected]>

Merge branch 'master' into stb

bd29810

Start adding tests

aa62ccb

Signed-off-by: Omar Shrit <[email protected]>

Add flowers images for test

d0c5bad

Signed-off-by: Omar Shrit <[email protected]>

Bring image from the coco dataset

2369e41

Signed-off-by: Omar Shrit <[email protected]>

Include all of STB header in this file

7bde2bb

Signed-off-by: Omar Shrit <[email protected]>

Fix STB configs

990705b

Signed-off-by: Omar Shrit <[email protected]>

Clean CMakeLists and look for stb_resize

500f0f0

Signed-off-by: Omar Shrit <[email protected]>

Make it compile with STB integrated

a1f4399

Signed-off-by: Omar Shrit <[email protected]>

Fix stb path

ed542e9

Signed-off-by: Omar Shrit <[email protected]>

Fix the endif at the end of the file

c624fb2

Signed-off-by: Omar Shrit <[email protected]>

rcurtin reviewed Nov 17, 2024

View reviewed changes

shrit and others added 4 commits November 17, 2024 11:10

Update LICENSE.txt

084d68e

Co-authored-by: Ryan Curtin <[email protected]>

Update CMakeLists.txt

91eb425

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/config.hpp

85fc326

Co-authored-by: Ryan Curtin <[email protected]>

Adding the function and test, not finished

5717486

Signed-off-by: Omar Shrit <[email protected]>

shrit mentioned this pull request Nov 20, 2024

Automate STB Header Version Update in GitHub Workflows #3834

Closed

eddelbuettel mentioned this pull request Dec 5, 2024

Quieten STB compilation with -Wno-missing-field-initializers #3846

Merged

github-actions bot added the s: stale label Dec 18, 2024

shrit and others added 8 commits January 29, 2025 11:15

Update doc/user/load_save.md

ddba6e2

Co-authored-by: Ryan Curtin <[email protected]>

Update doc/user/load_save.md

c589668

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/core/data/image_resize_crop.hpp

fccd773

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/core/data/image_resize_crop.hpp

ff3ea01

Co-authored-by: Ryan Curtin <[email protected]>

Update src/mlpack/core/data/image_resize_crop.hpp

d3a9b5c

Co-authored-by: Ryan Curtin <[email protected]>

Update doc/user/load_save.md

11acb93

Co-authored-by: Ryan Curtin <[email protected]>

Apply comments from @rcurtin

f417a2f

Signed-off-by: Omar Shrit <[email protected]>

Adding the test to test if it is identical

2b86801

Signed-off-by: Omar Shrit <[email protected]>

rcurtin reviewed Jan 29, 2025

View reviewed changes

src/mlpack/tests/image_test.cpp Outdated Show resolved Hide resolved

shrit added 3 commits January 30, 2025 12:23

Fix image tests

9f030e9

Signed-off-by: Omar Shrit <[email protected]>

Fix the example

cd26af0

Signed-off-by: Omar Shrit <[email protected]>

Add the link and the patch to download the images

c69190b

Signed-off-by: Omar Shrit <[email protected]>

shrit added 2 commits February 2, 2025 18:08

Fix the script and make all of them green

714d330

Signed-off-by: Omar Shrit <[email protected]>

Remove wget not needed

b7f8530

Signed-off-by: Omar Shrit <[email protected]>

Minor style fixes.

7513f00

rcurtin approved these changes Feb 4, 2025

View reviewed changes

Avoid temporary matrices, and test with multiple different types.

f15b83c

github-actions bot approved these changes Feb 5, 2025

View reviewed changes

shrit and others added 3 commits February 5, 2025 11:30

Update src/mlpack/config.hpp

5e18974

Co-authored-by: Ryan Curtin <[email protected]>

Update HISTORY.md

5ffc81f

Co-authored-by: Ryan Curtin <[email protected]>

Fix R build by including bundled STB headers in package.

6122562

shrit merged commit 821a95f into mlpack:master Feb 5, 2025
18 of 20 checks passed

shrit deleted the stb branch February 5, 2025 20:13

This was referenced Mar 31, 2025

Release version 4.6.0 #3914

Closed

Release version 4.6.0 #3915

Closed

Release version 4.6.0 #3916

Merged

ErniGH mentioned this pull request May 6, 2025

Updating the available versions in the mlpack package conan-io/conan-center-index#27203

Closed

	images = std::move(resizedImages);
	images = arma::conv_to<Mat<eT>>::from(std::move(resizedImages));

Uh oh!

Stb #3823

Stb #3823

Uh oh!

Conversation

shrit commented Nov 2, 2024

Uh oh!

Uh oh!

Uh oh!

rcurtin commented Nov 2, 2024

Uh oh!

rcurtin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rcurtin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rcurtin commented Jan 30, 2025

Uh oh!

shrit commented Feb 2, 2025

Uh oh!

rcurtin commented Feb 3, 2025

Uh oh!

rcurtin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rcurtin Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

rcurtin Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

rcurtin Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

rcurtin Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants