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 751071 - applemedia: CMBlockBuffer can be non-contiguous
applemedia: CMBlockBuffer can be non-contiguous
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Mac OS
: Normal normal
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 751072
 
 
Reported: 2015-06-16 19:48 UTC by Ilya Konstantinov
Modified: 2015-08-16 13:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
applemedia: CMBlockBuffer can be non-contiguous (1.85 KB, patch)
2015-06-16 19:49 UTC, Ilya Konstantinov
none Details | Review
applemedia: CMBlockBuffer can be non-contiguous (1.85 KB, patch)
2015-06-24 21:58 UTC, Ilya Konstantinov
none Details | Review
applemedia: CMBlockBuffer can be non-contiguous (1.80 KB, patch)
2015-06-24 22:00 UTC, Ilya Konstantinov
committed Details | Review

Description Ilya Konstantinov 2015-06-16 19:48:22 UTC
The function that wraps Apple's CMBlockBuffer -- gst_core_media_buffer_wrap_block_buffer -- makes an incorrect assumption about CMBlockBuffer being a single memory block, and uses CMBlockBufferGetDataLength incorrectly.

In fact, they're more like our GstBuffer than GstMemory.
Comment 1 Ilya Konstantinov 2015-06-16 19:49:14 UTC
Created attachment 305422 [details] [review]
applemedia: CMBlockBuffer can be non-contiguous
Comment 2 Sebastian Dröge (slomo) 2015-06-22 12:14:03 UTC
Review of attachment 305422 [details] [review]:

::: sys/applemedia/coremediabuffer.c
@@ +159,3 @@
+    status =
+        CMBlockBufferGetDataPointer (block_buf, offset, &length_at_offset,
+        &total_length, &data);

total_length is always containing the total length from the beginning of the block_buf? I.e. it will always return the same value here in this loop?

@@ +171,3 @@
+      break;
+    offset += length_at_offset;
+  } while (TRUE);

Why not "while (offset < total_length)"?
Comment 3 Ilya Konstantinov 2015-06-22 12:20:09 UTC
(In reply to Sebastian Dröge (slomo) from comment #2)
> ::: sys/applemedia/coremediabuffer.c
> @@ +159,3 @@
> +    status =
> +        CMBlockBufferGetDataPointer (block_buf, offset, &length_at_offset,
> +        &total_length, &data);
> 
> total_length is always containing the total length from the beginning of the
> block_buf? I.e. it will always return the same value here in this loop?

Yes, it's a funny API.

"The caller can compare (offset+lengthAtOffset) with totalLength to determine whether the entire CMBlockBuffer has been referenced and whether it is possible to access the CMBlockBuffer's data with a contiguous reference."
 
> @@ +171,3 @@
> +      break;
> +    offset += length_at_offset;
> +  } while (TRUE);
> 
> Why not "while (offset < total_length)"?

You're right, what you propose is nicer.
Comment 4 Ilya Konstantinov 2015-06-24 21:58:39 UTC
Created attachment 306057 [details] [review]
applemedia: CMBlockBuffer can be non-contiguous

Changed to "while (offset < total_length)".

Tested with:
gst-launch-1.0 avfvideosrc ! vtenc_h264 ! vtdec ! videoflip ! glimagesink

(inserted `videoflip` since something is broken with GLmemory in my repo)
Comment 5 Ilya Konstantinov 2015-06-24 22:00:23 UTC
Created attachment 306059 [details] [review]
applemedia: CMBlockBuffer can be non-contiguous

Oops, wrong file.
Comment 6 Sebastian Dröge (slomo) 2015-06-25 08:35:02 UTC
commit 8cd65c325093e8c951d8b39d403591871519f750
Author: Ilya Konstantinov <ilya.konstantinov@gmail.com>
Date:   Mon Apr 27 15:02:59 2015 +0300

    applemedia: CMBlockBuffer can be non-contiguous
    
    CMBlockBufferGetDataLength would return the entire data length, while
    size of individual blocks can be smaller. Iterate over the block buffer
    and add the individual (possibly non-contiguous) memory blocks.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=751071