Skip to content
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

Customizable WebcamMotionDetector, WebcamUpdater, FsWebcamDevice #307

Open
krok32 opened this issue Feb 10, 2015 · 4 comments
Open

Customizable WebcamMotionDetector, WebcamUpdater, FsWebcamDevice #307

krok32 opened this issue Feb 10, 2015 · 4 comments

Comments

@krok32
Copy link
Contributor

krok32 commented Feb 10, 2015

I'd like to implement further changes.
All of them should be backward compatible and allow customization.

1] WebcamMotionDetector
Currently, I can only override whole class WebcamMotionDetector, but fields are private, so it would be pain. Also WebcamMotionDetector implemens Runner and Inverter which i don't want to re-implement.

I suggest:
Algorithm for motion detection would be customizable, so part of the detect() method would be moved to the DefaultDetectorAlgorithm implementing IDetectorAlgorithm

I'm considering following methods:
BufferedImage IDetectorAlgorithm.prepareImage(BufferedImage original) // blur, gray, resize, etc...
boolean IDetectorAlgorithm.detect(BufferedImage previousModified, BufferedImage currentModified);
Poing IDetectorAlgorithm.getCog();
double IDetectorAlgorithm.getArea();

2] WebcamUpdater
Currently, I can't modify delay, and since I'm running it on RasPI, it's pretty slow and delay is always 0.

I suggest:
Part of the method WebcamUpdater.tick() would be moved to new class DefaultDelayCalculator implementing IDelayCalculator, so another implementation may be specified as parameter of Webcam.open() method.

Following method should be enough...
long IDelayCalculator.calculateDelay(snapshotDuration, deviceFps)

3] FsWebcamDevice

fswebcam needs some specific parameters to work reliably on RasPI with some cheap webcams.
If also some other drivers needed parameters, some common method
setParameters(Map<String,Object>) would be helpful.
I'm not sure about this. Anyway, I would commit configurable FsWebcamDevice if I knew how to parametrize it properly.

Please let me know what you thing about it, so I can implement it.

@sarxos
Copy link
Owner

sarxos commented Feb 11, 2015

Hi @krok32,

These are indeed very useful changes 👍 Please go ahead and do what you want to do.

When you will be doing your changes please push them in separate pull requests (for motion detector, webcam updater and fswebcam driver).

Just my few cents, so we keep the code style consistent:

  1. If this is possible, please avoid the "I" interface naming convention so we have DetectorAlgorithm instead of IDetectorAlgorithm and DelayCalculator instead of IDelayCalculator.
  2. Please make DefaultDetectorAlgorithm and DetectorAlgorithm public inner class/interface of the WebcamMotionDetector, or rather, if you want to have them separated in package (which is also good), please add the Webcam prefix at the beginning of class name.
  3. In regards to device specific parameters my first though is to make possibility to set them thru WebcamDriver impl. In such a case the parameters map would be common for all devices created by given driver. I'm not sure if this is what you desire or not. Something like:
class FsWebcamDevice {
   final Map<String, ?> parameters;
   FsWebcamDevice(File vfile, Map<String, ?> parameters) {
      this.parameters = parameters;
   }
   ... 
}
class FsWebcamDriver {
   Map<String, ?> parameters = ...
   public FsWebcamDriver(Map<String, ?> parameters) {
      this.parameters = parameters;
   }
   @Override
   public List<WebcamDevice> getDevices() {
      for (File vfile : VFFILTER.getVideoFiles()) {
         devices.add(new FsWebcamDevice(vfile, parameters));
      }
   }
   ...
}

The source code indents are made by tabs. If you want you can reuse formatter.xml (from project root) which can be imported to the Eclipse or IntelliJ to perform automated code formatting on save action.

krok32 added a commit to krok32/webcam-capture that referenced this issue Feb 13, 2015
sarxos added a commit that referenced this issue Feb 14, 2015
Customizable WebcamUpdater; issue #307
@krok32
Copy link
Contributor Author

krok32 commented Feb 15, 2015

Regarding the device parameters, i suggest these changes

  • to modify Webcam.open() method like this:
    public boolean open(boolean async, DelayCalculator delayCalculator, Map<String, ?> deviceParameters)
  • add method to the interface WebcamDevice
    WebcamDevice.setParameters(Map<String, ?> deviceParameters)
    ... And add empty implemementation to all your drivers.
    It's not compatible change, so alternatively, i can add some
    interface ConfigurableDevice {
    void setParameters(Map<String, ?> deviceParameters)
    }

@sarxos
Copy link
Owner

sarxos commented Feb 15, 2015

Hi @krok32,

Adding new interface ConfigurableDevice or simply Configurable is imho a better option than modifying WebcamDevice interface. This new interface could be public static inner interface of WebcamDevice, same as BufferAccess and FPSSource are.

Except the Webcam.open(boolean async, DelayCalculator delayCalculator, Map<String, ?> params) please consider adding Webcam.setParameters(Map<String, ?> params), so one could set parameters before the open(..) is invoked.

public void setParameters(Map<String, ?> parameters) {
  WebcamDevice device = getDevice();
  if (device instanceof Configurable) {
    ((Configurable) device).setParameters(parameters);
  } else {
    LOG.debug("Webcam device {} is not configurable", device);
  }
}

And, of course, your new open(..) signature would be still added where setParameters(..) is invoked:

public boolean open(boolean async, DelayCalculator delayCalculator,Map<String, ?> parameters) {
  if (open.compareAndSet(false, true)) {
    setParameters(parameters);
    ...
  }
  ...
}

Please let me know if this fit into your idea and needs.

I hope you don't mind if I add @author Martin Krok (krok32) annotation on the classes/interfaces you created?

@krok32
Copy link
Contributor Author

krok32 commented Feb 16, 2015

Ok,
I'll add

  • Configurable interface
  • Webcam.setParameters(Map<String, ?> params) method
    It would be fine for me, so I don't need new open() method.
    Annotation is also ok for me.
    Thanks.

krok32 added a commit to krok32/webcam-capture that referenced this issue Feb 16, 2015
sarxos added a commit that referenced this issue Feb 16, 2015
Passing parameters to WebcamDevice instances, issue #307
krok32 added a commit to krok32/webcam-capture that referenced this issue Mar 23, 2015
sarxos added a commit that referenced this issue Mar 31, 2015
Parametrized FsWebcamDriver and FsWebcamDevice issue #307
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants