Noblis / INVSC-janice

An open API for computer vision algorithms
https://noblis.github.io/janice/
Other
9 stars 4 forks source link

JaniceMediaIterator should expose more data about its contents #16

Closed taa01776 closed 6 years ago

taa01776 commented 6 years ago

Many of the JaniceMediaIterator calls are documented to fail if the iterator is encapsulating an image, rather than a video. However, there is no call that provides that information. There should be.

Given that an iterator is either a single image or a single video, is there ever a case where the image height, width, or depth could change from one image to the next? If not, the iterator should provide a call to return those values.

It would be very helpful to provide a call returning the number of images available from the iterator.

JordanCheney commented 6 years ago

Based on #15 and this issue I propose the following-

  1. Add a JaniceError is_video(bool&) method to iterators to test if an iterator is a video
  2. Remove get() in favor of calling seek() + next() explicitly
  3. Adjust the documentation of seek() and tell(). Both should return JANICE_INVALID_MEDIA if the media is an image. Adjust the provided reference implementation to match the documentation.
  4. Add a JaniceError get_frame_rate(float&) method to query the video frame rate. If the media is an image, this function should return JANICE_INVALID_MEDIA. This has been requested by other users in the past.

Returning size information about an iterator has some performance implications. Notably, querying the height, width or depth of an image or video requires an implicit load of the media. An implementation could cache those values after an initial load but that information is also available in the returned JaniceImage and could be stored by the caller if they need to keep it around.

We've had significant issues internally with OpenCV returning an incorrect number of video frames when queried with CV_CAP_PROP_FRAME_COUNT. The only way we've found to reliably get the total number of frames in a video is to iterate and count them. This is the main reason we don't provided a length() method on JaniceMedia currently. If you have a more reliable workaround we'd certainly be open to it.

taa01776 commented 6 years ago

These all sound good. I understand the issue with querying image metadata and/or video information. When I get to building our own I/O library, I’ll let you know if I find any useful tricks for things like frame count. -ta

From: JordanCheney [mailto:notifications@github.com] Sent: Monday, 5 February, 2018 12:31 To: Noblis/janice janice@noreply.github.com Cc: Anderson, Timothy A. taa@stresearch.com; Author author@noreply.github.com Subject: Re: [Noblis/janice] JaniceMediaIterator should expose more data about its contents (#16)

Based on #15https://github.com/Noblis/janice/issues/15 and this issue I propose the following-

  1. Add a JaniceError is_video(bool&) method to iterators to test if an iterator is a video
  2. Remove get() in favor of calling seek() + next() explicitly
  3. Adjust the documentation of seek() and tell(). Both should return JANICE_INVALID_MEDIA if the media is an image. Adjust the provided reference implementation to match the documentation.
  4. Add a JaniceError get_frame_rate(float&) method to query the video frame rate. If the media is an image, this function should return JANICE_INVALID_MEDIA. This has been requested by other users in the past.

Returning size information about an iterator has some performance implications. Notably, querying the height, width or depth of an image or video requires an implicit load of the media. An implementation could cache those values after an initial load but that information is also available in the returned JaniceImage and could be stored by the caller if they need to keep it around.

We've had significant issues internally with OpenCV returning an incorrect number of video frames when queried with CV_CAP_PROP_FRAME_COUNT. The only way we've found to reliably get the total number of frames in a video is to iterate and count them. This is the main reason we don't provided a length() method on JaniceMedia currently. If you have a more reliable workaround we'd certainly be open to it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/Noblis/janice/issues/16#issuecomment-363158202, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABnTXQwQHszfJD0Mj8MOhv-Wx3yGR0SRks5tRzq6gaJpZM4RywoB.

taa01776 commented 6 years ago

I will say that get() as a true read-only operation (that is, one that does not change the media parameter) can be valuable--it's not uncommon for us to downsample a video in the course of processing it, so the resulting track will have non-sequential frame numbers. seek(N)/next() is not nearly as convenient, or, more important, as clear as get(N).

JordanCheney commented 6 years ago

This is addressed by 2228c21