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 701237 - videomixer : on query caps, we should return FALSE if the source pad has no current caps.
videomixer : on query caps, we should return FALSE if the source pad has no c...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.1.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-05-30 03:49 UTC by Mathieu Duponchelle
Modified: 2013-05-30 19:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch to fix the issue (2.25 KB, patch)
2013-05-30 03:49 UTC, Mathieu Duponchelle
none Details | Review
Proposed patch to fix the issue (811 bytes, patch)
2013-05-30 03:53 UTC, Mathieu Duponchelle
rejected Details | Review
Proposed patch to fix the issue (1.30 KB, patch)
2013-05-30 19:30 UTC, Mathieu Duponchelle
committed Details | Review

Description Mathieu Duponchelle 2013-05-30 03:49:18 UTC
Created attachment 245607 [details] [review]
Proposed patch to fix the issue

For now, I have a case where

1) the caps get queried
2) get_current_caps(srcpad) returns NULL
3) make_writable is called on NULL, which causes a miniobject != NULL assertion.

I have not investigated the cause of the caps being NULL in that specific case,
but it seems quite logical to me to return FALSE to the query instead of raising that assertion, and the case I'm implementing now (videomixing in GESVideoTracks) (kind of) works, at least I don't experience any caps / caps negotiation issues.

The proposed patch does exactly that.
Comment 1 Mathieu Duponchelle 2013-05-30 03:53:01 UTC
Created attachment 245608 [details] [review]
Proposed patch to fix the issue

oops that's better
Comment 2 Sebastian Dröge (slomo) 2013-05-30 06:19:09 UTC
How can it even happen that we have no current caps but GST_VIDEO_INFO_FORMAT (&mix->info) != GST_VIDEO_FORMAT_UNKNOWN. That might be the real bug here.

Otherwise, if that is acceptable and not a problem at all, you probably want to return the template caps in that case too.
Comment 3 Nicolas Dufresne (ndufresne) 2013-05-30 12:53:52 UTC
Oh, I forgot about it, but I had a patch somewhere for that (which is better then returning FALSE iirc). In answer to your question, the new code delay the caps event until _collected() is called, which mean there exist a gap between the moment we do have mix->info set, and the moment the pad has current caps. Hope this answer the question.

Mathieu, the right patch would be to return the current caps (maybe from the mix->info).
Comment 4 Nicolas Dufresne (ndufresne) 2013-05-30 12:54:45 UTC
Review of attachment 245608 [details] [review]:

As per previous comment, mix->info is set, so we do have caps.
Comment 5 Mathieu Duponchelle 2013-05-30 19:30:00 UTC
Created attachment 245672 [details] [review]
Proposed patch to fix the issue

OK so I think that would be the correct fix then.
Comment 6 Nicolas Dufresne (ndufresne) 2013-05-30 19:37:51 UTC
Review of attachment 245672 [details] [review]:

commit 5223868caa9dc82d18dc3c92113bf2b8e37761fe
Author: Mathieu Duponchelle <mathieu.duponchelle@epitech.eu>
Date:   Thu May 30 21:20:59 2013 +0200

    videomixer: Set a reference to mix->current_caps as the QUERY_CAPS result.
Comment 7 Nicolas Dufresne (ndufresne) 2013-05-30 19:38:13 UTC
Thanks.
Comment 8 Nicolas Dufresne (ndufresne) 2013-05-30 19:39:27 UTC
Oops, sorry forgot to add the bug reference in the commit.