[vlc-devel] [PATCH] Let Render() throw an error message and return right away if the SPU region height turns out to be zero.

[Fabian Keil]

"Rémi Denis-Courmont" <remi at remlab.net> wrote:

> Le dimanche 5 juillet 2009 11:38:51 Fabian Keil, vous avez écrit :
> > The attached patch fixes a problem on FreeBSD 8.0 where vlc
> > triggers an assertion in jemalloc if the SPU region height
> > is zero.
> 
> I fail to see the link between a zero SPU region height and a crash an 
> assertion failure in libc.
> 
> > The backtrace looks like this:
> > #0  0x0000000801118eac in thr_kill () from /lib/libc.so.7
> > #1  0x00000008011afa0b in abort () from /lib/libc.so.7
> > #2  0x0000000801196e45 in __assert () from /lib/libc.so.7
> > #3  0x000000080113ae81 in malloc_usable_size () from /lib/libc.so.7
> > #4  0x000000080113e098 in posix_memalign () from /lib/libc.so.7
> > #5  0x00000008007cb842 in __vout_AllocatePicture (p_this=Variable "p_this"
> > is not available.) at vout_pictures.h:126
> 
> From the stack trace, I guess the allocated size is invalid. *But* I do not 
> understand why the problem would be _specific_ to zero height SPU region in 
> the SPU decoder. If vout_AllocatePicture() or whatever has an integer 
> underflow or some other kind of arithmetic error, I would rather have it fixed 
> that worked around.

It's not specific to a zero height SPU region, but would
happen if any of the factors used to calculate i_bytes in
src/video_output/vout_pictures.c is zero:

   570      /* Calculate how big the new image should be */
   571      i_bytes = p_pic->format.i_bits_per_pixel *
   572          i_width_aligned * i_height_aligned / 8;
   573

I assume any of those factors being zero would be an error,
but the zero height is the only problem I've seen on actual
DVDs so far.

> In principle, while useless, there is no reason why allocating an empty 
> picture should not be possible. posix_memalign() does support zero-length 
> allocation anyway:
> http://www.opengroup.org/onlinepubs/9699919799/functions/posix_memalign.html
> | If the size of the space requested is 0, the behavior is
> | implementation-defined; the value returned in memptr shall be either a
> | null pointer or a unique pointer.

I assume posix_memalign() on FreeBSD 8.0 currently
isn't as compliant as the man page says it is, then.

If you prefer not to work around this in vlc, I can forward
the patch to the maintainer of the FreeBSD port. I'll also
ask the FreeBSD developers about this.

Fabian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20090705/9c3b1235/attachment.sig>