MemoryAlignment: Fixes #10810

fritsch · 2016-10-29T16:54:34Z

Whenever we provide a user buffer directly into ffmpeg we should take care that its alignment is 32 byte, cause our ffmpeg build with AVX support might do nasty things.

This PR does two things:
a) Use av_malloc when we directly communicate with ffmpeg / sws_scale and so on
b) Use _aligned_malloc(size, 32) whenever we don't want ffmpeg dependencies in that other code, e.g. Texture.cpp and friends.

fritsch · 2016-10-29T16:59:14Z

Hi @FernetMenta please have a look. I think I did not get the remarks correctly yet.

xbmc/guilib/Texture.cpp

@@ -108,7 +108,7 @@ void CBaseTexture::Allocate(unsigned int width, unsigned int height, unsigned in
  if (GetPitch() * GetRows() > 0)
  {
    size_t size = GetPitch() * GetRows();
-    m_pixels = (unsigned char*) _aligned_malloc(size, 16);
+    m_pixels = (unsigned char*) _aligned_malloc(size, 32);


xbmc/cores/VideoPlayer/DVDDemuxers/DVDDemuxUtils.cpp

@@ -68,7 +65,7 @@ DemuxPacket* CDVDDemuxUtils::AllocateDemuxPacket(int iDataSize)
        * Note, if the first 23 bits of the additional bytes are not 0 then damaged
        * MPEG bitstreams could cause overread and segfault
        */
-      pPacket->pData =(uint8_t*)_aligned_malloc(iDataSize + FF_INPUT_BUFFER_PADDING_SIZE, 16);
+      pPacket->pData =(uint8_t*)av_malloc(iDataSize + FF_INPUT_BUFFER_PADDING_SIZE);


fritsch · 2016-10-29T18:03:36Z

jenkins build this please

xbmc/cores/VideoPlayer/DVDDemuxers/DVDDemuxUtils.cpp

@@ -68,7 +68,7 @@ DemuxPacket* CDVDDemuxUtils::AllocateDemuxPacket(int iDataSize)
        * Note, if the first 23 bits of the additional bytes are not 0 then damaged
        * MPEG bitstreams could cause overread and segfault
        */
-      pPacket->pData =(uint8_t*)_aligned_malloc(iDataSize + FF_INPUT_BUFFER_PADDING_SIZE, 16);
+      pPacket->pData =(uint8_t*)_aligned_malloc(iDataSize + FF_INPUT_BUFFER_PADDING_SIZE, 32);


fritsch · 2016-10-29T19:11:16Z

jenkins build this please

akva2 · 2016-10-31T09:31:53Z

with aadfc21 image support is completely broken on my machine. reverting it restores operation.

fritsch · 2016-10-31T09:35:50Z

See: #10826 thx for reporting back.

xbmc/guilib/FFmpegImage.cpp

@@ -404,7 +404,7 @@ bool CFFmpegImage::DecodeFrame(AVFrame* frame, unsigned int width, unsigned int

  bool needsCopy = false;
  int pixelsSize = pitch * height;
-  bool aligned = (((uintptr_t)(const void *)(pixels)) % (16) == 0);
+  bool aligned = (((uintptr_t)(const void *)(pixels)) % (32) == 0);


fritsch · 2017-02-03T15:22:38Z

One is the address. The other the entries of the buffer beginning at that address. Should be fine, right? Am 03.02.2017 2:46 nachm. schrieb "Chris Browet" <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In xbmc/guilib/FFmpegImage.cpp <#10810 (review)>: > @@ -404,7 +404,7 @@ bool CFFmpegImage::DecodeFrame(AVFrame* frame, unsigned int width, unsigned int bool needsCopy = false; int pixelsSize = pitch * height; - bool aligned = (((uintptr_t)(const void *)(pixels)) % (16) == 0); + bool aligned = (((uintptr_t)(const void *)(pixels)) % (32) == 0); You probably want to align on 32 in the av_frame_get_buffer below, too — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#10810 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABCfHWKcEs4PSt3Hd9ILo5yozRIin1Ugks5rYy-7gaJpZM4KkKS-> .

koying · 2017-02-03T15:26:43Z

Are we on the same page? ;)
I mean https://github.com/fritsch/xbmc/blob/2bf1886fb80dff007a4f37ca55a5e38830d0609f/xbmc/guilib/FFmpegImage.cpp#L422 should be aligned on 32 as well, I guess

fritsch · 2017-02-03T15:41:38Z

Nope, we aren't. There is the "alignment" of the memory adress, so that intrinsics can properly work and the alignment of the size of the buffer. See: https://ffmpeg.org/doxygen/2.2/frame_8c_source.html line 134 for example. For the rest ffmpeg cares.

…

-- Key-ID: 0x1A995A9B keyserver: pgp.mit.edu ============================================================== Fingerprint: 4606 DA19 EC2E 9A0B 0157 C81B DA07 CF63 1A99 5A9B

fritsch force-pushed the avmalloc branch from 46d408a to 33713e0 Compare October 29, 2016 16:58

fritsch assigned FernetMenta Oct 29, 2016

FernetMenta reviewed Oct 29, 2016

View reviewed changes

FernetMenta added Type: Fix non-breaking change which fixes an issue v17 Krypton labels Oct 29, 2016

FernetMenta added this to the Krypton 17.0-beta6 milestone Oct 29, 2016

fritsch force-pushed the avmalloc branch from 33713e0 to 98105f6 Compare October 29, 2016 17:26

fritsch added 4 commits October 29, 2016 19:54

Picture: Use av_malloc when using ffmpeg scalers

aadfc21

DVDVideoPPFFmpeg: Use av_malloc and av_free

f5f9b8d

ActiveAESink: Align to 32 bytes for using AVX

79f14f5

Texture: Align to 32 byte - we might end up in ffmpeg buffer

2bf1886

fritsch force-pushed the avmalloc branch from 98105f6 to 13368c8 Compare October 29, 2016 17:54

FernetMenta reviewed Oct 29, 2016

View reviewed changes

fritsch force-pushed the avmalloc branch from 13368c8 to 2bf1886 Compare October 29, 2016 19:09

fritsch merged commit abc5af8 into xbmc:master Oct 30, 2016

koying reviewed Feb 3, 2017

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MemoryAlignment: Fixes #10810

MemoryAlignment: Fixes #10810

fritsch commented Oct 29, 2016

fritsch commented Oct 29, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

fritsch commented Oct 29, 2016

This comment was marked as spam.

This comment was marked as spam.

fritsch commented Oct 29, 2016

akva2 commented Oct 31, 2016

fritsch commented Oct 31, 2016

This comment was marked as spam.

fritsch commented Feb 3, 2017 via email

koying commented Feb 3, 2017

fritsch commented Feb 3, 2017 via email •

edited

MemoryAlignment: Fixes #10810

MemoryAlignment: Fixes #10810

Conversation

fritsch commented Oct 29, 2016

fritsch commented Oct 29, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

fritsch commented Oct 29, 2016

This comment was marked as spam.

This comment was marked as spam.

fritsch commented Oct 29, 2016

akva2 commented Oct 31, 2016

fritsch commented Oct 31, 2016

This comment was marked as spam.

fritsch commented Feb 3, 2017 via email

koying commented Feb 3, 2017

fritsch commented Feb 3, 2017 via email • edited

fritsch commented Feb 3, 2017 via email •

edited