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 794611 - GStreamer-CRITICAL updating the plugin registry on windows due to ladspa
GStreamer-CRITICAL updating the plugin registry on windows due to ladspa
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: dont know
1.14.0
Other Windows
: Normal blocker
: 1.14.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 795143 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2018-03-22 21:51 UTC by Nirbheek Chauhan
Modified: 2018-04-11 00:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ladspa: Fix critical during plugin load on Windows (951 bytes, patch)
2018-03-23 03:47 UTC, Nirbheek Chauhan
committed Details | Review
audiobasesrc: posting errors should be always be safe (1.06 KB, patch)
2018-04-09 11:56 UTC, Nirbheek Chauhan
committed Details | Review
wasapi: Don't open the device in get_caps() (3.40 KB, patch)
2018-04-09 11:56 UTC, Nirbheek Chauhan
committed Details | Review

Description Nirbheek Chauhan 2018-03-22 21:51:24 UTC
(gst-launch-1.0:9756): GStreamer-CRITICAL **: gst_object_ref: assertion 'object != NULL' failed


Not entirely sure yet why this happens; likely something in registry creation or loading?

This does not cause any failures of course, but it confuses people who are using gstreamer on windows.
Comment 1 Nirbheek Chauhan 2018-03-23 03:47:15 UTC
Created attachment 370044 [details] [review]
ladspa: Fix critical during plugin load on Windows
Comment 2 Nirbheek Chauhan 2018-03-23 03:48:44 UTC
Nicolas, can you please review this patch? I don't know if this is the expected behaviour :)
Comment 3 Sebastian Dröge (slomo) 2018-03-25 09:41:44 UTC
Review of attachment 370044 [details] [review]:

While this patch looks correct, how does it fix a gst_object_ref() critical warning? That sounds like at least some check would be needed elsewhere too to not work with NULL objects.
Comment 4 Nirbheek Chauhan 2018-03-25 14:11:21 UTC
Hmm, I wasn't paying enough attention while debugging this. I just set G_DEBUG=fatal-criticals and found this. This patch actually fixes this critical:

(gst-launch-1.0:1436): GLib-CRITICAL **: g_string_insert_len: assertion 'len == 0 || val != NULL' failed

I can't reproduce the other critical anymore. I am baffled. Maybe it was fixed between one of the dev releases and the final release?
Comment 5 Sebastian Dröge (slomo) 2018-03-25 16:02:08 UTC
Maybe it reappears again if you let it recreate the whole registry?
Comment 6 Nirbheek Chauhan 2018-03-27 06:33:39 UTC
(In reply to Sebastian Dröge (slomo) from comment #5)
> Maybe it reappears again if you let it recreate the whole registry?

I did all my testing my deleting the registry and forcing a full rescan every time. I guess I'll push this and close the bug, and see if someone else reports it.
Comment 7 Sebastian Dröge (slomo) 2018-03-27 06:34:22 UTC
Yes, go ahead :)
Comment 8 Nirbheek Chauhan 2018-04-09 11:52:36 UTC
(In reply to Nirbheek Chauhan from comment #0)
> (gst-launch-1.0:9756): GStreamer-CRITICAL **: gst_object_ref: assertion
> 'object != NULL' failed
> 

Turns out this happens when you pass an invalid device to wasapisrc and we post an ELEMENT_ERROR in get_caps(), which calls gst_audio_base_src_post_message() and does gst_object_ref (src->ringbuffer) before the ringbuffer is allocated.

The fix is in two parts:

1. Don't call ELEMENT_ERROR in get_caps() before the device is opened; just return the template caps in that case
2. Check that the ringbuffer is allocated before trying to ref it in gst_audio_base_src_post_message()
Comment 9 Nirbheek Chauhan 2018-04-09 11:56:10 UTC
Created attachment 370683 [details] [review]
audiobasesrc: posting errors should be always be safe

Don't try to signal an error in the ringbuffer if it hasn't been
allocated yet.
Comment 10 Nirbheek Chauhan 2018-04-09 11:56:29 UTC
Created attachment 370684 [details] [review]
wasapi: Don't open the device in get_caps()

We can just return the template caps till the device is opened when
going from READY -> PAUSED. This fixes a CRITICAL when calling
ELEMENT_ERROR before the ringbuffer is allocated.

Also fixes a couple of leaks in error conditions.
Comment 11 Nirbheek Chauhan 2018-04-09 11:57:28 UTC
Comment on attachment 370683 [details] [review]
audiobasesrc: posting errors should be always be safe

Attachment 370683 [details] pushed as b569899 - audiobasesrc: posting errors should be always be safe
Comment 12 Nirbheek Chauhan 2018-04-09 11:57:55 UTC
Attachment 370684 [details] pushed as 371a787 - wasapi: Don't open the device in get_caps()
Comment 13 Nirbheek Chauhan 2018-04-11 00:31:01 UTC
*** Bug 795143 has been marked as a duplicate of this bug. ***