2016-11-15 7:08 GMT-07:00 Andrew Eikum aeikum@codeweavers.com:
This is an improvement, but I think we should go further and fix leaks in all of the failure cases. The problem is we're using the amt_from_gst_caps_video return value inconsistently. In accept_caps_sink, we unconditionally free the resulting AM_MEDIA_TYPE. But in setcaps_sink, we only free it, via the pin->pmt assignment, if amt_from_gst_caps_video succeeds.
I think we should always free the allocated memory in the failure cases in amt_from_gst_caps_video, and then only free the resulting AMT in the calling functions if amt_from_gst_caps_video succeeded. The resulting AMT should never be used or freed if amt_from_gst_caps_video fails.
Note that amt_from_gst_caps_audio has the same inconsistent return type handling. I would accept a separate patch to fix this handling, too. (Unlike for video, it doesn't currently matter in practice because of amt_from_gst_caps_audio's implementation.)
Does that make sense?
Yes, that makes sense. I'll send new patches tonight, unless you beat me to it ;-)
-Alex