[libavg-devel] video4Linux2 on libavg: some design choices

[Ulrich von Zadow]

Hi,

I just checked out the branch here and got it to work with a firewire
camera again with minimal changes. Good work!

I've checked in the changes to the branch (some additional #ifdefs to
avoid unresolved externals when V4L isn't present on the system), and
I'm attaching a diff as well so you can see what's going on.

Some more comments are inline.

OXullo Intersecans schrieb:

>> Looks good. The 'mode' attribute seems a bit strange to me in the v2l
>> case, but I don't have any good idea how to change it.
> 
> sure? we followed your traces!
> <width>x<height>_<pixelformat>
> 
> you'll see a tokenizer on V4LCamera.cpp code to split up these two
> fields. Furthermore another tokenization between width and size seems to
> be due

It's really a minor issue, but for firewire cameras, there is a number
of fixed modes. The dimensions and the pixel format of each mode is set
by the standard for all 'normal' modes, so the string is just converted
to a mode constant directly. With V4L, you need a tokenizer etc., and I
sort of dislike adding another level of parsing when we've got xml ;-).

But it's not very important, so we'll leave it like it is.

>>> We split this method in two: addFWTracker(device, mode) and
>>> addV4LTracker(device, mode, channel). This seems not to be the best way,
>>> but was the fastest, waiting for some Uli's advices basing on his best
>>> flavours.
>>
>> Yeah, that doesn't seem like we want it to stay that way. I'll look at
>> the code over the weekend.
> 
> waiting for your suggestions!

Can't we just add a <source> node to .avgtrackerrc and an appropriate
field to TrackerConfig? Then we only have to look at that field in
addTracker and instantiate the correct camera?

I might be able to do that tomorrow, but I can't really promise much at
the moment.

>>> At this time, V4LCamera does not honour tune-in attributes, therefore
>>> the <camera> node of the rc is simply ignored.
>>
>> Are you going to implement that? If not, we should throw an error if the
>> camera node is there.
> 
> gone with the last two commits

:-)

> unsupported features (they may vary) are simply ignored with a WARNING
> you'll notice that if you instantiate a CameraNode you'll receive a
> non-fatal error of a setFeature() called before device initialization.
> Will we have to proxy feature set requests and throw them only at device
> initialization time?

libavg usually attempts to honor feature requests even if the
appropriate resource is not yet available. This works, for instance,
with video.play() or seek() called before AVGPlayer.play(). (Or if it
doesn't, that's a bug and I'll try to fix it). So yes, it would be great
if you could save feature requests and apply them when the camera
becomes available.

Ah, and I think I saw some tab characters in the source code. Could you
clean those up?

I'll see if I can check in an example .avgtrackerrc tomorrow as well.

Cheers,

  Uli
-------------- next part --------------
Index: src/player/CameraNode.cpp
===================================================================
--- src/player/CameraNode.cpp	(revision 2152)
+++ src/player/CameraNode.cpp	(working copy)
@@ -31,7 +31,9 @@
 #include "../base/XMLHelper.h"
 
 #include "../imaging/Camera.h"
+#ifdef AVG_ENABLE_V4L2
 #include "../imaging/V4LCamera.h"
+#endif
 
 #include <iostream>
 #include <sstream>
@@ -64,26 +66,31 @@
     double FrameRate = getDefaultedDoubleAttr (xmlNode, "framerate", 15);
     string sMode = getDefaultedStringAttr (xmlNode, "mode", "640x480_RGB");
     string sSource = getDefaultedStringAttr (xmlNode, "source", "firewire");
-	int Channel = getDefaultedIntAttr (xmlNode, "channel", 1);
 		
 	AVG_TRACE(Logger::APP, "CameraNode() constructor. Source=" << sSource);
 
-#if defined(AVG_ENABLE_1394) || defined(AVG_ENABLE_1394_2) || defined(AVG_ENABLE_V4L2)
 	if (sSource == "firewire")
 	{
+#if defined(AVG_ENABLE_1394) || defined(AVG_ENABLE_1394_2) || defined(AVG_ENABLE_V4L2)
 		m_pCamera = CameraPtr(new Camera(sDevice, FrameRate, sMode, true));
 		AVG_TRACE(Logger::APP, "FWCamera created");
+#else
+        AVG_TRACE(Logger::ERROR, "Firewire camera specified, but firewire support not compiled in.");
+#endif
 	}
 	else if (sSource == "v4l")
 	{
+#if defined(AVG_ENABLE_V4L2)
+	    int Channel = getDefaultedIntAttr (xmlNode, "channel", 1);
 		m_pCamera = CameraPtr(new V4LCamera(sDevice, Channel, sMode, true));
 		AVG_TRACE(Logger::APP, "V4LCamera created");
-	}
-
 #else
-    AVG_TRACE(Logger::ERROR,
-            "Unable to set up camera. Camera support not compiled.");
+        AVG_TRACE(Logger::ERROR, "Video4Linux camera specified, but Video4Linux support not compiled in.");
 #endif
+	} else {
+        AVG_TRACE(Logger::ERROR,
+            "Unable to set up camera. Camera source '"+sSource+"' unknown.");
+    }
 
     if (m_pCamera) {
         m_pCamera->setFeature ("brightness", getDefaultedIntAttr(xmlNode, "brightness", -1));
Index: src/player/Player.cpp
===================================================================
--- src/player/Player.cpp	(revision 2152)
+++ src/player/Player.cpp	(working copy)
@@ -63,7 +63,9 @@
 #include "../graphics/Rect.h"
 
 #include "../imaging/Camera.h"
+#ifdef AVG_ENABLE_V4L2
 #include "../imaging/V4LCamera.h"
+#endif
 #include "../imaging/FakeCamera.h"
 
 #include <Magick++.h>
@@ -378,7 +380,9 @@
     TrackerConfig Config;
     Config.load();
     CameraPtr pCamera;
+#ifdef AVG_ENABLE_V4L2
     pCamera = CameraPtr(new V4LCamera(sDevice, channel, sMode, false));
+#endif
     m_pTracker = new TrackerEventSource(pCamera, Config, IntPoint(m_DP.m_Width, m_DP.m_Height), true);
     m_pEventDispatcher->addSource(m_pTracker);
     return m_pTracker;
Index: src/player/TrackerCalibrator.cpp
===================================================================
--- src/player/TrackerCalibrator.cpp	(revision 2152)
+++ src/player/TrackerCalibrator.cpp	(working copy)
@@ -33,7 +33,7 @@
 
 using namespace std;
 
-#define NUM_POINTS 5 
+#define NUM_POINTS 4 
 #define MIN_DIST_FROM_BORDER 30
 //#define DEBUG_FIT  1