-
Notifications
You must be signed in to change notification settings - Fork 16
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
New cluster #43
base: phase2
Are you sure you want to change the base?
New cluster #43
Conversation
…ry instead of as part of the implementation. Refactored verify to work in a lazy-loading fashion using the matches file to determine what templates to match
Fixed memory leaks in janus_harness_verify
…tell if multiple frames come from the same source video.
…o metadata in 1N probe video and allow additional tracking information to be captured during janus_create_template
janus_template_id replaced with uint32_t Updated cluster harness to support template lists and media lists Added command line argument to janus_cluster utility to support template lists and media lists Updated PP5 routines to handle lazy loading Updated PP5 Clustering for template list support Removed janus_association structure and put janus_media pointer in janus_track Changed janus_create_template function prototype to qsupport std::vector<janus_track> instead of std::vector<janus_association> Changed protocol parser to support one piece of metadata per subject per media
…w_cluster Conflicts: include/iarpa_janus.h src/janus_io.cpp src/opencv_io/opencv_io.cpp src/opencv_io/opencv_io.h
@JordanCheney I have no problems with items 2-7, with the minor exception of #3 - changing the underlying type is fine, but I'd prefer to retain the typedef to keep our code clearer. For #1, I think the change to providing an iterator and indexed access to video frames is great. On the other hand, if I'm reading the proposed implementation correctly, the only way we would have access to the janus_media interface is to either implement our own, or include one of the provided test harness implementations into our library. Instead of an opaque pointer to struct, it seems better to provide an abstract base class as an interface, and to have the janus_media implementations inherit from that. Implementing our own janus_media would not be desirable for us - it would mean committing to supporting a fixed set of video container types and codecs, as well as image formats. For example, OpenCV doesn't support GIFs, so we would need to include an additional library to support that, and our code would not be future proof to new formats. It might also introduce licensing or patent issues with libraries we would need to distribute with our code. We would rather deal with raw pixels. The second option for us would be including, e.g., opencv_io.h from the test harness to get the interface for that implementation. That would mean that a user of our library that wanted to change to a different janus_media implementation would need to edit and recompile our code, which also seems undesirable. I'd prefer defining a janus_media interface in iarpa_janus.h. |
@nxwhite-str Thanks for the quick review, these are all valid points. For #1 we will bring back the typedef. I didn't consider some of the license difficulties with opaque media pointers. I think your proposed public interface solves that problem cleanly. I'm envisioning two structures- janus_image which is basically a pointer to some data with size information and janus_media_iterator which implements the interface laid out in opencv_io.h. I will put the header together and add it here for review. |
Hi @JordanCheney -- for clarification, is the plan to push this change prior to the upcoming API delivery at the end of January, or is this change going to happen afterwards? Thanks, |
@stephenrawls Hi Stephen, I'm sure you saw on the mailing list but this change will not effect the February delivery but will be for the final phase 2 delivery. |
Removed strict failure mode in janus_ocv_util for corrupt videos
@nxwhite-str @carlosdcastillo @stephenrawls
This is a large commit but I hope it makes things easier for you as you implement clustering and addresses a few pain points. I will outline the major changes below:
Please let me know your thoughts, questions, concerns as you work through the review.
Best,
Jordan