GNOME Bugzilla – Bug 752154
metainfo: handle return value properly
Last modified: 2015-07-09 14:33:13 UTC
Return value of gst_buffer_foreach_meta states that * Returns: %FALSE when @func returned %FALSE for one of the metadata. but the return value of function in adapter is always TRUE even if the transform_func returns TRUE. But i feel the return value should be handled because, once we know the transform_func is not able to handle, there is no need of re-iterating the whole loop to just fail again.
Created attachment 307128 [details] [review] handle return value of transform_func
Created attachment 307130 [details] [review] buffer: stop iterating when transform_func fails. This is basically same as the patch in adapter. When the transform_func fails, there is no need to continue with the loop as it will fail again.
And i am of the opinion that in the transform_func, the default return value should be FALSE. And the return should be TRUE only when it is able to handle the copy. There is no need to call the transform function again, once we know that it wont be able to handle it. This way it can satisfy the below statement. * Returns: %FALSE when @func returned %FALSE for one of the metadata.
I've grepped around and all uses of foreach_metadata() ignore the return value. Either you return FALSE when the transform_func fails and callers use this information, or the function could be turned into a void.
Comment on attachment 307130 [details] [review] buffer: stop iterating when transform_func fails. That doesn't seem good, if a transform fails then the meta is just not copied and is dropped. That might be worth a GST_ERROR, but other than that it should just be ignored and the next meta should be tried.
Comment on attachment 307128 [details] [review] handle return value of transform_func Same here
(In reply to Luis de Bethencourt from comment #4) > I've grepped around and all uses of foreach_metadata() ignore the return > value. Either you return FALSE when the transform_func fails and callers use > this information, or the function could be turned into a void. It is useful information to print in debug logs, but otherwise it should not be handled as a hard error.