Ok, since I'm an open-source developper myself, I'm pretty conscious just posting the report won't get me the feature ;-)

Here are a few pointers on what should be done, with indications how when I know.

(1) the current dv1394src doesn't provide the user-friendly name

This doesn't look that hard to add : there's already a call to rom1394_get_directory in the node discovery. It's just a matter of using rom_dir.label and storing it somewhere (a "device-name" property sounds nice).

(2) The current dv1394src doesn't have PropertyProbe introspection

I don't know exactly how this works, but I guess factoring out the current detection code would help here : the property probing should be able to return a list of either directly the port, or the ports as-uri.

Does that sound sane ?

Created attachment 74208 [details] [review]
Header part of adding the new GObject property

Hopefully that header part is ok :-)

Created attachment 74209 [details] [review]
Implementation part of the new GObject property

As can be seen, there's a place where I should probably add some more code. But at least the fundamentals should be right.

I have two questions on the current code of dv1394src :
(1) why are strings freed in _dispose? They should be in _finalize.
(2) how come rom1394_free_directory is never called although rom1394_get_directory is?

Hmmmm... could a gstreamer developper answer my questions please? I'm willing to do most of the work, but some help will definitely be needed.

> I have two questions on the current code of dv1394src :
> (1) why are strings freed in _dispose? They should be in _finalize.

For all practical purposes, it doesn't really matter (as long as you keep in mind that dispose may be called multiple times). It's probably just that someone copy'n'pasted it from elsewhere. Feel free to fix and send a patch.


> (2) how come rom1394_free_directory is never called although
> rom1394_get_directory is?

(Disclaimer: I'm not familiar with the code or the library) It might just be an oversight/leak. You can verify this by running an application (or gst-launch-0.10 pipeline containing the element and triggering the code path in question) in valgrind with --leak-check=yes and see if it reports this as leak, for example. Please fix and send a patch if you find there's a leak :)

Thanks for your answer.

I'll probably try to make some patches in the future, if I find the time.

What about the existing patches ?

Which of the copyright holders should I bug (pun intended) to discuss about this all ? :-)

> What about the existing patches ?

Ah, sorry, missed the fact that no one has looked at them yet.

Looks good mostly, but the device-name property should probably be read-only instead of read-write. Also, you'd typically do something like g_object_notify (G_OBJECT (obj), "device-name"); when you set or change the device name from within the code.

 
> Which of the copyright holders should I bug (pun intended) to discuss about
> this all ? :-)

None of them really, at least not directly. The best place to discuss issues like this is in bugzilla (for specific issues/items/patches) or on the gstreamer-devel mailing list (for more general things). If you don't get an answer in bugzilla, it's fine to 'ping' again after a week or two without reply (either in bugzilla or on IRC).

Can't help you with the above issue (2) I'm afriad, since I don't know anything about this stuff from the library/device point of view. Just give it a try and see how far you get; you can always implement it independently first and re-factor the code later.

Thanks, I'll try to have a look.

Created attachment 82387 [details]
Stupid test to print the device name

This primitive program just prints the name of the default dv device.

Created attachment 82388 [details] [review]
Implementation part of the new GObject property

This patch works with my test program, makes the property readonly, and calls

... rom1394_free_directory after a successful rom1394_get_directory.

Sorry for the incomplete comment, my fingers slipped.

Maybe I should explicitly say that I'm waiting for a review for the header&implementation of the new property : bugzilla doesn't have a "needs-review" state, it seems :-)

Then I'll try to implement the property probe which is the goal of this report.

I'll attach the implementation of the property probing ; unfortunately my tests show that the patch which adds the device-name property isn't good enough yet : it doesn't initialize the property when the guid is changed -- which means I can't have the name before I really use the element.

Changing the status of the patch to reflect this.

Created attachment 83027 [details]
Header file for the property probe

Created attachment 83028 [details]
Implementation file for the property probe

Created attachment 83029 [details] [review]
Patch to build the property probe

Created attachment 83030 [details] [review]
Patch to enable the property probe

Created attachment 83031 [details]
Better test to test the property probing feature

Created attachment 83041 [details] [review]
Implementation part of the new GObject property

This new version adds a new function, which is called when the guid property is set : this makes sure that the test runs perfectly.

What should I do to make this code go in ?

Created attachment 87468 [details] [review]
The patch in a single file

This is the otherwise unmodified patch, but in a single file which is easier to apply (tested against cvs today May 3rd 2007). It seems to work fine for me with a Canopus ADVC55 but not with an ADVC700 for which "Default" is always returned. I don't know if this is a limitation of the ADVC700 or a problem with the code, which I haven't even read.

Could this bug be fixed before the next set of releases?

Committed with small changes:

 2007-11-14  Tim-Philipp Müller  <tim at centricular dot net>

	Patch by: Julien Puydt <julien dot puydt at laposte net>

	* ext/raw1394/Makefile.am:
	* ext/raw1394/gst1394probe.c: (gst_1394_get_guid_array),
	  (gst_1394_property_probe_get_properties),
	  (gst_1394_property_probe_probe_property),
	  (gst_1394_property_probe_needs_probe),
	  (gst_1394_property_probe_get_values),
	  (gst_1394_property_probe_interface_init),
	  (gst_1394_type_add_property_probe_interface):
	* ext/raw1394/gst1394probe.h: (GST_1394_PROBE_H):
	* ext/raw1394/gstdv1394src.c: (_do_init), (gst_dv1394src_class_init),
	  (gst_dv1394src_init), (gst_dv1394src_dispose),
	  (gst_dv1394src_set_property), (gst_dv1394src_get_property),
	  (gst_dv1394src_discover_avc_node), (gst_dv1394src_query),
	  (gst_dv1394src_update_device_name):
	* ext/raw1394/gstdv1394src.h:
	  Implement GstPropertyProbe interface and add "device-name" property,
	  so applications can use this to probe for available devices in the
	  same way they can already with v4lsrc and v4l2src (however horrible
	  this property probe interface may be). Fixes #358841.

(renamed new files to make the filename slighty shorter; renamed _find_device_name() to _update_device_name(); added rom1394_free_directory() in update_device_name() and some additional checks/logging; removed FIXME in gst_dv1394src_discover_avc_node() - calling update_device_name() shouldn't be necessary there, because that branch is executed only if a GUID has been set explicitly, at which point the name would already have been updated via the set_property function).

Please test version in CVS to make sure I didn't mess anything up.

Thanks a lot for the patch, and sorry for the long wait.