[libavg-devel] Re: V4L2-libAVG integration

[Ulrich von Zadow]

OXullo Intersecans schrieb:
> Dear Uli,
> today we had a very first approach to V4L2 integration in libAVG.
> I should ask you some question regarding conventions and general
> approach to code merging.
> 
> First of all: do you prefer that these questions belong to mailing list
> and not only to a private thread?

I've cc'd the list :-).

> 1. Workspace
> We reaped a checkout (rev 2095) and imported the whole tree to our svn
> repository. We are supposed to work there and at last we'll give you a
> bunch of diffs for a manual and assisted integration in your head.
> We'll keep your side updated at the latest revision.

If you want to do that, that's ok, but I can also give you write access
to the libavg repository. You could just do all development on a branch.
That would probably make updating and merging a lot easier. The only
condition would be that you send me a diff before you merge to the trunk.

We could also merge more often with this setup, so I can check if any of
your changes break Firewire support. The smaller the patches, the
better. You probably don't have a FW camera, right?

> 2. autoconf / preprocessor directives
> Trying to interpret your coding standard, we approached this way:
>> configure.in: enable-v4l2 flag with header (linux/videodev2.h)
> availability test
>> src/imaging/Makefile.am conditional append of header and
> implementation of our ICamera child

Sounds good.

>> player/CameraNode.cpp, imaging/CameraUtils.cpp: we'll manage defines,
> ORring V4L2 availability and integrating some specific-level code

There shouldn't really be many #ifdefs nessesary in these files. In
CameraNode.cpp, the #ifdefs in the constructors need to stay, but the
rest... I've just checked in a new version that shouldn't need any others.

And CameraUtils should probably be renamed FWCameraUtils. It's only
there to hide away differences between different versions of libdc1394.

> PS: why only CameraUtils.(h | cpp) files are subjected to ENABLE_1394
> | ENABLE_1394_2 defines and Camera.(h | cpp) are not? Is the only reason
> the fact that Camera would be a dummy device, still instantiable? (eg:
> Player.cpp:327) 

Hm - I don't think there's a good reason Camera.(h|cpp) is still
compiled when there's no camera support at all. Maybe instantiate
DummyCamera instead and issue a warning when there's no camera support?
Then we would always have a camera object and get rid of a lot of ifs in
CameraNode as well :-).

Anyway, feel free to clean up that part of the code, it's not really ok
the way it is. (In a separate patch, if that's easy for you.)

> 3. naming conventions
> V4LCamera.(h | cpp) for V4LCamera class

Yup.

> Is CameraUtils intended to be a general purpose interface for common
> camera functions?

No - see above, it should be renamed to FWCameraUtils.

> 4. Inheritances
> As you pointed out, Camera (actual main interface to your firewire
> camera) would be changed to FWCamera.
> Both FWCamera and V4LCamera are ICamera derived classes.

Perfect :-). If any code moves to ICamera, that class would need to be
renamed to Camera.

> X. Various questions (forgive_naiveness_please!)
>> At compile time user can get or not get V4L code integration, using
> 'enable-v4l2' configure parameter: we should decide how user can switch
> between firewire and v4l camera (runtime). An example for CameraNode:
> avg file camera node can provide a specific attribute like 'fw' or
> 'v4l'. But there's also TrackerEventSource * Player::addTracker, how to
> treat this latter?

For CameraNode: <camera source="firewire|video4linux|dummy".../>

For the tracker: This belongs in .avgtrackerrc:

  <camera>
    <source value="firewire"/>
    ...
  </camera>

>> How frame per second setting is used, looking that image dequeue
> timing is handled by TrackerThread and CameraNode (doFrame, in the
> latter case?) ? Maybe firewire camera hasn't a timecode setting at all,
> in the contrary of a V4L video feed which comes usually from an analogue
> device.

The fps is a configuration setting of FW cameras (Camera.cpp, 144).

>> According to our actual knowledge of libAVG, camera video feed can be
> used as avg node (via CameraNode, instantiated by the avg file parser
> Player::loadFile) or as a tracker using Player::addTracker, right?

Yes, and you can use src/test/TestCamera.py to check if camera support
is still working. It should never crash, with or without V4L or Firewire
camera :-).

Regards,

  Uli