Add support for feed-forward multi-track video requests.

[jrobble]

Issues:

• Support feeding forward all tracks to one sub-job (for Python video jobs only) openmpf#1907

Related PRs:

• Add support for feed-forward multi-track video requests. openmpf#1906
• Add support for feed-forward multi-track video requests. #91
• Add support for feed-forward multi-track video requests. openmpf-cpp-component-sdk#129
• Document FEED_FORWARD_ALL_TRACKS behavior. openmpf.github.io#203
• DUMMY: Add PythonTestComponent. openmpf-docker#242

---

This change is 

[brosenberg42]

Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on @jrobble)

---

detection/api/mpf_component_api/mpf_component_api.py line 83 at r2 (raw file):

    job_properties: Mapping[str, str]
    media_properties: Mapping[str, str]
    feed_forward_tracks: Optional[List[VideoTrack]] = None

Shouldn't this job type always have feed_forward_tracks set? If so, the field should not be Optional and should not have a default value.

---

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

        if video_job.feed_forward_track is not None:
            video_job.feed_forward_track.detection_properties['annotated_prop_single_track'] = 'annotated_val_single_track'

These properties are only ever set, not read. These changes look like they were for debugging or you intended to update a unit test, but never did.

[jrobble]

Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on @brosenberg42 and @jrobble)

---

detection/api/mpf_component_api/mpf_component_api.py line 83 at r2 (raw file):

Previously, brosenberg42 wrote…

Shouldn't this job type always have feed_forward_tracks set? If so, the field should not be Optional and should not have a default value.

According to this logic we never create a feed-forward all tracks request if there are no feed-forward tracks generated in the previous task:

private Optional<DetectionRequest> createFeedForwardAllTracksRequest(Media media, DetectionContext context) {
        int topQualityCount = getTopQualityCount(context);
        String topQualitySelectionProp = context.getQualitySelectionProperty();
        var tracks = _triggerProcessor.getTriggeredTracks(media, context)
                .filter(t -> {
                    if (t.getDetections().isEmpty()) {
                        log.warn("Found track with no detections. "
                                    + "No feed forward request will be created for: {}", t);
                        return false;
                    }
                    return true;
                })
                .collect(Collectors.toList());

        if (tracks.isEmpty()) {
            return Optional.empty();
        }

---

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

Previously, brosenberg42 wrote…

These properties are only ever set, not read. These changes look like they were for debugging or you intended to update a unit test, but never did.

The reason I set the properties you're referring to is to show the difference between feed-forward all tracks and feed-forward single tracks in the JSON output object when running this component in a Docker deployment with the WFM.

It appears that this component is only used as a developer reference. Technically, none of the properties are ever read.

This component is very similar to openmpf-projects/openmpf/trunk/detection/executor/cpp/batch/test/test_python_components/test_component.py, which is used by openmpf-projects/openmpf/trunk/detection/executor/cpp/batch/test/test_batch_executor.cpp. Properties in that test_component.py are read by those tests.

I updated the test_component.py used by the test_batch_executor.cpp in order to test feed-forward all tracks behavior.

Is your desire to keep the two test_component.py files nearly identical?

[brosenberg42]

@brosenberg42 reviewed 8 of 8 files at r2.
Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @jrobble)

---

detection/api/mpf_component_api/mpf_component_api.py line 83 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

According to this logic we never create a feed-forward all tracks request if there are no feed-forward tracks generated in the previous task:

private Optional<DetectionRequest> createFeedForwardAllTracksRequest(Media media, DetectionContext context) {
        int topQualityCount = getTopQualityCount(context);
        String topQualitySelectionProp = context.getQualitySelectionProperty();
        var tracks = _triggerProcessor.getTriggeredTracks(media, context)
                .filter(t -> {
                    if (t.getDetections().isEmpty()) {
                        log.warn("Found track with no detections. "
                                    + "No feed forward request will be created for: {}", t);
                        return false;
                    }
                    return true;
                })
                .collect(Collectors.toList());

        if (tracks.isEmpty()) {
            return Optional.empty();
        }

Ok, please change feed_forward_tracks: Optional[List[VideoTrack]] = None to feed_forward_tracks: List[VideoTrack]

---

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

The reason I set the properties you're referring to is to show the difference between feed-forward all tracks and feed-forward single tracks in the JSON output object when running this component in a Docker deployment with the WFM.

It appears that this component is only used as a developer reference. Technically, none of the properties are ever read.

This component is very similar to openmpf-projects/openmpf/trunk/detection/executor/cpp/batch/test/test_python_components/test_component.py, which is used by openmpf-projects/openmpf/trunk/detection/executor/cpp/batch/test/test_batch_executor.cpp. Properties in that test_component.py are read by those tests.

I updated the test_component.py used by the test_batch_executor.cpp in order to test feed-forward all tracks behavior.

Is your desire to keep the two test_component.py files nearly identical?

Part of the reason I made the initial comment was because I wanted you to add a unit test for the all tracks behavior. I do not see a reason for the two to be identical. If you would like to leave the properties here, that is fine.

[jrobble]

Reviewable status: 7 of 8 files reviewed, 2 unresolved discussions (waiting on @brosenberg42)

---

detection/api/mpf_component_api/mpf_component_api.py line 83 at r2 (raw file):

Previously, brosenberg42 wrote…

Ok, please change feed_forward_tracks: Optional[List[VideoTrack]] = None to feed_forward_tracks: List[VideoTrack]

Done.

---

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

Previously, brosenberg42 wrote…

Part of the reason I made the initial comment was because I wanted you to add a unit test for the all tracks behavior. I do not see a reason for the two to be identical. If you would like to leave the properties here, that is fine.

Is TEST(PythonComponentHandleTest, TestAllVideoTracksFeedForward) in /openmpf/trunk/detection/executor/cpp/batch/test/test_batch_executor.cpp sufficient?

[brosenberg42]

@brosenberg42 reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @jrobble)

---

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

Previously, jrobble (Jeff Robble) wrote…

Is TEST(PythonComponentHandleTest, TestAllVideoTracksFeedForward) in /openmpf/trunk/detection/executor/cpp/batch/test/test_batch_executor.cpp sufficient?

Yes