[Bug 732283] dshowvideosrc: Port to 1.0

[GStreamer (bugzilla.gnome.org)]

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

--- Comment #10 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-09-16 11:41:19 UTC ---
(In reply to comment #9)
> (In reply to comment #8)
> > Review of attachment 285731 [details] [details]:
> > 
> > Thanks, looks generally good :) Just some comments below
> > 
> > ::: sys/dshowsrcwrapper/BUILD.txt
> > @@ +18,3 @@
> > +installed in <SDK>/Samples/multimedia/directshow/baseclasses. Just
> > +open the SLN and build both Debug and Release (NOT the MBCS versions
> > +though).
> > 
> > Why can't we build them together with the plugin directly?
> 
> Well, it's a third party static library. I can build it as part of the plugin
> but
> it kind of defeats the purpose of a library.

Sure, but it requires to build stuff manually via msbuild.exe or visual studio
:)

> > Also previously we
> > had a visual studio project to build this plugin :)
> 
> I noticed that. Actually, I noticed three different Visual solutions with some
> missing project
> files, none of them being intended for the version of Visual I use. The big
> advantage of CMake
> IMO is that you can generate a solution for whatever version of Visual you
> intend to use, and
> only have to maintain a text file. That, and avoid messing with the IDE, which
> is a PITA.

Agreed

> > ::: sys/dshowsrcwrapper/gstdshowaudiosrc.cpp
> > @@ +40,3 @@
> > +        " }, "
> > +        "rate = " GST_AUDIO_RATE_RANGE ", "
> > +        "channels = " GST_AUDIO_CHANNELS_RANGE)
> > 
> > Previously this only had channels=[1,2], not the full range
> 
> Right. But I just noticed DirectShow knows about audio with more than 2
> channels, so I'll implement that instead.

For more than 2 channels you'll have to worry about setting the correct
channel-mask though. I would do the >2 channel support in a separate patch
later.

> > @@ +98,3 @@
> >        GST_DEBUG_FUNCPTR (gst_dshowaudiosrc_get_property);
> > 
> > +  gstbasesrc_class->query = GST_DEBUG_FUNCPTR (gst_dshowaudiosrc_query);
> > 
> > Why don't you use get_caps() here?
> 
> I was under the impression that caps negotiation used queries and events now,
> and that get/set caps methods were deprecated. If not so, I'm fixing this.

They're not deprecated :) The only difference here to 0.10 is that all these
are internally implemented as queries/events

> > @@ +177,3 @@
> >    CoInitializeEx (NULL, COINIT_MULTITHREADED);
> > 
> > +  gst_pad_set_event_function (GST_BASE_SRC_PAD (src), GST_DEBUG_FUNCPTR
> > (gst_dshowvideosrc_event));
> > 
> > And here you could use the event vfunc
> 
> If I use the set_caps vfunc there will be no need for an event method any more
> isn't it ?

Yes

-- 
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.