[Bug 732283] dshowvideosrc: Port to 1.0
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Tue Sep 16 01:19:46 PDT 2014
https://bugzilla.gnome.org/show_bug.cgi?id=732283
GStreamer | gst-plugins-bad | git
Sebastian Dröge (slomo) <slomo> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #285731|none |needs-work
status| |
--- Comment #8 from Sebastian Dröge (slomo) <slomo at coaxion.net> 2014-09-16 08:19:43 UTC ---
Review of attachment 285731:
--> (https://bugzilla.gnome.org/review?bug=732283&attachment=285731)
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? Also previously we
had a visual studio project to build this plugin :)
::: 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
@@ +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?
@@ +253,3 @@
+ gst_query_parse_caps (query, &filter);
+ if (filter) {
+ GstCaps *temp = gst_caps_intersect (caps, filter);
gst_caps_intersect_full(filter, caps, GST_CAPS_INTERSECT_FIRST)
@@ +567,2 @@
error:
+ // Don't restart the graph, we're out anyway.
Don't use C99 comments, just use /* bla */
@@ +643,3 @@
+ GstClock *clock = gst_element_get_clock (GST_ELEMENT (asrc));
+ *timestamp = gst_clock_get_time(clock);
+ gst_object_unref(clock);
You can keep *timestamp unset (i.e. GST_CLOCK_TIME_NONE as initialized by the
base class).
This here is only useful if you can get timestamps from the device/driver
somehow
@@ +726,3 @@
wavformat->wBitsPerSample, "depth", G_TYPE_INT,
wavformat->wBitsPerSample, "endianness", G_TYPE_INT, G_BYTE_ORDER,
"signed", G_TYPE_BOOLEAN, TRUE, "channels", G_TYPE_INT,
Ideally use GstAudioInfo and create the caps from that with
gst_audio_info_from_caps(). Also there are no depth, width, endianness and
signed fields in 1.x
::: sys/dshowsrcwrapper/gstdshowvideosrc.cpp
@@ +43,3 @@
+ "height = " GST_VIDEO_SIZE_RANGE ", "
+ "framerate = " GST_VIDEO_FPS_RANGE ", "
+ "systemstream = (boolean) FALSE; "
The systemstream field was only for video/x-dv, not video/x-raw
@@ +121,3 @@
gstbasesrc_class->unlock_stop =
GST_DEBUG_FUNCPTR (gst_dshowvideosrc_unlock_stop);
+ gstbasesrc_class->query = GST_DEBUG_FUNCPTR (gst_dshowvideosrc_query);
Why don't you use get_caps()?
@@ +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
@@ -288,3 @@
- if (src->buffer_cond) {
- g_cond_free (src->buffer_cond);
- src->buffer_cond = NULL;
You should clear the cond and mutex here
@@ +304,3 @@
+
+ switch (GST_EVENT_TYPE (event)) {
+ case GST_EVENT_CAPS:
You can use the set_caps vfunc here
@@ +471,3 @@
+ if (src->media_filter) {
+ // Setting this to TRUE because set_caps may be invoked before
+ // Run() returns.
No C99 comments
@@ +1035,3 @@
GST_BUFFER_DURATION (buf) = duration;
+ GstMapInfo info;
Put variable declarations at the beginning of blocks
--
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.
More information about the gstreamer-bugs
mailing list