Use cscore for PublishVideoOperation #866
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,146 +10,121 @@ | |
| import com.google.common.collect.ImmutableList; | ||
|
|
||
| import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; | ||
| import edu.wpi.cscore.CameraServerJNI; | ||
| import edu.wpi.cscore.CvSource; | ||
| import edu.wpi.cscore.MjpegServer; | ||
| import edu.wpi.cscore.VideoMode; | ||
| import edu.wpi.first.wpilibj.networktables.NetworkTable; | ||
| import edu.wpi.first.wpilibj.tables.ITable; | ||
|
|
||
| import org.apache.commons.lang.SystemUtils; | ||
| import org.bytedeco.javacpp.opencv_core; | ||
| import org.opencv.core.Mat; | ||
|
|
||
| import java.io.BufferedReader; | ||
| import java.io.IOException; | ||
| import java.io.InputStreamReader; | ||
| import java.lang.reflect.Field; | ||
| import java.util.Arrays; | ||
| import java.util.Deque; | ||
| import java.util.LinkedList; | ||
| import java.util.List; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
|
|
||
| /** | ||
| * Publish an M-JPEG stream with the protocol used by SmartDashboard and the FRC Dashboard. This | ||
| * allows FRC teams to view video streams on their dashboard during competition even when GRIP has | ||
| * exclusive access to the camera. Uses cscore to host the image streaming server. | ||
| */ | ||
| public class PublishVideoOperation implements Operation { | ||
|
|
||
| private static final Logger logger = Logger.getLogger(PublishVideoOperation.class.getName()); | ||
|
|
||
| /** | ||
| * Flags whether or not cscore was loaded. If it could not be loaded, the MJPEG streaming server | ||
| * can't be started, preventing this operation from running. | ||
| */ | ||
| private static final boolean cscoreLoaded; | ||
|
|
||
| static { | ||
| boolean loaded; | ||
| try { | ||
| // Loading the CameraServerJNI class will load the appropriate platform-specific OpenCV JNI | ||
| CameraServerJNI.getHostname(); | ||
| loaded = true; | ||
| } catch (Throwable e) { | ||
|
There was a problem hiding this comment. Can native bindings ever throw an There was a problem hiding this comment. It's most likely to be There was a problem hiding this comment. Unless we explicitly throw interrupted from the JNI level, nothing in JNI otherwise would throw it. And since its a dumb idea to interrupt a thread anyway, we would never throw it internally. Pretty much all that shows up is UnsatisfiedLinkError, or one of the few runtime exceptions we throw. Note this should be using forceLoad(), not getHostname() (I don't think we had forceLoad() then), but I will my sure my version does that. |
||
| logger.log(Level.SEVERE, "CameraServerJNI load failed!", e); | ||
| loaded = false; | ||
| } | ||
| cscoreLoaded = loaded; | ||
| } | ||
|
There was a problem hiding this comment. Does this need to happen statically? Can't this be done when the operation is constructed? There was a problem hiding this comment. Then the cscore loading check would happen every time a new operation is created instead of only once, when the class loads. I'd prefer to just have a flag set so the operation can be created normally and just throw an exception in |
||
|
|
||
| public static final OperationDescription DESCRIPTION = | ||
| OperationDescription.builder() | ||
| .name("Publish Video") | ||
| .summary("Publish an M_JPEG stream to the dashboard.") | ||
| .category(OperationDescription.Category.NETWORK) | ||
| .icon(Icon.iconStream("publish-video")) | ||
| .build(); | ||
| private static final int INITIAL_PORT = 1180; | ||
| private static final int MAX_STEP_COUNT = 10; // limit ports to 1180-1189 | ||
|
|
||
|
|
||
| @SuppressWarnings("PMD.AssignmentToNonFinalStatic") | ||
| private static int totalStepCount; | ||
| @SuppressWarnings("PMD.AssignmentToNonFinalStatic") | ||
| private static int numSteps; | ||
| private static final Deque<Integer> availablePorts = | ||
| Stream.iterate(INITIAL_PORT, i -> i + 1) | ||
| .limit(MAX_STEP_COUNT) | ||
| .collect(Collectors.toCollection(LinkedList::new)); | ||
|
|
||
| private final InputSocket<opencv_core.Mat> inputSocket; | ||
| private final InputSocket<Number> qualitySocket; | ||
| private final MjpegServer server; | ||
| private final CvSource serverSource; | ||
|
|
||
| // Write to the /CameraPublisher table so the MJPEG streams are discoverable by other | ||
| // applications connected to the same NetworkTable server (eg Shuffleboard) | ||
| private final ITable cameraPublisherTable = NetworkTable.getTable("/CameraPublisher"); // NOPMD | ||
| private final ITable ourTable; | ||
| private final Mat publishMat = new Mat(); | ||
| private long lastFrame = -1; | ||
|
|
||
| @SuppressWarnings("JavadocMethod") | ||
| @SuppressFBWarnings(value = "ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD", | ||
| justification = "Do not need to synchronize inside of a constructor") | ||
| public PublishVideoOperation(InputSocket.Factory inputSocketFactory) { | ||
| if (numSteps >= MAX_STEP_COUNT) { | ||
| throw new IllegalStateException( | ||
| "Only " + MAX_STEP_COUNT + " instances of PublishVideoOperation may exist"); | ||
| } | ||
| this.inputSocket = inputSocketFactory.create(SocketHints.Inputs.createMatSocketHint("Image", | ||
| false)); | ||
| this.qualitySocket = inputSocketFactory.create(SocketHints.Inputs | ||
| .createNumberSliderSocketHint("Quality", 80, 0, 100)); | ||
|
|
||
| if (cscoreLoaded) { | ||
| int ourPort = availablePorts.removeFirst(); | ||
|
|
||
| server = new MjpegServer("GRIP video publishing server " + totalStepCount, ourPort); | ||
| serverSource = new CvSource("GRIP CvSource " + totalStepCount, | ||
| VideoMode.PixelFormat.kMJPEG, 0, 0, 0); | ||
| server.setSource(serverSource); | ||
|
|
||
| ourTable = cameraPublisherTable.getSubTable("GRIP-" + totalStepCount); | ||
| ourTable.putStringArray("streams", | ||
| new String[]{"mjpeg:http://" + getHostName() + ":" + ourPort + "/?action=stream"}); | ||
| } else { | ||
| server = null; | ||
| serverSource = null; | ||
| ourTable = null; | ||
| } | ||
|
|
||
| numSteps++; | ||
| totalStepCount++; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -167,25 +142,109 @@ public List<OutputSocket> getOutputSockets() { | |
|
|
||
| @Override | ||
| public void perform() { | ||
| final long now = System.nanoTime(); // NOPMD | ||
|
|
||
| if (!cscoreLoaded) { | ||
| throw new IllegalStateException( | ||
| "cscore could not be loaded. The image streaming server cannot be started."); | ||
|
|
||
| } | ||
|
|
||
| opencv_core.Mat input = inputSocket.getValue().get(); | ||
| if (input.empty() || input.isNull()) { | ||
| throw new IllegalArgumentException("Input image must not be empty"); | ||
| } | ||
|
|
||
| copyJavaCvToOpenCvMat(input, publishMat); | ||
| // Make sure the output resolution is up to date. Might not be needed, depends on cscore updates | ||
| serverSource.setResolution(input.size().width(), input.size().height()); | ||
| serverSource.putFrame(publishMat); | ||
| if (lastFrame != -1) { | ||
| long dt = now - lastFrame; | ||
|
There was a problem hiding this comment. I've seen this actually be There was a problem hiding this comment. It'll actually be There was a problem hiding this comment.
There was a problem hiding this comment. In kotlin: There was a problem hiding this comment. It's technically infinite since the new frame comes at the same time as the previous one. But it doesn't happen here because it's rate limited by the pipeline and the update rate of the pipeline sources. I seriously doubt |
||
| serverSource.setFPS((int) (1e9 / dt)); | ||
| } | ||
| lastFrame = now; | ||
| } | ||
|
|
||
| @Override | ||
| public synchronized void cleanUp() { | ||
| numSteps--; | ||
| if (cscoreLoaded) { | ||
| availablePorts.addFirst(server.getPort()); | ||
| ourTable.getKeys().forEach(ourTable::delete); | ||
| serverSource.setConnected(false); | ||
| serverSource.free(); | ||
| server.free(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Copies the data from a JavaCV Mat wrapper object into an OpenCV Mat wrapper object so it's | ||
| * usable by the {@link CvSource} for this operation. | ||
| * | ||
| * <p>Since the JavaCV and OpenCV bindings both target the same native version of OpenCV, this is | ||
| * implemented by simply changing the OpenCV Mat's native pointer to be the same as the one for | ||
| * the JavaCV Mat. This prevents memory copies and resizing/reallocating memory for the OpenCV | ||
| * wrapper to fit the source image. Updating the pointer is a simple field write (albeit via | ||
| * reflection), which is much faster and easier than allocating and copying byte buffers.</p> | ||
| * | ||
| * <p>A caveat to this approach is that the memory layout used by the OpenCV binaries bundled with | ||
| * both wrapper libraries <i>must</i> be identical. Using the same OpenCV version for both | ||
| * libraries should be enough.</p> | ||
| * | ||
| * @param javaCvMat the JavaCV Mat wrapper object to copy from | ||
| * @param openCvMat the OpenCV Mat wrapper object to copy into | ||
| * @throws RuntimeException if the OpenCV native pointer could not be set | ||
| */ | ||
| private static void copyJavaCvToOpenCvMat(opencv_core.Mat javaCvMat, Mat openCvMat) | ||
|
There was a problem hiding this comment. Can you add some unit tests for this function to make sure that it gets covered. I don't want someone in the future to be updating dependencies and have this break because this code doesn't get run by a test. |
||
| throws RuntimeException { | ||
| // Make the OpenCV Mat object point to the same block of memory as the JavaCV object. | ||
| // This requires no data transfers or copies and is O(1) instead of O(n) | ||
| if (javaCvMat.address() != openCvMat.nativeObj) { | ||
| try { | ||
| Field nativeObjField = Mat.class.getField("nativeObj"); | ||
| nativeObjField.setAccessible(true); | ||
| nativeObjField.setLong(openCvMat, javaCvMat.address()); | ||
|
There was a problem hiding this comment. This seems like a terrible idea, but I understand what you are doing here. This is taking the memory address of the OpenCV mat and basically telling JavaCV to use it's memory instead. There was a problem hiding this comment. First it'd say, what is missing from JavaCV that you need to use the other set of bindings? IMO, we should fix that so we don't need to do things like that. And even when that's necessary, we don't need to use reflection. Just passing the pointer address around is sufficient, as long as you make sure they are from the same version of OpenCV, compiled with the same compiler, the same flags, etc and to make sure memory isn't prematurely deallocated and that it gets deallocated by the same binaries used to allocate it. There was a problem hiding this comment. Is there a way of creating a There was a problem hiding this comment. @SamCarlberg Ping on this? Thoughts on making sure the memory isn't inconsistently GCed? There was a problem hiding this comment. See my comment at the bottom. In the rewrite, all the memory mapping between different opencv versions will be gone. There will be no wpilib opencv interface anymore. And the API designed is allocation free. There was a problem hiding this comment. As an example, The following class would be added to GRIP. It will use the JavaCV version of OpenCV, and always copy from the raw image into the requested Mat. It is memory safe, and does not depend at all on the internal representation of how an OpenCV Mat looks. There's some tricks on the cscore jni side to avoid allocations when needed, but everything is in spec, no reflection or undefined behavior. This would be what the source side looks like We internally copy the pointer, so again not depending on any memory issues, or native layouts. There was a problem hiding this comment. @SamCarlberg Do we want to wait for that update then? There was a problem hiding this comment. I think so. Thad's offered to do do the rewrite with the new cscore stuff. Maybe we should also use the cameraserver library for doing the stream hosting instead of manually doing the Networktable side There was a problem hiding this comment. I wouldn't be able to bring it in directly (because of the opencv dependency) but I could bring in most of it. There's a lot extra we wouldnt need there as well. There was a problem hiding this comment. I wouldn't be able to bring it in directly (because of the opencv dependency) but I could bring in most of it. There's a lot extra we wouldnt need there as well. |
||
| } catch (ReflectiveOperationException e) { | ||
| logger.log(Level.WARNING, "Could not set native object pointer", e); | ||
| throw new RuntimeException("Could not copy the image", e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Multi platform method for getting the hostname of the local computer. cscore's | ||
| * {@link CameraServerJNI#getHostname() getHostName() function} only works on Linux, so we need to | ||
| * implement the method for Windows and Mac ourselves. | ||
| */ | ||
| private static String getHostName() { | ||
|
There was a problem hiding this comment. Replace with: There was a problem hiding this comment. Oooh. That's much better 👍 |
||
| if (SystemUtils.IS_OS_WINDOWS) { | ||
| // Use the Windows `hostname` command-line utility | ||
| // This will return a single line of text containing the hostname, no parsing required | ||
| ProcessBuilder builder = new ProcessBuilder("hostname"); | ||
| Process hostname; | ||
| try { | ||
| hostname = builder.start(); | ||
| } catch (IOException e) { | ||
| logger.log(Level.WARNING, "Could not start hostname process", e); | ||
| return ""; | ||
| } | ||
| try (BufferedReader in = | ||
| new BufferedReader(new InputStreamReader(hostname.getInputStream()))) { | ||
| return in.readLine() + ".local"; | ||
| } catch (IOException e) { | ||
| logger.log(Level.WARNING, "Could not read the hostname process output", e); | ||
| return ""; | ||
| } | ||
| } else if (SystemUtils.IS_OS_LINUX) { | ||
| // cscore already defines it for linux | ||
| return CameraServerJNI.getHostname(); | ||
| } else if (SystemUtils.IS_OS_MAC) { | ||
| // todo | ||
| return "TODO-MAC"; | ||
| } else { | ||
| throw new UnsupportedOperationException( | ||
| "Unsupported operating system " + System.getProperty("os.name")); | ||
| } | ||
| } | ||
|
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out because there's no
opencv-3.2.0-1.3with thelinux-frcclassifier and I'm worried about different memory models between binding versions (JavaCV with 3.0.0, OpenCV with 3.2.0) . This could get removed entirely if we remove the deploy functionality.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this commented out as it is is the deploy functionality essentially nerfed? If so, then we should remove the deployer for this season.