-
Notifications
You must be signed in to change notification settings - Fork 107
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
Use cscore for PublishVideoOperation #866
base: master
Are you sure you want to change the base?
Changes from 3 commits
d16e05d
af7ebeb
1691326
86829a7
a65ffd2
40343c8
93022f7
b4bff3b
c1869f8
54da545
644cab8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,34 +10,47 @@ | |
import com.google.common.collect.ImmutableList; | ||
|
||
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; | ||
|
||
import org.bytedeco.javacpp.BytePointer; | ||
import org.bytedeco.javacpp.IntPointer; | ||
|
||
import java.io.DataInputStream; | ||
import java.io.DataOutputStream; | ||
import java.io.IOException; | ||
import java.net.ServerSocket; | ||
import java.net.Socket; | ||
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.bytedeco.javacpp.opencv_core; | ||
import org.opencv.core.Mat; | ||
|
||
import java.lang.reflect.Field; | ||
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; | ||
|
||
import static org.bytedeco.javacpp.opencv_core.Mat; | ||
import static org.bytedeco.javacpp.opencv_imgcodecs.CV_IMWRITE_JPEG_QUALITY; | ||
import static org.bytedeco.javacpp.opencv_imgcodecs.imencode; | ||
import static org.bytedeco.javacpp.opencv_core.CV_8S; | ||
import static org.bytedeco.javacpp.opencv_core.CV_8U; | ||
|
||
/** | ||
* 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. In addition, an intermediate processed image in the pipeline | ||
* could be published instead. Based on WPILib's CameraServer class: | ||
* https://github.com/robotpy/allwpilib/blob/master/wpilibj/src/athena/java/edu/wpi/first/wpilibj | ||
* /CameraServer.java | ||
* 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()); | ||
|
||
static { | ||
try { | ||
// Loading the CameraServerJNI class will load the appropriate platform-specific OpenCV JNI | ||
CameraServerJNI.getHostname(); | ||
} catch (Throwable e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can native bindings ever throw an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's most likely to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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! Exiting", e); | ||
System.exit(31); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should set a flag and make the operation throw an exception in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, we want to limit the number crashes that don't display the crash dialog. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Theres actually nothing to catch, because internally in the JNI loading classes we call System.exit(1). There would be no exception ever caught here. It would be impossible to catch. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After a little discussion, we will leave the IO Exception that the static constructor can throw in flight, that way its catchable. That will be a change on our side. We will do the same thing to ntcore as well. |
||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this need to happen statically? Can't this be done when the operation is constructed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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") | ||
|
@@ -46,110 +59,55 @@ public class PublishVideoOperation implements Operation { | |
.icon(Icon.iconStream("publish-video")) | ||
.build(); | ||
private static final int PORT = 1180; | ||
private static final byte[] MAGIC_NUMBER = {0x01, 0x00, 0x00, 0x00}; | ||
|
||
@SuppressWarnings("PMD.AssignmentToNonFinalStatic") | ||
private static int totalStepCount; | ||
@SuppressWarnings("PMD.AssignmentToNonFinalStatic") | ||
private static int numSteps; | ||
private final Object imageLock = new Object(); | ||
private final BytePointer imagePointer = new BytePointer(); | ||
private final Thread serverThread; | ||
private final InputSocket<Mat> inputSocket; | ||
private static final int MAX_STEP_COUNT = 10; // limit ports to 1180-1189 | ||
private static final Deque<Integer> availablePorts = | ||
Stream.iterate(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; | ||
@SuppressWarnings("PMD.SingularField") | ||
private volatile boolean connected = false; | ||
/** | ||
* Listens for incoming connections on port 1180 and writes JPEG data whenever there's a new | ||
* frame. | ||
*/ | ||
private final Runnable runServer = () -> { | ||
// Loop forever (or at least until the thread is interrupted). This lets us recover from the | ||
// dashboard | ||
// disconnecting or the network connection going away temporarily. | ||
while (!Thread.currentThread().isInterrupted()) { | ||
try (ServerSocket serverSocket = new ServerSocket(PORT)) { | ||
logger.info("Starting camera server"); | ||
|
||
try (Socket socket = serverSocket.accept()) { | ||
logger.info("Got connection from " + socket.getInetAddress()); | ||
connected = true; | ||
|
||
DataOutputStream socketOutputStream = new DataOutputStream(socket.getOutputStream()); | ||
DataInputStream socketInputStream = new DataInputStream(socket.getInputStream()); | ||
|
||
byte[] buffer = new byte[128 * 1024]; | ||
int bufferSize; | ||
|
||
final int fps = socketInputStream.readInt(); | ||
final int compression = socketInputStream.readInt(); | ||
final int size = socketInputStream.readInt(); | ||
|
||
if (compression != -1) { | ||
logger.warning("Dashboard video should be in HW mode"); | ||
} | ||
|
||
final long frameDuration = 1000000000L / fps; | ||
long startTime = System.nanoTime(); | ||
|
||
while (!socket.isClosed() && !Thread.currentThread().isInterrupted()) { | ||
// Wait for the main thread to put a new image. This happens whenever perform() is | ||
// called with | ||
// a new input. | ||
synchronized (imageLock) { | ||
imageLock.wait(); | ||
|
||
// Copy the image data into a pre-allocated buffer, growing it if necessary | ||
bufferSize = imagePointer.limit(); | ||
if (bufferSize > buffer.length) { | ||
buffer = new byte[imagePointer.limit()]; | ||
} | ||
imagePointer.get(buffer, 0, bufferSize); | ||
} | ||
|
||
// The FRC dashboard image protocol consists of a magic number, the size of the image | ||
// data, | ||
// and the image data itself. | ||
socketOutputStream.write(MAGIC_NUMBER); | ||
socketOutputStream.writeInt(bufferSize); | ||
socketOutputStream.write(buffer, 0, bufferSize); | ||
|
||
// Limit the FPS to whatever the dashboard requested | ||
int remainingTime = (int) (frameDuration - (System.nanoTime() - startTime)); | ||
if (remainingTime > 0) { | ||
Thread.sleep(remainingTime / 1000000, remainingTime % 1000000); | ||
} | ||
|
||
startTime = System.nanoTime(); | ||
} | ||
} | ||
} catch (IOException e) { | ||
logger.log(Level.WARNING, e.getMessage(), e); | ||
} catch (InterruptedException e) { | ||
Thread.currentThread().interrupt(); // This is really unnecessary since the thread is | ||
// about to exit | ||
logger.info("Shutting down camera server"); | ||
return; | ||
} finally { | ||
connected = false; | ||
} | ||
} | ||
}; | ||
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 static final ITable cameraPublisherTable = NetworkTable.getTable("/CameraPublisher"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you not load this statically? Instead do it at instantiation time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would that gain? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it fails then GRIP doesn't crash immediately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So instead of crashing to desktop at initialization when creating the operations list, it'll crash to desktop when adding a new PublishVideo operation. I guess that's better because at least users will see it happens when adding the operation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll also need to update NetworkTables to 4.0, but that's a different PR |
||
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 != 0) { | ||
throw new IllegalStateException("Only one instance of PublishVideoOperation may exist"); | ||
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)); | ||
numSteps++; | ||
|
||
serverThread = new Thread(runServer, "Camera Server"); | ||
serverThread.setDaemon(true); | ||
serverThread.start(); | ||
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[]{CameraServerJNI.getHostname() + ":" + ourPort + "/?action=stream"}); | ||
|
||
numSteps++; | ||
totalStepCount++; | ||
} | ||
|
||
@Override | ||
|
@@ -167,25 +125,51 @@ public List<OutputSocket> getOutputSockets() { | |
|
||
@Override | ||
public void perform() { | ||
if (!connected) { | ||
return; // Don't waste any time converting images if there's no dashboard connected | ||
} | ||
|
||
if (inputSocket.getValue().get().empty()) { | ||
final long now = System.nanoTime(); // NOPMD | ||
opencv_core.Mat input = inputSocket.getValue().get(); | ||
if (input.empty() || input.isNull()) { | ||
throw new IllegalArgumentException("Input image must not be empty"); | ||
} | ||
|
||
synchronized (imageLock) { | ||
imencode(".jpeg", inputSocket.getValue().get(), imagePointer, | ||
new IntPointer(CV_IMWRITE_JPEG_QUALITY, qualitySocket.getValue().get().intValue())); | ||
imageLock.notifyAll(); | ||
copyJavaCvToOpenCvMat(input, publishMat); | ||
serverSource.putFrame(publishMat); | ||
if (lastFrame != -1) { | ||
long dt = now - lastFrame; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen this actually be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It'll actually be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In kotlin: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, that's fine then. |
||
serverSource.setFPS((int) (1e9 / dt)); | ||
} | ||
lastFrame = now; | ||
server.setSource(serverSource); | ||
} | ||
|
||
@Override | ||
public synchronized void cleanUp() { | ||
// Stop the video server if there are no Publish Video steps left | ||
serverThread.interrupt(); | ||
numSteps--; | ||
availablePorts.addFirst(server.getPort()); | ||
ourTable.getKeys().forEach(ourTable::delete); | ||
} | ||
|
||
private void copyJavaCvToOpenCvMat(opencv_core.Mat javaCvMat, Mat openCvMat) { | ||
if (javaCvMat.depth() != CV_8U && javaCvMat.depth() != CV_8S) { | ||
throw new IllegalArgumentException("Only 8-bit depth images are supported"); | ||
} | ||
|
||
final opencv_core.Size size = javaCvMat.size(); | ||
|
||
// Make sure the output resolution is up to date | ||
serverSource.setResolution(size.width(), size.height()); | ||
|
||
// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way of creating a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SamCarlberg Ping on this? Thoughts on making sure the memory isn't inconsistently GCed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SamCarlberg Do we want to wait for that update then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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); | ||
} | ||
} | ||
} | ||
|
||
} |
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.3
with thelinux-frc
classifier 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.