GNOME Bugzilla – Bug 728326
Check for return values of gst_buffer_map and gst_memory_map
Last modified: 2016-02-23 16:38:09 UTC
They can fail in various situations.
Created attachment 274437 [details] [review] buffer: Check return value of gst_memory_map() Only do memory operations if the memory was succesfully map'ed
Created attachment 274438 [details] [review] plugins: Check return values of gst_buffer_map() They can fail for various reasons. For non-fatal cases (such as the dump feature of identiy and fakesink), we just silently skip it. For other cases post an error message.
Created attachment 274439 [details] [review] gstcheck: Check return value of gst_buffer_map We can't check contents if we don't have access to it
Created attachment 274440 [details] [review] manual: Fix examples to check for gst_buffer_map return values Otherwise people reading the manual will expect it to always succeed :)
Comment on attachment 274437 [details] [review] buffer: Check return value of gst_memory_map() If mapping fails it should probably do *something* different. Like return an error (NULL buffer) or doing a g_warning() or ... Otherwise this is good to go.
Comment on attachment 274440 [details] [review] manual: Fix examples to check for gst_buffer_map return values For the first one, gst_buffer_make_writable() can fail ;) For the second, this needs some explanation
Edward?
Created attachment 284983 [details] [review] buffer: Check return value of gst_memory_map() Only do memory operations if the memory was succesfully map'ed
Created attachment 284984 [details] [review] plugins: Check return values of gst_buffer_map() They can fail for various reasons. For non-fatal cases (such as the dump feature of identiy and fakesink), we just silently skip it. For other cases post an error message.
Created attachment 284985 [details] [review] gstcheck: Check return value of gst_buffer_map We can't check contents if we don't have access to it
Created attachment 284986 [details] [review] manual: Fix examples to check for gst_buffer_map return values Otherwise people reading the manual will expect it to always succeed :)
Review of attachment 284984 [details] [review]: ::: plugins/elements/gstfakesink.c @@ +481,3 @@ + gst_util_dump_mem (info.data, info.size); + gst_buffer_unmap (buf, &info); + } Maybe an else case that says that the buffer couldn't be mapped? Or a GST_WARNING or something? Also in the other elements
Review of attachment 284983 [details] [review]: ::: gst/gstbuffer.c @@ +248,3 @@ + gst_memory_unmap (result, &dinfo); + } else if (result) { + GST_WARNING ("Can't get writable GstMemory to copy/contatenate buffer"); typo: contatenate And maybe just say "merge" instead of "copy/concatenate"?
Ping.
Ping²
commit 01ad1fb343b020e0278bd284a6d49e96629614fb Author: Edward Hervey <edward@collabora.com> Date: Wed Apr 16 11:42:18 2014 +0200 manual: Fix examples to check for gst_buffer_map return values Otherwise people reading the manual will expect it to always succeed :) https://bugzilla.gnome.org/show_bug.cgi?id=728326 commit 1f7fba19f7aec15bd1f1adcfc39aab827c1ac44b Author: Edward Hervey <edward@collabora.com> Date: Wed Apr 16 11:40:46 2014 +0200 gstcheck: Check return value of gst_buffer_map We can't check contents if we don't have access to it https://bugzilla.gnome.org/show_bug.cgi?id=728326 commit 555df9d614149fa0a094c14788b61e16a439b3f8 Author: Edward Hervey <edward@collabora.com> Date: Wed Apr 16 11:39:15 2014 +0200 plugins: Check return values of gst_buffer_map() They can fail for various reasons. For non-fatal cases (such as the dump feature of identiy and fakesink), we just silently skip it. For other cases post an error message. https://bugzilla.gnome.org/show_bug.cgi?id=728326 commit f7cba27157f9e36c34bda2d8741be4ccb2d17d96 Author: Edward Hervey <bilboed@bilboed.com> Date: Tue Feb 23 17:23:43 2016 +0100 buffer: Check return value of gst_memory_map() Only do memory operations if the memory was succesfully map'ed https://bugzilla.gnome.org/show_bug.cgi?id=728326