Check ffprobe's error logging

[brosenberg42]

Issues:

• ffmpeg hangs when processing some kinds of unsupported/corrupted media #1678

---

This change is 

[jrobble]

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

---

trunk/workflow-manager/src/main/java/org/mitre/mpf/wfm/camel/operations/mediainspection/FfprobeMetadataExtractor.java line 468 at r1 (raw file):

    private int getNumStdErrLines(BatchJob job, Media media) {
        try {
            return Integer.parseInt(_aggregateJobPropertiesUtil.getValue(

Check that the value is > 0.

---

trunk/workflow-manager/src/main/resources/workflow-properties.json line 486 at r1 (raw file):

    },
    {
        "name": "FFPROBE_STDERR_NUM_LINES",

Add this to ties-db-check-ignorable-props.json.

[jrobble]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @brosenberg42)

---

trunk/workflow-manager/src/main/java/org/mitre/mpf/wfm/camel/operations/mediainspection/FfprobeMetadataExtractor.java line 79 at r1 (raw file):

        }
        catch (MediaInspectionException e) {
            if (shouldIgnoreStdErr(job, media)) {

A MediaInspectionException will be thrown if:

1. We can't start the ffprobe process
2. We can't read the output stream
3. The exit code is not zero
4. There is content in the error stream and ffprobe.ignore.stderr is false

Even if ffprobe.ignore.stderr is true, I think we should at least log a warning message if any of these things occurs. That includes logging the decoder error lines that would otherwise have shown up in the JSON output object.

However, I then ran some more tests which lead me to believe we should always treat 1, 2, and 3 as critical errors for audio and video files. Read below.

I modified the code to return a non-zero ffprobe exit code. When running an image job it ends in COMPLETE_WITH_ERRORS and the exit code error shows up on the JSON output, as expected. Then happens when ffprobe.ignore.stderr is true or false. Right now the behavior is inconsistent between images and videos. For videos, when ffprobe.ignore.stderr is true, the job ends in COMPLETE_WITH_ERRORS due to:

"code": "UNSUPPORTED_DATA_TYPE",
"message": "The detection component does not support detection data type of UNKNOWN"

I know that end users find that confusing. TBH, this line of code seems a bit odd:

return new FfprobeMetadata(Optional.empty(), Optional.empty());

When ffprobe fails to run, meaning we don't get any meaningful JSON output, it seems like we should treat that as a critical error, at least for audio and video files. Image inspection has Tika and BufferedImage as a fallback. There are no fallback options for audio and video files.

[jrobble]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @brosenberg42)

[jrobble]

Reviewed all commit messages.
Reviewable status: 7 of 9 files reviewed, all discussions resolved (waiting on @brosenberg42)

---

trunk/workflow-manager/src/test/java/org/mitre/mpf/wfm/camelOps/TestMediaInspectionProcessor.java line 362 at r3 (raw file):
Please rename this to testInvalidVideo and the log line below to:

"Starting media inspection test with invalid video."

---

Also, please add a separate test called testVideoToUnknownFallback that uses a video container format that has no audio or video stream and actually results in the media being treated as the UNKNOWN type.

[jrobble]

Reviewable status: 7 of 9 files reviewed, 1 unresolved discussion (waiting on @brosenberg42)

[brosenberg42]

Reviewable status: 7 of 9 files reviewed, 1 unresolved discussion (waiting on @jrobble)

---

trunk/workflow-manager/src/main/java/org/mitre/mpf/wfm/camel/operations/mediainspection/FfprobeMetadataExtractor.java line 468 at r1 (raw file):

Previously, jrobble (Jeff Robble) wrote…

Check that the value is > 0.

Done.

---

trunk/workflow-manager/src/main/resources/workflow-properties.json line 486 at r1 (raw file):

Previously, jrobble (Jeff Robble) wrote…

Add this to ties-db-check-ignorable-props.json.

Done.

---

trunk/workflow-manager/src/test/java/org/mitre/mpf/wfm/camelOps/TestMediaInspectionProcessor.java line 362 at r3 (raw file):

Previously, jrobble (Jeff Robble) wrote…

Please rename this to testInvalidVideo and the log line below to:

"Starting media inspection test with invalid video."

---

Also, please add a separate test called testVideoToUnknownFallback that uses a video container format that has no audio or video stream and actually results in the media being treated as the UNKNOWN type.

Done.

[jrobble]

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