[PATCH v2 1/5] winegstreamer: Stop creating the filter if gstreamer fails to render.
Otherwise the push_data() thread will run and try to use resources we never allocated. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index a19c64f..b48648c 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1103,7 +1103,6 @@ static void unknown_type(GstElement *bin, GstPad *pad, GstCaps *caps, gpointer u static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTIES *props) { GSTImpl *This = (GSTImpl*)pPin->pin.pinInfo.pFilter; - HRESULT hr; int ret, i; LONGLONG avail, duration; GstStaticPadTemplate src_template = GST_STATIC_PAD_TEMPLATE( @@ -1163,19 +1162,20 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI WaitForSingleObject(This->event, -1); gst_element_get_state(This->container, NULL, NULL, -1); - if (!This->cStreams) { + if (!This->cStreams) + { FIXME("GStreamer could not find any streams\n"); - hr = E_FAIL; - } else { - gst_pad_query_duration(This->ppPins[0]->their_src, GST_FORMAT_TIME, &duration); - for (i = 0; i < This->cStreams; ++i) { - This->ppPins[i]->seek.llDuration = This->ppPins[i]->seek.llStop = duration / 100; - This->ppPins[i]->seek.llCurrent = 0; - if (!This->ppPins[i]->seek.llDuration) - This->ppPins[i]->seek.dwCapabilities = 0; - WaitForSingleObject(This->ppPins[i]->caps_event, -1); - } - hr = S_OK; + return E_FAIL; + } + + gst_pad_query_duration(This->ppPins[0]->their_src, GST_FORMAT_TIME, &duration); + for (i = 0; i < This->cStreams; ++i) + { + This->ppPins[i]->seek.llDuration = This->ppPins[i]->seek.llStop = duration / 100; + This->ppPins[i]->seek.llCurrent = 0; + if (!This->ppPins[i]->seek.llDuration) + This->ppPins[i]->seek.dwCapabilities = 0; + WaitForSingleObject(This->ppPins[i]->caps_event, INFINITE); } *props = This->props; @@ -1191,7 +1191,7 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI gst_pad_set_active(This->my_src, 1); This->nextofs = This->nextpullofs = 0; - return hr; + return S_OK; } static inline GSTOutPin *impl_from_IMediaSeeking( IMediaSeeking *iface ) -- 2.7.4
Especially since pads might be exposed before failure is reported. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index b48648c..b4c29db 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1160,11 +1160,11 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI ResetEvent(This->event); gst_element_set_state(This->container, GST_STATE_PLAYING); WaitForSingleObject(This->event, -1); - gst_element_get_state(This->container, NULL, NULL, -1); + ret = gst_element_get_state(This->container, NULL, NULL, -1); - if (!This->cStreams) + if (ret == GST_STATE_CHANGE_FAILURE) { - FIXME("GStreamer could not find any streams\n"); + ERR("GStreamer failed to play stream\n"); return E_FAIL; } -- 2.7.4
Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Thu, Feb 22, 2018 at 09:06:14AM -0600, Zebediah Figura wrote:
Especially since pads might be exposed before failure is reported.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index b48648c..b4c29db 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1160,11 +1160,11 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI ResetEvent(This->event); gst_element_set_state(This->container, GST_STATE_PLAYING); WaitForSingleObject(This->event, -1); - gst_element_get_state(This->container, NULL, NULL, -1); + ret = gst_element_get_state(This->container, NULL, NULL, -1);
- if (!This->cStreams) + if (ret == GST_STATE_CHANGE_FAILURE) { - FIXME("GStreamer could not find any streams\n"); + ERR("GStreamer failed to play stream\n"); return E_FAIL; }
-- 2.7.4
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index b4c29db..8ccf0ad 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1081,7 +1081,6 @@ static GstBusSyncReply watch_bus(GstBus *bus, GstMessage *msg, gpointer data) gst_message_parse_error(msg, &err, &dbg_info); FIXME("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); WARN("%s\n", dbg_info); - SetEvent(This->event); } else if (GST_MESSAGE_TYPE(msg) & GST_MESSAGE_WARNING) { gst_message_parse_warning(msg, &err, &dbg_info); WARN("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); @@ -1159,7 +1158,6 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI This->initial = This->discont = TRUE; ResetEvent(This->event); gst_element_set_state(This->container, GST_STATE_PLAYING); - WaitForSingleObject(This->event, -1); ret = gst_element_get_state(This->container, NULL, NULL, -1); if (ret == GST_STATE_CHANGE_FAILURE) @@ -1168,6 +1166,8 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI return E_FAIL; } + WaitForSingleObject(This->event, INFINITE); + gst_pad_query_duration(This->ppPins[0]->their_src, GST_FORMAT_TIME, &duration); for (i = 0; i < This->cStreams; ++i) { -- 2.7.4
Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Thu, Feb 22, 2018 at 09:06:15AM -0600, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index b4c29db..8ccf0ad 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1081,7 +1081,6 @@ static GstBusSyncReply watch_bus(GstBus *bus, GstMessage *msg, gpointer data) gst_message_parse_error(msg, &err, &dbg_info); FIXME("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); WARN("%s\n", dbg_info); - SetEvent(This->event); } else if (GST_MESSAGE_TYPE(msg) & GST_MESSAGE_WARNING) { gst_message_parse_warning(msg, &err, &dbg_info); WARN("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); @@ -1159,7 +1158,6 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI This->initial = This->discont = TRUE; ResetEvent(This->event); gst_element_set_state(This->container, GST_STATE_PLAYING); - WaitForSingleObject(This->event, -1); ret = gst_element_get_state(This->container, NULL, NULL, -1);
if (ret == GST_STATE_CHANGE_FAILURE) @@ -1168,6 +1166,8 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI return E_FAIL; }
+ WaitForSingleObject(This->event, INFINITE); + gst_pad_query_duration(This->ppPins[0]->their_src, GST_FORMAT_TIME, &duration); for (i = 0; i < This->cStreams; ++i) { -- 2.7.4
Source pads can be created asynchronously, so they might not be created until during the call to gst_element_set_state(). Therefore don't copy This->ppPins into a local variable. This condition can be triggered by failing to demux a stream. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index 8ccf0ad..518d995 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1913,7 +1913,6 @@ static HRESULT GST_RemoveOutputPins(GSTImpl *This) { HRESULT hr; ULONG i; - GSTOutPin **ppOldPins = This->ppPins; TRACE("(%p)\n", This); mark_wine_thread(); @@ -1927,17 +1926,17 @@ static HRESULT GST_RemoveOutputPins(GSTImpl *This) This->my_src = This->their_sink = NULL; for (i = 0; i < This->cStreams; i++) { - hr = BaseOutputPinImpl_BreakConnect(&ppOldPins[i]->pin); + hr = BaseOutputPinImpl_BreakConnect(&This->ppPins[i]->pin); TRACE("Disconnect: %08x\n", hr); - IPin_Release(&ppOldPins[i]->pin.pin.IPin_iface); + IPin_Release(&This->ppPins[i]->pin.pin.IPin_iface); } This->cStreams = 0; + CoTaskMemFree(This->ppPins); This->ppPins = NULL; gst_element_set_bus(This->container, NULL); gst_object_unref(This->container); This->container = NULL; BaseFilterImpl_IncrementPinVersion(&This->filter); - CoTaskMemFree(ppOldPins); return S_OK; } -- 2.7.4
Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Thu, Feb 22, 2018 at 09:06:16AM -0600, Zebediah Figura wrote:
Source pads can be created asynchronously, so they might not be created until during the call to gst_element_set_state(). Therefore don't copy This->ppPins into a local variable.
This condition can be triggered by failing to demux a stream.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index 8ccf0ad..518d995 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1913,7 +1913,6 @@ static HRESULT GST_RemoveOutputPins(GSTImpl *This) { HRESULT hr; ULONG i; - GSTOutPin **ppOldPins = This->ppPins;
TRACE("(%p)\n", This); mark_wine_thread(); @@ -1927,17 +1926,17 @@ static HRESULT GST_RemoveOutputPins(GSTImpl *This) This->my_src = This->their_sink = NULL;
for (i = 0; i < This->cStreams; i++) { - hr = BaseOutputPinImpl_BreakConnect(&ppOldPins[i]->pin); + hr = BaseOutputPinImpl_BreakConnect(&This->ppPins[i]->pin); TRACE("Disconnect: %08x\n", hr); - IPin_Release(&ppOldPins[i]->pin.pin.IPin_iface); + IPin_Release(&This->ppPins[i]->pin.pin.IPin_iface); } This->cStreams = 0; + CoTaskMemFree(This->ppPins); This->ppPins = NULL; gst_element_set_bus(This->container, NULL); gst_object_unref(This->container); This->container = NULL; BaseFilterImpl_IncrementPinVersion(&This->filter); - CoTaskMemFree(ppOldPins); return S_OK; }
-- 2.7.4
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 8 ++++---- dlls/winegstreamer/gsttffilter.c | 4 ++-- dlls/winegstreamer/main.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index 518d995..0ce92a4 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1079,8 +1079,8 @@ static GstBusSyncReply watch_bus(GstBus *bus, GstMessage *msg, gpointer data) if (GST_MESSAGE_TYPE(msg) & GST_MESSAGE_ERROR) { gst_message_parse_error(msg, &err, &dbg_info); - FIXME("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); - WARN("%s\n", dbg_info); + ERR("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); + ERR("%s\n", dbg_info); } else if (GST_MESSAGE_TYPE(msg) & GST_MESSAGE_WARNING) { gst_message_parse_warning(msg, &err, &dbg_info); WARN("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); @@ -1095,7 +1095,7 @@ static GstBusSyncReply watch_bus(GstBus *bus, GstMessage *msg, gpointer data) static void unknown_type(GstElement *bin, GstPad *pad, GstCaps *caps, gpointer user) { gchar *strcaps = gst_caps_to_string(caps); - FIXME("Could not find a filter for caps: %s\n", strcaps); + ERR("Could not find a filter for caps: %s\n", strcaps); g_free(strcaps); } @@ -1126,7 +1126,7 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI gstfilter = gst_element_factory_make("decodebin", NULL); if (!gstfilter) { - FIXME("Could not make source filter, are gstreamer-plugins-* installed for %u bits?\n", + ERR("Could not make source filter, are gstreamer-plugins-* installed for %u bits?\n", 8 * (int)sizeof(void*)); return E_FAIL; } diff --git a/dlls/winegstreamer/gsttffilter.c b/dlls/winegstreamer/gsttffilter.c index 401a92f..29e679b 100644 --- a/dlls/winegstreamer/gsttffilter.c +++ b/dlls/winegstreamer/gsttffilter.c @@ -111,7 +111,7 @@ static const char *Gstreamer_FindMatch(const char *strcaps) g_list_free(copy); if (!bestfactory) { - FIXME("Could not find plugin for %s\n", strcaps); + ERR("Could not find plugin for %s\n", strcaps); return NULL; } return gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(bestfactory)); @@ -312,7 +312,7 @@ static HRESULT Gstreamer_transform_ConnectInput(GstTfImpl *This, const AM_MEDIA_ This->filter = gst_element_factory_make(This->gstreamer_name, NULL); if (!This->filter) { - FIXME("Could not make %s filter\n", This->gstreamer_name); + ERR("Could not make %s filter\n", This->gstreamer_name); return E_FAIL; } This->my_src = gst_pad_new("yuvsrc", GST_PAD_SRC); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 2bb69f4..dc988cf 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -276,7 +276,7 @@ DWORD Gstreamer_init(void) inited = gst_init_check(&argc, &argv, &err); HeapFree(GetProcessHeap(), 0, argv); if (err) { - FIXME("Failed to initialize gstreamer: %s\n", err->message); + ERR("Failed to initialize gstreamer: %s\n", err->message); g_error_free(err); } if (inited) { -- 2.7.4
Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Thu, Feb 22, 2018 at 09:06:17AM -0600, Zebediah Figura wrote:
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 8 ++++---- dlls/winegstreamer/gsttffilter.c | 4 ++-- dlls/winegstreamer/main.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index 518d995..0ce92a4 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1079,8 +1079,8 @@ static GstBusSyncReply watch_bus(GstBus *bus, GstMessage *msg, gpointer data)
if (GST_MESSAGE_TYPE(msg) & GST_MESSAGE_ERROR) { gst_message_parse_error(msg, &err, &dbg_info); - FIXME("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); - WARN("%s\n", dbg_info); + ERR("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); + ERR("%s\n", dbg_info); } else if (GST_MESSAGE_TYPE(msg) & GST_MESSAGE_WARNING) { gst_message_parse_warning(msg, &err, &dbg_info); WARN("%s: %s\n", GST_OBJECT_NAME(msg->src), err->message); @@ -1095,7 +1095,7 @@ static GstBusSyncReply watch_bus(GstBus *bus, GstMessage *msg, gpointer data) static void unknown_type(GstElement *bin, GstPad *pad, GstCaps *caps, gpointer user) { gchar *strcaps = gst_caps_to_string(caps); - FIXME("Could not find a filter for caps: %s\n", strcaps); + ERR("Could not find a filter for caps: %s\n", strcaps); g_free(strcaps); }
@@ -1126,7 +1126,7 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI
gstfilter = gst_element_factory_make("decodebin", NULL); if (!gstfilter) { - FIXME("Could not make source filter, are gstreamer-plugins-* installed for %u bits?\n", + ERR("Could not make source filter, are gstreamer-plugins-* installed for %u bits?\n", 8 * (int)sizeof(void*)); return E_FAIL; } diff --git a/dlls/winegstreamer/gsttffilter.c b/dlls/winegstreamer/gsttffilter.c index 401a92f..29e679b 100644 --- a/dlls/winegstreamer/gsttffilter.c +++ b/dlls/winegstreamer/gsttffilter.c @@ -111,7 +111,7 @@ static const char *Gstreamer_FindMatch(const char *strcaps) g_list_free(copy);
if (!bestfactory) { - FIXME("Could not find plugin for %s\n", strcaps); + ERR("Could not find plugin for %s\n", strcaps); return NULL; } return gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(bestfactory)); @@ -312,7 +312,7 @@ static HRESULT Gstreamer_transform_ConnectInput(GstTfImpl *This, const AM_MEDIA_
This->filter = gst_element_factory_make(This->gstreamer_name, NULL); if (!This->filter) { - FIXME("Could not make %s filter\n", This->gstreamer_name); + ERR("Could not make %s filter\n", This->gstreamer_name); return E_FAIL; } This->my_src = gst_pad_new("yuvsrc", GST_PAD_SRC); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 2bb69f4..dc988cf 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -276,7 +276,7 @@ DWORD Gstreamer_init(void) inited = gst_init_check(&argc, &argv, &err); HeapFree(GetProcessHeap(), 0, argv); if (err) { - FIXME("Failed to initialize gstreamer: %s\n", err->message); + ERR("Failed to initialize gstreamer: %s\n", err->message); g_error_free(err); } if (inited) { -- 2.7.4
Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> On Thu, Feb 22, 2018 at 09:06:13AM -0600, Zebediah Figura wrote:
Otherwise the push_data() thread will run and try to use resources we never allocated.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/winegstreamer/gstdemux.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/dlls/winegstreamer/gstdemux.c b/dlls/winegstreamer/gstdemux.c index a19c64f..b48648c 100644 --- a/dlls/winegstreamer/gstdemux.c +++ b/dlls/winegstreamer/gstdemux.c @@ -1103,7 +1103,6 @@ static void unknown_type(GstElement *bin, GstPad *pad, GstCaps *caps, gpointer u static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTIES *props) { GSTImpl *This = (GSTImpl*)pPin->pin.pinInfo.pFilter; - HRESULT hr; int ret, i; LONGLONG avail, duration; GstStaticPadTemplate src_template = GST_STATIC_PAD_TEMPLATE( @@ -1163,19 +1162,20 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI WaitForSingleObject(This->event, -1); gst_element_get_state(This->container, NULL, NULL, -1);
- if (!This->cStreams) { + if (!This->cStreams) + { FIXME("GStreamer could not find any streams\n"); - hr = E_FAIL; - } else { - gst_pad_query_duration(This->ppPins[0]->their_src, GST_FORMAT_TIME, &duration); - for (i = 0; i < This->cStreams; ++i) { - This->ppPins[i]->seek.llDuration = This->ppPins[i]->seek.llStop = duration / 100; - This->ppPins[i]->seek.llCurrent = 0; - if (!This->ppPins[i]->seek.llDuration) - This->ppPins[i]->seek.dwCapabilities = 0; - WaitForSingleObject(This->ppPins[i]->caps_event, -1); - } - hr = S_OK; + return E_FAIL; + } + + gst_pad_query_duration(This->ppPins[0]->their_src, GST_FORMAT_TIME, &duration); + for (i = 0; i < This->cStreams; ++i) + { + This->ppPins[i]->seek.llDuration = This->ppPins[i]->seek.llStop = duration / 100; + This->ppPins[i]->seek.llCurrent = 0; + if (!This->ppPins[i]->seek.llDuration) + This->ppPins[i]->seek.dwCapabilities = 0; + WaitForSingleObject(This->ppPins[i]->caps_event, INFINITE); } *props = This->props;
@@ -1191,7 +1191,7 @@ static HRESULT GST_Connect(GSTInPin *pPin, IPin *pConnectPin, ALLOCATOR_PROPERTI gst_pad_set_active(This->my_src, 1);
This->nextofs = This->nextpullofs = 0; - return hr; + return S_OK; }
static inline GSTOutPin *impl_from_IMediaSeeking( IMediaSeeking *iface ) -- 2.7.4
participants (2)
-
Andrew Eikum -
Zebediah Figura