GNOME Bugzilla – Bug 760479
basesrc: Only respond to duration/position queries if the quantity is valid
Last modified: 2016-01-11 20:43:57 UTC
Created attachment 318765 [details] [review] fix for basesrc duration&position queries Currently, basesrc can set invalid value combinations in the duration and position queries. Example: duration query uses the TIME format, the basesrc only knows a duration in BYTES of 400000 bytes, and conversion fails. However, prior to this change, the code still answers the query, setting the format to TIME, and the duration to 400000. This misleads downstream elements who first ask upstream if it knows the duration in TIME format; they misinterpret the value 400000 (which is actually the size in bytes) as a duration in TIME. With this patch, the queries are answered only if the duration/position value is actually valid. Downstream elements then simply get a failed query, as opposed to a "successful" but actually invalid one.
Review of attachment 318765 [details] [review]: Makes sense but why would the query be "successful" right now? res is FALSE, so gst_pad_query() should return FALSE. The query only contains invalid values afterwards
Actually, that is an excellent question :) I dug further, and as it turns out, there was one custom sink element in the application here that didn't check the gst_element_query() return value, assuming that it always succeeds. Still, I think the patch is valid. I'll update the commit message.
I think so too, it should be merged with a more correct commit message :)
Created attachment 318822 [details] [review] fix for basesrc duration&position queries v2
commit 4b0db9fd2412c1102d3c22dece1243fe441ac3f8 Author: Carlos Rafael Giani <dv@pseudoterminal.org> Date: Mon Jan 11 21:17:25 2016 +0100 basesrc: Only set duration/position query values in case of query success Currently, the query values are being set even if the query itself was determined to have failed. Fix this to ensure the values are only set in case of a query success. https://bugzilla.gnome.org/show_bug.cgi?id=760479