[Bug 678402] Device discovery/listing replacement for GstPropertyProbe

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=678402
  GStreamer | gst-plugins-base | git

--- Comment #49 from Olivier Crete (Tester) <olivier.crete at ocrete.ca> 2013-11-06 23:54:54 UTC ---
(In reply to comment #48)
> The rationale for exposing actual API for derived GstDevices wasn't primarily
> for filtering, but because I think people will want additional information from
> devices sooner or later, additional information which is probably closely tied
> to the kind of device that you're dealing with, and doing this via 'anonymous'
> GObject API is not very nice.

If we want to add additional information later, we can always create
subclasses, I don't think this is required now.

> For example, one might want medium status / type on a disc drive, mount/unmount
> or eject operations. On a video source or sink one might want to query
> available video norms if available, or if it's a tunable device. And the same
> probably applies to other devices too.

Most of those things could just be properties? Operations like eject might make
sense, but nothing prevents us from adding GstDevice subclasses later.

> I agree that we want to filter monitors. But having a hierarchical description
> of the possible device categories does not preclude that, you just have to
> register a list of hierarchy roots in the monitor "klass" rather than a
> collection of tags, for example.

We could make the "klass" into a "path".. Use use a g_str_prefix() has a
filter. So we could have klasses like "/Source/Video/Camera/V4l2/" which would
match "/Source/" or "/Source/Video/". And I guess we could put the klass into
both the filter and the gstdevice itself..

That clashes a bit with your other suggestion.. if you have a single monitor
for both pulsesrc and pulsesink, what klass would it be ?


> > >  - multiple places: why use GPtrArray over GList ? Most of our other API uses
> > > GLists I think
> > 
> > Because you can return a ref without doing a copy. But it could also be a
> > GList.
> 
> True, but you have to return a copy anyway because of writability semantics
> (some other thread of the monitor might ask the monitor to add/remove a device
> while some other code is using the reffed array), no? Not that it really
> matters, was just wondering.

Good point, GList it is.

> > > - gst_device_get_caps() - need signalling somewhere whether these
> > >   caps are likely to be the final probed caps or just template
> > >   caps (not always possible to probe for the actual caps)
> > 
> > In my mind these were always probed. I'm not sure in which case they aren't?
> 
> You seem to assume you can always probe things, and with well-behaved APIs
> that's the case. But there may be others. You might only be able to probe a
> device if it's currently not in use, for example. Of course then one could just
> return NULL which is just as well.

I guess if probing is impossible it might return some kind of template caps. I
think even probed caps can possibly be a superset of what is actually possible
right now.

> > >  - where are the keys defined? what is it good for? re.:
> > >      const gchar * gst_device_monitor_factory_get_metadata      
> > > (GstDeviceMonitorFactory *factory, const gchar *key);
> > >      gchar **      gst_device_monitor_factory_get_metadata_keys 
> > > (GstDeviceMonitorFactory *factory);
> > 
> > It's there on GstElementFactory, not sure why?
> 
> That's so element factories can store additional information there, e.g.
> something they have probed once. I don't think we need all that stuff for the
> monitor factory, we should get rid of it (along with the long name, description
> and author) until we have a use case for it IMHO (KISS).

I'd like to keep long name/desc/author so we can add an option to list monitors
to gst-inspect. But metadata can do, I guess we can re-add it later.

> > > GstGlobalDeviceMonitor:
> > > 
> > >  - not nice enough / high-level enough for my taste
> > 
> > Anything else than the class system you don't like ?
> 
> That, and the naming :) It's not really 'global' is it? IMHO
> GstGlobalDeviceMonitor should really be called GstDeviceMonitor, but then I
> don't know what GstDeviceMonitor should be called instead - maybe
> GstDeviceProbe or something. 

I'm not a super fan of the name. I don't think Probe is good either (as it's
not only probing!). Maybe the Global one could be a GstAggregatedDeviceMonitor
?

> > >  - should it be a singleton? (should we assume that one can always create
> > >    multiple monitor factories of the same kind?)
> > 
> > No, we want to be able to run separate monitors for lets say camera, audio
> > source, audio sink. It's easier for the application devs if they can just
> > receive the right events.
> 
> Making it a singleton doesn't mean you can't have individual filters.

You mean have a:
GstBus * gst_singleton_global_device_monitor_add_filter(caps, klass,
filter_func, user_data); ?
This kind of API makes it harder to extend the filtering later. If the
Aggregator is an object, then more functions/properties can be added later for
more types of filtering.

I'm not sure what the benefits of having a singleton are ?

> The way I pictured it was that the global device monitor (singleton or not)
> loads the device monitors that match the current filter requirements, and then
> it is basically a broker/filter that forwards the devices it gets from the
> various monitors to the 1-N listeners, applying the respective filters for
> each. That would require a slightly different API I guess, like passing a bus
> to each with the filters, or a callback.

Why have a single one? Maybe it makes sense to have each user create it's own
"monitor aggregator" with appropriate filters, but that the actual monitors
each be singleton that is running as long as any aggregator is running.


It might make sense if each monitor is a singleton though, if we only do the
filtering in the "global" one. Maybe this could be done with some help from the
factory (having a flag that makes the factory always return the same one if
possible).

That said, I'm not sure there is a huge benefit in having anything be
singleton. Except maybe the probing part.

> > >  - not yet convinced by the filter options - one migh want to track different
> > >    kinds of devices with the same monitor, or is the idea to use different
> > >    monitors for that?
> > 
> > The idea is to use different monitors.
> 
> But why? It doesn't seem to make much sense to me. Why not just one monitor per
> API? It makes very optimistic assumptions about the various APIs involved IMHO,
> and reading through the pulse and v4l2 implementations it seems that having one
> monitor would simplify the code a lot, and not introduce any additional
> overhead apart from creating a few devices that aren't needed, which is
> negligible.

You mean have one monitor for sinks and sources ? I could be convinced of that,
but it doesn't match well with the hierarchical model well as mentioned
earlier.

> > >  - how about some kind of _add_filter (monitor, type, caps or NULL) ?
> > 
> > And get_filter(monitor, &type, &caps) ?
> 
> Why would you ever need to get the filter?  I was thinking of it more like a
> watch - you add it, and then you can remove it if you don't need it any more,
> but there's no need to query the parameters - the app knows those anyway.

I just like getters to match setters.

> > >  - what about medium/disc inserted/ejected?
> > 
> > How is that different from added/removed ? We may want to have separate device
> > types or even separate monitors for disks drives and disks?
> 
> How would you express this? Does the inserted medium show up as a new device?
> Not sure that maps well to how applications would want to use this. I think it
> could be done as "device X changed: changetype = medium_inserted/ejected" or
> similar..

I would have the disk show up as it's own GstDevice, of a type different from
the drive device. I think GstDevice objects property should never change.
Changing properties means applications need to remember the old ones and do
diffs, and that's painful.

> > >  - is 'operation' the right word here? maybe it should be DEVICE_EVENT ?
> > 
> > Isn't "event" a bit too generic? (and meaningless?).. And well EVENT already
> > means something else in Gst.
> 
> Well, it would be an event, just a device event. As I see it, an "operation" is
> some action that's to be triggered or going on over a longer period of time,
> and device-added/removed does not seem like an operation to me. Seems easier to
> just differentiate it using different message types, if we're not restricted by
> the number of flags any more with the extended type.

Yeah, maybe we need a better name than operation or event.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.