GNOME Bugzilla – Bug 377713
mpeg2dec: should negotiate alignment of 16 bytes with xvimagesink et al.
Last modified: 2012-06-25 07:45:09 UTC
+++ This bug was initially created as a clone of Bug #327350 +++ If possible, xvimagesink should align buffers to 16 byte boundaries. libmpeg2's altivec routines require this, for example, and if the sink doesn't align buffers, the decoder might need to allocate buffers itself leading to unnessary memcpy()ing in the sink.
I've been meaning to look at this too. I believe that for XShm, it ought to happen automatically, so we need to figure out why it isn't, if it isn't. When XShm is used, the shared memory segment should also be on a page boundary aiui, so that's automatically a 16 byte multiple. The non-XShm case is a different matter, but people should only be hitting that when running over a remote X connection.
We definitely need this for all the sse-optimised upstream elements that are doing buffer allocs.
This should be OBSOLETE in 0.11, shouldn't it? Though looking at the mpeg2dec code I am not entirely sure it really negotiates 16-byte alignment properly.
(In reply to comment #3) > This should be OBSOLETE in 0.11, shouldn't it? > > Though looking at the mpeg2dec code I am not entirely sure it really negotiates > 16-byte alignment properly. Which problem do you see in mpeg2dec? It passes an alignment of 15 (i.e. memory_address & 15 == 0) as allocation parameters to the buffer pool. But unless that's broken this is an obsolete bug in 0.11 as elements can request a specific alignment now.
I didn't say there was a problem, I meant that looking at the code it wasn't at first glance obvious to me that it does in fact negotiate/request 16-byte alignment in all code paths, or even in the most common code path. I only found this code in gst_mpeg2dec_alloc_sized_buf(): GstAllocationParams params = { 0, 15, 0, 0 }; frame->output_buffer = gst_buffer_new_allocate (NULL, size, ¶ms); but that's only used when we have to crop ourselves and not related to the bufferpool, is it? Could you point me towards the line where the alignment is configured in the normal case? Moving to mpeg2dec.
It was indeed missing in the normal case. Thanks commit a7aa984d6752a47dec3add2bc1a36492a599efc2 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon Jun 25 09:34:53 2012 +0200 mpeg2dec: Set allocation parameters to guarantee 16-byte aligned output buffers Fixes bug #377713.