Skip to content
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

ENH: Add ImageToVideoFilter and accompanying wrappings #2635

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

tbirdso
Copy link
Contributor

@tbirdso tbirdso commented Jul 6, 2021

Adding itk::ImageToVideoFilter to ITKVideoCore module along with baseline test and wrappings.

This class serves as a bridge to convert static itk::Image objects to itk::VideoStream representation. The user supplies an itk::Image input to the filter and designates a single axis to interpret as a temporal axis in the output. When Update() is called the filter allocates the output itk::VideoStream and grafts slices of the input along its designated axis onto output frames.

The goal of adding this filter is to make it easier to use static data sets as test sets for real-time systems making use of itk::VideoStream for data collection and representation. The filter is single-threaded and could be revisited to take advantage of multithreading from the base itk::VideoSource class at some point in the future.

Comments and revisions are welcomed.

@thewtex

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

@github-actions github-actions bot added area:Python wrapping Python bindings for a class area:Video Issues affecting the Video module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct labels Jul 6, 2021
@thewtex thewtex requested a review from brad-t-moore July 6, 2021 16:02
@tbirdso tbirdso force-pushed the image-to-video branch 4 times, most recently from 8de6afb to 5a30133 Compare July 6, 2021 23:56
@tbirdso
Copy link
Contributor Author

tbirdso commented Jul 6, 2021

Working on resolving upstream CI warnings not appearing on my local platform.

EDIT: Looks like warnings are resolved.

@thewtex thewtex requested review from dzenanz and aylward July 7, 2021 20:21
thewtex
thewtex previously requested changes Jul 7, 2021
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tbirdso very nice!!

A few comments inline.

Modules/Video/Core/include/itkImageToVideoFilter.h Outdated Show resolved Hide resolved
Modules/Video/Core/include/itkImageToVideoFilter.hxx Outdated Show resolved Hide resolved
Modules/Video/Core/include/itkImageToVideoFilter.h Outdated Show resolved Hide resolved
Modules/Video/Core/include/itkImageToVideoFilter.hxx Outdated Show resolved Hide resolved
Modules/Video/Core/include/itkImageToVideoFilter.hxx Outdated Show resolved Hide resolved
Modules/Video/Core/include/itkImageToVideoFilter.hxx Outdated Show resolved Hide resolved
Modules/Video/Core/include/itkImageToVideoFilter.hxx Outdated Show resolved Hide resolved
Modules/Video/Core/include/itkImageToVideoFilter.hxx Outdated Show resolved Hide resolved
Copy link
Member

@aylward aylward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Posted two questions - perhaps they need to be addressed...or ignored...

Modules/Video/Core/include/itkImageToVideoFilter.h Outdated Show resolved Hide resolved
@dzenanz
Copy link
Member

dzenanz commented Jul 7, 2021

@tbirdso please ping me after you have addressed the existing reviews. That way I have somewhat fresh eyes updated PR.

@tbirdso tbirdso force-pushed the image-to-video branch 2 times, most recently from 835a456 to e825084 Compare July 7, 2021 22:28
@brad-t-moore
Copy link
Contributor

brad-t-moore commented Jul 8, 2021 via email

Copy link
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, some comments in line.

Modules/Video/Core/test/itkImageToVideoFilterTest.cxx Outdated Show resolved Hide resolved
Modules/Video/Core/test/itkImageToVideoFilterTest.cxx Outdated Show resolved Hide resolved
@tbirdso tbirdso force-pushed the image-to-video branch 2 times, most recently from 8738fce to 310f561 Compare July 9, 2021 13:53
@dzenanz dzenanz requested a review from thewtex July 9, 2021 13:54
@tbirdso tbirdso force-pushed the image-to-video branch 3 times, most recently from 467aa8a to 82e3350 Compare July 12, 2021 18:55
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@thewtex thewtex merged commit fb4fc01 into InsightSoftwareConsortium:master Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:Python wrapping Python bindings for a class area:Video Issues affecting the Video module type:Enhancement Improvement of existing methods or implementation type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants