Reviewing merge request #14: Implement video capture device support
What is provided:
- a method to get a video capture device list
- a method to set the desired capture device by using MediaSource.
Currently it works with V4L devices by using Phonon-VLC.
There are also changes to AudioCaptureDevices, but these are not tested.
There is a test application for video capture. It works fine on my system. I've
tested it with 2 v4l devices attached. The lines that "make contact" with the
changes are:
// The part for grabbing a video capture device list
QList<Phonon::VideoCaptureDevice> l = Phonon::BackendCapabilities::availableVideoCaptureDevices();
m_deviceModel = new Phonon::VideoCaptureDeviceModel(l, 0);
// The part for setting the current video capture device
Phonon::VideoCaptureDevice vc = m_deviceModel->modelData(mi); // mi is the model index
Phonon::MediaSource mediaSource(vc);
There are other ways of doing the job, including an ugly one (by specifying the device name directly)
These video capture changes are only implemented in the Phonon-VLC backend.
Some of the changes just uncomment things for VideoCaptureDevice and are like those for AudioOutputDevice or AudioCaptureDevice.
Summary of the changes:
------------------------------------
backendcapabilities.h --
availableVideoCaptureDevices()
signals for changed video devices
factory_p.h --
signals for changed video devices
globalconfig.h --
deviceProperties(Phonon::ObjectDescriptionType, index)
audioOutputDeviceProperties(index)
audioCaptureDeviceProperties(index)
videoCaptureDeviceProperties(index)
videoCaptureDeviceListFor(Phonon::Category)
videoCaptureDeviceFor(Phonon::Category)
phononnamespace.h.in --
added an enum for CaptureDeviceType. It can be either invalid, or V4LVideo or Audio. It is used by the MediaSource to know that it uses a capture device and act accordingly (by using deviceName). This enum may be used for other types of capture devices, for instance ones that don't have anything to do with a device name. V4L isn't platform independent, but this name can be changed because it is mostly used internally. The exception is when creating a MediaSource by providing a platform-dependent device name.
mediasource.h --
constructors for creating a media source from Phonon::AudioCaptureDevice or Phonon::VideoCaptureDevice or directly by giving Phonon::V4LVideo and /dev/videoX
added captureDeviceType, audioCaptureDevice, videoCaptureDevice to the MediaSource data
objectdescription.h --
added VideoCaptureDeviceType
- Requested by:
Casian Andrei - Status: Merged
- Source repository: soc-phonon-capture-api:master
- Target repository: phonon:master
- Created at: June 28th, 2010 16:25
Commits that would be merged:
- 59e71ff
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
59e71ff-cd288cfComments
nice, and thanks! :–)
You have some (seemingly) unrelated qsettinggroups stuff in there too, it’s not a big deal, but it would have been nice to keep separate (in the future, maybe).
thanks :D
I was modifying the code over there and I noticed some weird placed ifdef’s. Anyway next time I’ll keep the unrelated stuff separate :)
so, following the small discussion on IRC: remove the V4L from the names in phononnamespace.h.in (or move it somewhere more private), don’t remove symbols from experimental (just make it call the normal non-experimental functions), so we can preserve the ABI.
I'm starting work on the following modifications:
1. get rid of V4LVideo/Audio by using a QByteArray capture device type property for MediaSource
2. get rid of (something == “v4l”) also, by using a “type” property for a device
3. put stuff back in experimental, like you said
4. find the best solution for devices that have both video and audio
Made the modifications, but I made git push —force so I don’t know if it breaks the merge request somehow.
There are two unrelated commits in Phonon::VLC. They just add an experimental subdir.
First a general point: Keeping API in experimental doesn’t really hurt you much. To the contrary you are free to release unfinished API and then just create a second revision (and a third) until it is good – at which point it can be folded to Phonon proper. It takes a burden off you.
There are no official requirements wrt. backend support for moving to Phonon proper, but I think there should be.
Don’t remove anything from Experimental. You have to keep SC and BC. That means for example the change to phonon/experimental/objectdescription.h is a problem. Because source that uses Experimental::VideoCaptureDevice that gets recompiled changes its binary interface and would fail to link against older code (unlikely to happen because Experimental isn’t used that much – but that is not a good metric to go by).
Instead the old Experimental types would have to keep the exact same binary interface. You’re only allowed to change the implementation of non-inline functions.
phonon/experimental/tests/avcapturetest.cpp is missing a license and copyright header
Okay,
I will try to add the things in Experimental back. I already tried to do so, but I missed some of them, and others are not back the way they were. Next time I won’t remove anything from Experimental from the start. I was thinking… ahh what errors, if someone uses experimental, then he/she is competent enough to solve them, and I didn’t know for sure if Experimental was used anyway.
Now I'm working on a simple AVCapture implementation for Phonon-VLC, and I prefer to update the merge request after I finish that (next week, probably). Btw, I've kept AVCapture in Experimental.
But there is a small issue: I've renamed AvCapture to AVCapture because I was doing typos, and now I found out it’s not good. Would a #define AvCapture AVCapture work? (I don’t have enough tech skills to figure this out).
No macro hackery please. The name should really be AvCapture (to follow the Qt/kdelibs naming guidelines).
just do
$ sed -i ’s,AVCapture,AvCapture,g' *.{h,cpp}
and you’re set.
No more AVCapture. From now on it’s AvCapture in all places.
Updated the request. Noticeable improvements for the information regarding the way devices can be accessed.
Most notably, the DeviceAccessList is moved from the KDE platform plugin / kded module to Phonon, where both the plugin and the backends can use it.
Additionally, there’s no more v4l2devices file(s) in Phonon-VLC, now there’s a more generic scandevices file(s). Much prettier.
Alsa capture is implemented, but my VLC does not open alsa properly (some with no error) and so far I haven’t figured out the cause. Maybe it’s because of the PulseAudio configuration or some more hidden config somewhere. But from the Phonon and Phonon-VLC’s point of view, all is good. I will continue investigating the problem.
I will start implementing capture for other platforms in the near future. This should not be complicated.
thanks for your work! and sorry for the long delay…
Add a new comment:
Login or create an account to post a comment