WebcamPanel is not being disposed correctly, fixes #121

webcam-capture/src/example/java/ImageTransformerExample.java

@@ -1,4 +1,3 @@
-
 import java.awt.image.BufferedImage;
 
 import javax.swing.JFrame;
@@ -14,11 +13,6 @@ public class ImageTransformerExample implements WebcamImageTransformer {
 
 	private static final JHGrayFilter GRAY = new JHGrayFilter();
 
-	@Override
-	public BufferedImage transform(BufferedImage image) {
-		return GRAY.filter(image, null);
-	}
-
 	public ImageTransformerExample() {
 
 		Webcam webcam = Webcam.getDefault();
@@ -38,6 +32,11 @@ public ImageTransformerExample() {
 		window.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
 	}
 
+	@Override
+	public BufferedImage transform(BufferedImage image) {
+		return GRAY.filter(image, null);
+	}
+
 	public static void main(String[] args) {
 		new ImageTransformerExample();
 	}

webcam-capture/src/main/java/com/github/sarxos/webcam/WebcamPanel.java

@@ -224,12 +224,15 @@ public Thread newThread(Runnable r) {
 	private class ImageUpdater implements Runnable {
 
 		/**
-		 * Repainter updates panel when it is being started.
+		 * Repaint scheduler schedule panel updates.
 		 * 
 		 * @author Bartosz Firyn (sarxos)
 		 */
 		private class RepaintScheduler extends Thread {
 
+			/**
+			 * Repaint scheduler schedule panel updates.
+			 */
 			public RepaintScheduler() {
 				setUncaughtExceptionHandler(WebcamExceptionHandler.getInstance());
 				setName(String.format("repaint-scheduler-%s", webcam.getName()));
@@ -239,12 +242,14 @@ public RepaintScheduler() {
 			@Override
 			public void run() {
 
+				// do nothing when not running
 				if (!running.get()) {
 					return;
 				}
 
 				repaint();
 
+				// loop when starting, to wait for images
 				while (starting) {
 					try {
 						Thread.sleep(50);
@@ -253,7 +258,17 @@ public void run() {
 					}
 				}
 
+				// schedule update when webcam is open, otherwise schedule
+				// second scheduler execution
+
 				if (webcam.isOpen()) {
+
+					// FPS limit means that panel rendering frequency is limited
+					// to the specific value and panel will not be rendered more
+					// often then specific value
+
+					// TODO: rename FPS value in panel to rendering frequency
+
 					if (isFPSLimited()) {
 						executor.scheduleAtFixedRate(updater, 0, (long) (1000 / frequency), TimeUnit.MILLISECONDS);
 					} else {
@@ -266,20 +281,38 @@ public void run() {
 
 		}
 
+		/**
+		 * Update scheduler thread.
+		 */
 		private Thread scheduler = new RepaintScheduler();
 
+		/**
+		 * Is repainter running?
+		 */
 		private AtomicBoolean running = new AtomicBoolean(false);
 
+		/**
+		 * Start repainter. Can be invoked many times, but only first call will
+		 * take effect.
+		 */
 		public void start() {
 			if (running.compareAndSet(false, true)) {
 				executor = Executors.newScheduledThreadPool(1, THREAD_FACTORY);
 				scheduler.start();
 			}
 		}
 
-		public void stop() {
+		/**
+		 * Stop repainter. Can be invoked many times, but only first call will
+		 * take effect.
+		 * 
+		 * @throws InterruptedException
+		 */
+		public void stop() throws InterruptedException {
 			if (running.compareAndSet(true, false)) {
 				executor.shutdown();
+				executor.awaitTermination(5000, TimeUnit.MILLISECONDS);
+				scheduler.join();
 			}
 		}
 
@@ -293,18 +326,28 @@ public void run() {
 			}
 		}
 
+		/**
+		 * Perform single panel area update (repaint newly obtained image).
+		 */
 		private void update() {
 
+			// do nothing when updater not running, when webcam is closed, or
+			// panel repainting is paused
+
 			if (!running.get() || !webcam.isOpen() || paused) {
 				return;
 			}
 
+			// get new image from webcam
+
 			BufferedImage tmp = webcam.getImage();
 			if (tmp != null) {
 				errored = false;
 				image = tmp;
 			}
 
+			// and repaint it
+
 			repaint();
 		}
 	}
@@ -339,18 +382,18 @@ private void update() {
 	/**
 	 * Webcam object used to fetch images.
 	 */
-	private Webcam webcam = null;
+	private final Webcam webcam;
 
 	/**
-	 * Image currently being displayed.
+	 * Repainter is used to fetch images from camera and force panel repaint
+	 * when image is ready.
 	 */
-	private BufferedImage image = null;
+	private final ImageUpdater updater;
 
 	/**
-	 * Repainter is used to fetch images from camera and force panel repaint
-	 * when image is ready.
+	 * Image currently being displayed.
 	 */
-	private volatile ImageUpdater updater = null;
+	private BufferedImage image = null;
 
 	/**
 	 * Webcam is currently starting.
@@ -370,9 +413,12 @@ private void update() {
 	/**
 	 * Webcam has been started.
 	 */
-	private AtomicBoolean started = new AtomicBoolean(false);
+	private final AtomicBoolean started = new AtomicBoolean(false);
 
-	private Painter defaultPainter = new DefaultPainter();
+	/**
+	 * Default painter.
+	 */
+	private final Painter defaultPainter = new DefaultPainter();
 
 	/**
 	 * Painter used to draw image in panel.
@@ -385,7 +431,7 @@ private void update() {
 	/**
 	 * Preferred panel size.
 	 */
-	private Dimension size = null;
+	private Dimension defaultSize = null;
 
 	/**
 	 * Creates webcam panel and automatically start webcam.
@@ -425,11 +471,10 @@ public WebcamPanel(Webcam webcam, Dimension size, boolean start) {
 			throw new IllegalArgumentException(String.format("Webcam argument in %s constructor cannot be null!", getClass().getSimpleName()));
 		}
 
-		this.size = size;
+		this.defaultSize = size;
 		this.webcam = webcam;
-		this.webcam.addWebcamListener(this);
-
-		rb = WebcamUtils.loadRB(WebcamPanel.class, getLocale());
+		this.updater = new ImageUpdater();
+		this.rb = WebcamUtils.loadRB(WebcamPanel.class, getLocale());
 
 		addPropertyChangeListener("locale", this);
 
@@ -476,36 +521,6 @@ protected void paintComponent(Graphics g) {
 		}
 	}
 
-	@Override
-	public void webcamOpen(WebcamEvent we) {
-
-		// start image updater (i.e. start panel repainting)
-		if (updater == null) {
-			updater = new ImageUpdater();
-			updater.start();
-		}
-
-		// copy size from webcam only if default size has not been provided
-		if (size == null) {
-			setPreferredSize(webcam.getViewSize());
-		}
-	}
-
-	@Override
-	public void webcamClosed(WebcamEvent we) {
-		stop();
-	}
-
-	@Override
-	public void webcamDisposed(WebcamEvent we) {
-		webcamClosed(we);
-	}
-
-	@Override
-	public void webcamImageObtained(WebcamEvent we) {
-		// do nothing
-	}
-
 	/**
 	 * Open webcam and start rendering.
 	 */
@@ -515,25 +530,26 @@ public void start() {
 			return;
 		}
 
-		LOG.debug("Starting panel rendering and trying to open attached webcam");
-
-		starting = true;
+		webcam.addWebcamListener(this);
 
-		if (updater == null) {
-			updater = new ImageUpdater();
-		}
+		LOG.debug("Starting panel rendering and trying to open attached webcam");
 
 		updater.start();
 
+		starting = true;
+
 		try {
-			errored = !webcam.open();
+			if (!webcam.isOpen()) {
+				errored = !webcam.open();
+			}
 		} catch (WebcamException e) {
 			errored = true;
-			repaint();
 			throw e;
 		} finally {
+			repaint();
 			starting = false;
 		}
+
 	}
 
 	/**
@@ -545,19 +561,27 @@ public void stop() {
 			return;
 		}
 
+		webcam.removeWebcamListener(this);
+
 		LOG.debug("Stopping panel rendering and closing attached webcam");
 
-		updater.stop();
-		updater = null;
+		try {
+			updater.stop();
+		} catch (InterruptedException e) {
+			throw new RuntimeException(e);
+		}
 
 		image = null;
 
 		try {
-			errored = !webcam.close();
+			if (webcam.isOpen()) {
+				errored = !webcam.close();
+			}
 		} catch (WebcamException e) {
 			errored = true;
-			repaint();
 			throw e;
+		} finally {
+			repaint();
 		}
 	}
 
@@ -681,6 +705,10 @@ public boolean isFillArea() {
 		return fillArea;
 	}
 
+	public Painter getDefaultPainter() {
+		return defaultPainter;
+	}
+
 	@Override
 	public void propertyChange(PropertyChangeEvent evt) {
 		Locale lc = (Locale) evt.getNewValue();
@@ -689,7 +717,29 @@ public void propertyChange(PropertyChangeEvent evt) {
 		}
 	}
 
-	public Painter getDefaultPainter() {
-		return defaultPainter;
+	@Override
+	public void webcamOpen(WebcamEvent we) {
+
+		// if default size has not been provided, then use the one from webcam
+		// device (this will be current webcam resolution)
+
+		if (defaultSize == null) {
+			setPreferredSize(webcam.getViewSize());
+		}
+	}
+
+	@Override
+	public void webcamClosed(WebcamEvent we) {
+		stop();
+	}
+
+	@Override
+	public void webcamDisposed(WebcamEvent we) {
+		stop();
+	}
+
+	@Override
+	public void webcamImageObtained(WebcamEvent we) {
+		// do nothing
 	}
 }