Skip to content
This repository

Workaround for swscale crashing on pictures with odd number of pixels in width #1181

Merged
merged 1 commit into from over 1 year ago

3 participants

Memphiz jmarshallnz Joakim Plate
Memphiz
Owner

i have kind off tracked down the crash mentioned here:

http://forum.xbmc.org/showthread.php?tid=136153&page=2

Its easy to reproduce. Add a fake movie "Cars (2006)" - activate actors thumbnails and scrape it using TMDB. After it scraped go to info -> cast and scroll to "George Carlin".

It will download the actors image from here http://cf2.imgobject.com/t/p/original/5WSoAFzPlu1ZpKB3O8mqfIGOXB.jpg

and try to scale it to 720 pixels height.

This image is 753 pixels wide (odd number of pixels). libswscale will then do somthing in the context and sets context->chrSrcW to 377 (which should be the half but is somehow wrong rounded imho). It then accesses the source image with 377 as strideidx and gets a bad_access.

I couldn't track it down - i even tried to replicate the crash with the cmdline tools from ffmpeg and libav - but both use the mjpeg decoder/encoder for resizing on commandline instead of libswscale. So i couldn't reproduce it.

output of ffmpeg tool:

http://pastebin.com/DKS0gK5V

It looks like its not using swscale at all.

I have done multiple tests with odd width in input and output image - crash only is definitly there when input image has odd width. The crash doesn't seem to occur on all platforms (doesn't crash on my atv2, but on my ipod and my osx rig).

I also played a bit with scaling flags see here:

http://pastebin.com/xptE3Vq2

Well nothing helped, but the commit in this PR (which feels quiet hacky).

@elupus @Anssi your comments are welcome :)

jmarshallnz
Owner

A few more comments:

  1. The rounding done by swscale is intentional - they're rounding towards +inf as commented in the code.

  2. I suspect the reason that ffmpeg doesn't crash is because each row is padded out there to at least even (more probably aligned), so that it doesn't run into no-mans land.

  3. The reason for no crash on various platforms could potentially be due to differently optimised paths (different optims may have/not have the bug).

  4. Latest ffmpeg/libav appears to have a condition for where the destination is odd width it does full chroma subsampling - this may mask the issue in cases where the destination is odd (which is true for the image above, rescaled to height 720), but slows down the routine, ofcourse: https://lists.ffmpeg.org/pipermail/ffmpeg-cvslog/2012-January/045785.html

  5. A simple alternate fix would be just padding the textures to at least even width, assuming that this is what swscale is expecting. Given there's no suggestion of this in the docs for swscale, it is possibly a bug there?

Memphiz
Owner

Our forum moderators get a bit bumpy on this. They fear as more and more users are rebuilding there databases this crsh could pop up a lot in the forum.

@elupus a word?

Joakim Plate
Collaborator
elupus commented July 26, 2012

We've hit this issue before on video (there is likely a bug in swscale here). There we resorted to always making sure the buffer written to had a total size aligned to 16. The source was pretty much already aligned that way since it came from libavcodec to start with.

Joakim Plate
Collaborator
elupus commented July 26, 2012

That said it would be better to patch ffmpeg or backport any fixes from upstream.

Memphiz
Owner

Well i didn't see any upstream fixes for that nor do i know where this patch in ffmpeg would need to be. But i only watch the libav stuff (very seldom though). I will try to dig into the ffmpeg git and find something usefull...

Memphiz
Owner

This doesn't seem to be fixed in current ffmpeg trunk. Though i have written a little test util which does the same thing which XBMC does, but it doesn't crash.

This thing really starts to get annoying. The testtool gives only a "[swscaler @ 0x801000] Warning: data is not aligned! This can lead to a speedloss" but it doesn't crash.

So what about aligning the texture memory to even width? I'm not really in the mood for digging into it more...

Memphiz
Owner

http://pastebin.com/a4p5QSdU <- this also fixes it (aligning the texturewidth to a even number of bytes)

jmarshallnz
Owner

IMO the alternate fix is probably better, as it ensures that the texture width stays the same so we don't lose any pixels when downsampling. Obviously it would need decent commentary as to why it's being done.

In addition, providing the util would be useful as it might provide additional insight into what the crash is (eg it appears to be platform specific?)

Memphiz
Owner

Well here is the tool. But it doesn't reproduce the crash. Its platform and state specific as it seems.

http://pastebin.com/xrXjtjeR

It crashs in xbmc on osx all the time but maybe sometimes really seldom it doesn't. It crashs on iOS 4.3 on atv2 and it doesn't on atv2 with iOS 5.0.1.

So its like some memory corruption or something really strange.

jmarshallnz
Owner

Thanks - so you can't crash with that using our ffmpeg?

Memphiz
Owner

Yep it doesn't crash ... i just linked in our libswscale.a and libavutil.a from osx (i linked against different versions - hence the commented lines in the makefile - but it never crashed).

jmarshallnz
Owner

Is the pitch in XBMC the same as in your test? Also, does eliminating -m32 make any difference? Seems most strange that you can't repro using this - I'll have a play this evening.

Memphiz
Owner

Pitch is 4 like in xbmc. Also the cpuflags. I've basically stuck in the debugger and copied all parameters over to that test tool.

Well but its easy to reproduce in xbmc itself aswell.

Just scrape a fake movie "Cars" with TMDB. And after that you have to watch that artist called "george carlin".

This will pull and rescale the offender hehe http://cf2.imgobject.com/t/p/original/5WSoAFzPlu1ZpKB3O8mqfIGOXB.jpg

And should crash. It won't crash on linux btw as of vdrfan who tested it there.

Beside that i have updated this pr with (only aligning width to 2 - since this is enough - no need to have an aligned height) - in case nothing better comes up.

Memphiz
Owner

Ahh the -m32. I had to add this because our ffmpeg is built 32 bit - but my osx compiler defaults to 64bit - so the link would fail. I also did a complete 64bit build of the testtool and our ffmpeg - no change - it doesn't crash sadly.

I even tried with the data from the offending image (in case swscale would look on the image data itself - which i really doubt). Nothing changed in that case - still no crash.

Memphiz
Owner

I will merge this in august merge window - no point for leaving it crashing there...

jmarshallnz
Owner
Memphiz Memphiz merged commit 6e2ddbd into from July 31, 2012
Memphiz Memphiz closed this July 31, 2012
Memphiz
Owner

yeah even better :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Jul 26, 2012
Memphiz [fix] - workaround for swscale crashing on images which have an uneve…
…n number of pixels in width
88d6eed
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 10 additions and 0 deletions. Show diff stats Hide diff stats

  1. 10  xbmc/guilib/Texture.cpp
10  xbmc/guilib/Texture.cpp
@@ -91,6 +91,16 @@ void CBaseTexture::Allocate(unsigned int width, unsigned int height, unsigned in
91 91
     m_textureWidth = ((m_textureWidth + 3) / 4) * 4;
92 92
     m_textureHeight = ((m_textureHeight + 3) / 4) * 4;
93 93
   }
  94
+  else
  95
+  {
  96
+    // align all textures so that they have an even width
  97
+    // in some circumstances when we downsize a thumbnail
  98
+    // which has an uneven number of pixels in width
  99
+    // we crash in CPicture::ScaleImage in ffmpegs swscale
  100
+    // because it tries to access beyond the source memory
  101
+    // (happens on osx and ios)
  102
+    m_textureWidth = ((m_textureWidth + 1) / 2) * 2;
  103
+  }
94 104
 
95 105
   // check for max texture size
96 106
   #define CLAMP(x, y) { if (x > y) x = y; }
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.