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 695215 - Possible bug in gstbasetransform.c
Possible bug in gstbasetransform.c
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.0.5
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-05 13:57 UTC by Paul HENRYS
Modified: 2013-04-04 16:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.04 KB, patch)
2013-03-05 13:57 UTC, Paul HENRYS
none Details | Review

Description Paul HENRYS 2013-03-05 13:57:05 UTC
Created attachment 238129 [details] [review]
Patch

In gst_base_transform_acceptcaps_default(), gst_caps_is_subset() is used to check whether the caps in the query are a subset of the caps on the sink/src pad of the element. As specified in GStreamer doc about gst_caps_is_subset:

"This function does not work reliably if optional properties for caps are included on one caps and omitted on the other."

Therefore, I think gst_caps_can_intersect should be used instead (see patch).
By the way the original comment speak about "intersect".
What do you think?

Cheers,

Paul HENRYS
Comment 1 Tim-Philipp Müller 2013-03-05 14:05:38 UTC
What problem does this solve for you in what pipeline with what elements?

I am not sure if the comment is still correct, I believe this got fixed in git master (and is both more correct and stricter now than it used to be).
Comment 2 Paul HENRYS 2013-03-05 14:47:12 UTC
I got a problem with such a pipeline:

gst-launch-1.0 filesrc location=myvideo.mkv ! matroskademux ! identity ! videomixer ! xvimagesink

where myvideo file is video-raw file in an mkv container. Of course identity is useless here but I have a custom bin which by default contain an identity element if nothing is to be done.
Comment 3 Tim-Philipp Müller 2013-03-05 15:13:10 UTC
What caps does matroskademux output?
Comment 4 Paul HENRYS 2013-03-05 15:23:37 UTC
the caps are: video/x-raw, format=(string)I420, width=(int)1280, height=(int)720, framerate=(fraction)10000000/166833

The point is that videomixer get back the src pad caps from its src template caps (as the src caps are not set yet) and there is a pixel-aspect-ratio field which is not present in the caps above, thus it's not a subset but pixel-aspect-ratio is not mandatory, isn't it?
Besides, pixel-aspect-ratio is not in videomixer src template caps but is added somehow.
Comment 5 Nicolas Dufresne (ndufresne) 2013-03-05 16:26:06 UTC
Thsi patch http://bugzilla-attachments.gnome.org/attachment.cgi?id=229286 from bug #684237 address this specific issue.
Comment 6 Paul HENRYS 2013-03-06 08:08:49 UTC
Hi Nicolas,

Thx for your feedback, it seems this patch has not been applied on gst-plugins-good master branch yet, at least the add pixel-aspect-ration is still there.
Another question, is it relevant to use gst_caps_is_subset() in gst_base_transform_acceptcaps_default() as said above? By using gst_caps_can_intersect() it will not fail with optional parameters? Am I missing something?
Comment 7 Nicolas Dufresne (ndufresne) 2013-03-06 15:13:01 UTC
(In reply to comment #6)
> Hi Nicolas,
> 
> Thx for your feedback, it seems this patch has not been applied on
> gst-plugins-good master branch yet, at least the add pixel-aspect-ration is
> still there.
> Another question, is it relevant to use gst_caps_is_subset() in
> gst_base_transform_acceptcaps_default() as said above? By using
> gst_caps_can_intersect() it will not fail with optional parameters? Am I
> missing something?

Using can_intersect() is not needed when you start considering upstream caps during negotiation and re-negotiation. But I'm not done with that, as I am facing some race conditions, this is the reason the patch set was not merged. I think the patch 229286 is the only patch that may be be used by alone, but need testing. I'll try to finish this work by the end of march, but it's a difficult problem to do N to 1 negotiation. We have similar issues with adder and liveadder elements.
Comment 8 Wim Taymans 2013-04-04 16:32:43 UTC
can_intersect() is not the right thing to do, you need to use is_subset(), which is fixed in git now to do the right thing in all cases. 

The missing caps fields in the demuxer or extra fields in the mixer is another problem..