GNOME Bugzilla – Bug 735804
smpte: Creates incomplete raw video caps
Last modified: 2014-09-04 09:56:11 UTC
Created attachment 284993 [details] [review] Set caps sent by upstream element Example pipeline (mentioned below) throws errors as caps negotiation fails. gst-launch-1.0 -v videotestsrc pattern=1 ! smpte name=s border=20000 type=234 duration=2000000000 ! videoconvert ! ximagesink videotestsrc ! s. Solution patch attached. Please review.
Review of attachment 284993 [details] [review]: ::: gst/smpte/gstsmpte.c @@ +302,3 @@ + if (smpte->src_caps == NULL + || gst_caps_is_equal (caps, smpte->src_caps) == FALSE) { Don't check gbooleans with ==. Just do || gst_caps_is_equal()) @@ +386,3 @@ + if (smpte->src_caps) { + gst_caps_replace (&smpte->src_caps, NULL); + smpte->src_caps = NULL; The caps should already be cleared in PAUSED→READY state change @@ +532,3 @@ GstSegment segment; + gst_pad_set_caps (smpte->srcpad, smpte->src_caps); Why not just forward the caps directly from the setcaps function by calling gst_pad_set_caps() from there? Also what do you do if both sinkpads have different caps that are nonetheless compatible for the smpte element?
Also a bigger question is why caps negotiation fails, there's probably a deeper problem here
Thanks for the review. Here is my understanding - >>why caps negotiation fails caps nego failed because videotestsrc produced caps including properties "pixel-aspect-ratio" and "interlace-mode" however in smpte we create hard-wired caps without those 2 properties. When I added those properties, pipeline worked. Is it a good idea to modify the pad template and implement query function to handle QUERY_CAPS ? >>Why not just forward the caps directly from the setcaps function by calling gst_pad_set_caps() from there? When we receive caps event, stream-start event isn't initated yet. So, that would cause sticky-event misordering. >> Also what do you do if both sinkpads have different caps that are nonetheless compatible for the smpte element? SMPTE documentation says - smpte can accept I420 video streams with the *same* width, height and framerate. Please let me know if got something wrong.
No, seems correct. However it also should check that pixel-aspect-ratio, interlace-mode and other raw video properties are the same. Basically needs to check if the GstVideoInfo of both inputs are equal. And then you can simply create the caps from the GstVideoInfo of one of the inputs here.
Created attachment 285117 [details] [review] compare v1 and v2, create caps from one of GstVideoInfo New patch attached. Please review. Thanks.
commit ea43ef214a77a0219af9a9be922042fbf5e53f15 Author: Ravi Kiran K N <ravi.kiran@samsung.com> Date: Tue Sep 2 13:52:43 2014 +0530 smpte: Check if input caps are the same and create output caps from video info This makes sure that also properties like the pixel-aspect-ratio are the same between both streams and that the output caps contain all fields necessary for complete video caps. https://bugzilla.gnome.org/show_bug.cgi?id=735804