After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 728326 - Check for return values of gst_buffer_map and gst_memory_map
Check for return values of gst_buffer_map and gst_memory_map
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 1.7.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-16 09:45 UTC by Edward Hervey
Modified: 2016-02-23 16:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
buffer: Check return value of gst_memory_map() (2.16 KB, patch)
2014-04-16 09:46 UTC, Edward Hervey
needs-work Details | Review
plugins: Check return values of gst_buffer_map() (8.04 KB, patch)
2014-04-16 09:46 UTC, Edward Hervey
accepted-commit_now Details | Review
gstcheck: Check return value of gst_buffer_map (1.48 KB, patch)
2014-04-16 09:46 UTC, Edward Hervey
accepted-commit_now Details | Review
manual: Fix examples to check for gst_buffer_map return values (2.61 KB, patch)
2014-04-16 09:46 UTC, Edward Hervey
needs-work Details | Review
buffer: Check return value of gst_memory_map() (2.33 KB, patch)
2014-09-01 08:42 UTC, Edward Hervey
accepted-commit_now Details | Review
plugins: Check return values of gst_buffer_map() (8.04 KB, patch)
2014-09-01 08:42 UTC, Edward Hervey
reviewed Details | Review
gstcheck: Check return value of gst_buffer_map (1.48 KB, patch)
2014-09-01 08:42 UTC, Edward Hervey
accepted-commit_now Details | Review
manual: Fix examples to check for gst_buffer_map return values (2.83 KB, patch)
2014-09-01 08:42 UTC, Edward Hervey
accepted-commit_now Details | Review

Description Edward Hervey 2014-04-16 09:45:21 UTC
They can fail in various situations.
Comment 1 Edward Hervey 2014-04-16 09:46:09 UTC
Created attachment 274437 [details] [review]
buffer: Check return value of gst_memory_map()

Only do memory operations if the memory was succesfully map'ed
Comment 2 Edward Hervey 2014-04-16 09:46:13 UTC
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.
Comment 3 Edward Hervey 2014-04-16 09:46:17 UTC
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
Comment 4 Edward Hervey 2014-04-16 09:46:21 UTC
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 5 Sebastian Dröge (slomo) 2014-04-16 18:10:41 UTC
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 6 Sebastian Dröge (slomo) 2014-04-16 18:13:54 UTC
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
Comment 7 Sebastian Dröge (slomo) 2014-07-18 12:21:41 UTC
Edward?
Comment 8 Edward Hervey 2014-09-01 08:42:21 UTC
Created attachment 284983 [details] [review]
buffer: Check return value of gst_memory_map()

Only do memory operations if the memory was succesfully map'ed
Comment 9 Edward Hervey 2014-09-01 08:42:25 UTC
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.
Comment 10 Edward Hervey 2014-09-01 08:42:30 UTC
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
Comment 11 Edward Hervey 2014-09-01 08:42:35 UTC
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 :)
Comment 12 Sebastian Dröge (slomo) 2014-09-01 09:15:04 UTC
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
Comment 13 Sebastian Dröge (slomo) 2014-09-01 09:17:26 UTC
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"?
Comment 14 Nicolas Dufresne (ndufresne) 2015-04-02 21:49:53 UTC
Ping.
Comment 15 Sebastian Dröge (slomo) 2015-10-02 14:47:55 UTC
Ping²
Comment 16 Edward Hervey 2016-02-23 16:38:09 UTC
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