After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 699364 - dshowvideosink: port to 1.0
dshowvideosink: port to 1.0
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.x
Other Windows
: Normal enhancement
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-05-01 03:22 UTC by Fan, Chun-wei
Modified: 2014-12-05 08:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Port dshowvideoplugin to 1.0 and avoid deprecated GLib threading APIs (19.29 KB, patch)
2013-05-01 03:22 UTC, Fan, Chun-wei
committed Details | Review

Description Fan, Chun-wei 2013-05-01 03:22:49 UTC
Created attachment 242979 [details] [review]
Port dshowvideoplugin to 1.0 and avoid deprecated GLib threading APIs

Hi,

I have managed to make an initial attempt to port the dshowvideosink plugin to work with GStreamer-1.0, which I have manage to test with a Visual Studio 2008 build of it using "gst-launch-1.0 videotestsrc ! dshowvideosink".  In the process, I have also updated the code so that it does not use the deprecated GLib threading APIs.

I have attached my changes in the attached patch file.

With blessings, thank you!
Comment 1 Fan, Chun-wei 2013-08-15 00:11:12 UTC
Hi,

Any news or thoughts about this?
Comment 2 Edward Hervey 2013-08-15 06:11:08 UTC
Nice one.

Porting-wise it looks good and consistent. But a windows-using dev will have to confirm it's good.
Comment 3 Julien Isorce 2014-11-01 09:21:23 UTC
Maybe Jerôme you will have time to confirm the patch ?
Comment 4 Jérôme Laheurte 2014-11-03 15:20:44 UTC
Yep, works for me on Windows 7 32 bits (built with Visual 2013 Express).
Comment 5 Julien Isorce 2014-11-04 08:22:25 UTC
Review of attachment 242979 [details] [review]:

I added some remarks but it looks good. 

As a follow-up, I mean in an other bug you could use:
GstVideoFrame src;
ret = gst_video_frame_map (&src, &info, buf, GST_MAP_READ); 
It will simply "CopyToDestinationBuffer".

Also could investigate if it is possible to build a GstBufferPool of IMediaSample.
The blocking thing I see if whether or not it is possible to retrieve the dshow samples when setting up the graph (asking for N sample).
And then wrap them GstMemory. Also it raises the question if dshow can handle non-contiguous planes like V4l2.

::: sys/dshowvideosink/dshowvideofakesrc.cpp
@@ +258,3 @@
   AM_MEDIA_TYPE *mediatype;
 
+  gst_buffer_map (buffer, &map, GST_MAP_READ);

You should check return value of gst_buffer_map

::: sys/dshowvideosink/dshowvideosink.cpp
@@ +70,1 @@
 

Does it really implement navigation ?

@@ +1698,3 @@
   else if (IsEqualGUID (mediatype->subtype, MEDIASUBTYPE_UYVY))
+    caps = gst_caps_new_simple ("video/x-raw",
+            "format", GST_TYPE_VIDEO_FORMAT, GST_VIDEO_FORMAT_UYVY, NULL);

Where YUYV has gone ? Maybe you need to check whether it's available or not.

@@ +1797,3 @@
+          fourcc = GST_MAKE_FOURCC ('Y', 'U', 'Y', '2');
+          bpp = 16;
+          break;

Same question here.
Comment 6 Julien Isorce 2014-11-04 08:26:05 UTC
And "merci" Jerôme for having confirmed the patch.
Comment 7 Julien Isorce 2014-12-03 07:57:20 UTC
Short: I think we can push this patch.

I found out why YUYV handling has been removed within the patch. It seems to be a duplicate of YUY2 (cf http://www.fourcc.org/yuv.php)
And it is not here http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-gstvideo.html
Also it is not here anymore: http://msdn.microsoft.com/en-gb/library/aa924843.aspx

About the navigation interface, it is not fully implemented but worth to keep the base.

About gst_buffer_map I think the return check can be done in a separate patch. Also I can add a /* FIXME: check return value */ when merging the patch. I do not want to touch the patch more than that because it has been tested without any changes.
Comment 8 Sebastian Dröge (slomo) 2014-12-04 17:34:07 UTC
Yes, let's get that in... but maybe comment out the navigation interface parts. Julien, are you going to merge that?
Comment 9 Julien Isorce 2014-12-04 21:22:37 UTC
I have not the setup to compile it at the moment (to avoid making a mistake while removing the navigation interface part). So please go ahead.
Comment 10 Sebastian Dröge (slomo) 2014-12-04 21:24:45 UTC
Let's just merge it as is then :) A half-implemented navigation interface is better than not having this element at all.
Comment 11 Julien Isorce 2014-12-05 08:06:58 UTC
Comment on attachment 242979 [details] [review]
Port dshowvideoplugin to 1.0 and avoid deprecated GLib threading APIs

commit 788297fec36e8f8a28682b9f74dad9e96c8b8c5b
Author: Chun-wei Fan <fanchunwei@src.gnome.org>
Date:   Wed May 1 11:17:02 2013 +0800

    dshowvideosink: Port to 1.0 and new GLib threading API
    
    This updates the dshowvideosink to work with the GStreamer 1.0.x APIs, and
    avoids the use of deprecated GLib threading API that are now used since
    GLib 2.32+
    
    https://bugzilla.gnome.org/show_bug.cgi?id=699364