GNOME Bugzilla – Bug 722757
Fix derived object data initialization issues in gstvaapiobject.c
Last modified: 2014-07-03 21:55:02 UTC
patch1: gstvaapiobject: Initialize the derived object data with init() hook The gst_vaapi_object_new() should zero-initialize the derived object data if and only if the subclass have no init() implementation for GstVaapiObjectClass. eg use case: gstvaapiimage should be initialized with gst_vaapi_image_init() during object creation. patch2: gstvaapiobject: Fix crash in gst_vaapi_object_new() If the GstVaapiObjectClass argument to gst_vaapi_object_new() is NULL, then the size of the allocated object should the same as sizeof(GstVaapiObject). Handle the case of klass==NULL gracefully.
Created attachment 266949 [details] [review] gstvaapiobject: Initialize the derived object data with init() hook The gst_vaapi_object_new() should zero-initialize the derived object data if and only if the subclass have no init() implemention for GstVaapiObjectClass.
Created attachment 266950 [details] [review] gstvaapiobject: Fix crash in gst_vaapi_object_new() If the GstVaapiObjectClass argument to gst_vaapi_object_new()is NULL, then the size of the allocated object should the same as sizeof(GstVaapiObject). Handle the case of klass==NULL gracefully.
Hi, sorry, I had missed this one. Do we have cases where gst_vaapi_object_new() is called with NULL? I think the other issue is also that we don't actually call the init() function. :)
The aim of GstVaapiObject is to wrap a VA object, with its parent VA display. So, by default, it has value to be zero-initialized. However, only the init() function was not called, thus possibly causing crashes in GstVaapiImage release with certain VA drivers.
(In reply to comment #3) > Hi, sorry, I had missed this one. Do we have cases where gst_vaapi_object_new() > is called with NULL? I think the other issue is also that we don't actually > call the init() function. :) We are not calling gst_vaapi_object_new() with NULL in anywhere(at least I didn't see any places). And of course we are not calling init() hook which I fixed in this patch. >The aim of GstVaapiObject is to wrap a VA object, with its parent VA display. >So, by default, it has value to be zero-initialized. However, only the init() >function was not called, thus possibly causing crashes in GstVaapiImage release >with certain VA drivers. Possible. Eg case: At the moment gst_vaapi_image_init() is not calling because of the missing init() hook invocation. Also not properly handling the NULL value invocation of gst_vaapi_object_new() which has been fixed in the second patch.
commit 99bf1b1f9874f09a5c93f125f8f9a28faa2025a3 Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com> Date: Wed Jan 22 08:20:59 2014 +0200 GstVaapiObject: make gst_vaapi_object_new() more robust. Forbid GstVaapiObject to be created without an associated klass spec. It is mandatory that the subclass implements an adequate .finalize() hook, so it shall provide a valid GstVaapiObjectClass. https://bugzilla.gnome.org/show_bug.cgi?id=722757 [made non-NULL klass argument to gst_vaapi_object_new() a requirement] Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com> commit 502952d080f8608cca54cae7f5497cb39354da4a Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com> Date: Tue Jan 21 15:43:57 2014 +0200 GstVaapiObject: initialize the derived object data with init() hook. Call the subclass .init() function in gst_vaapi_object_new(), if needed. The default behaviour is to zero initialize the subclass object data, then the .init() function can be used to initialize fields to non-default values, e.g. VA object ids to VA_INVALID_ID. Also fix the gst_vaapi_object_new() description, which was merely copied from GstVaapiMiniObject. https://bugzilla.gnome.org/show_bug.cgi?id=722757 [changed to always zero initialize the subclass] Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Review of attachment 266949 [details] [review]: Changed to always zero initialize the subclass. This is needed because the .init() hook is only useful to set non-default values, e.g. VA object ids to VA_INVALID_ID. The rest is expected to be zero-initialized.
Review of attachment 266950 [details] [review]: Changed to actually forbid NULL klass arguments. It is necessary to provide a correct GstVaapiObjectClass with adequate .finalize() function.