[gstreamer-bugs] [Bug 572863] [32bit] ffdec_nellymoser seg faults (misaligned data)

[GStreamer (bugzilla.gnome.org)]

If you have any questions why you received this email, please see the text at
the end of this email. Replies to this email are NOT read, please see the text
at the end of this email. You can add comments to this bug at:
  http://bugzilla.gnome.org/show_bug.cgi?id=572863

  GStreamer | gst-ffmpeg | Ver: git




------- Comment #11 from Edward Hervey  2009-03-06 15:04 UTC -------
(In reply to comment #10)
> (From update of attachment 130186 [edit])
> >-  /* outgoing buffer */
> >-  *outbuf = gst_buffer_new_and_alloc (AVCODEC_MAX_AUDIO_FRAME_SIZE);
> 
> Oh wow, I would have assumed g_malloc() provides a reasonably aligned return
> value.

  Nope, it only provides (arch-size) bytes alignment (i.e. on 32bit : 4 bytes,
and on 64bits: 8 bytes). The ffmpeg implementation does 128bit (16bytes)
alignment (by using memalign/posix_memalign).

> Maybe we should fix gst_buffer_new_and_alloc() to do this automatically?
> Or provide new functions for that? (Both doesn't help us now of course)

  Was thinking about that indeed. Considering that any sse/sse2 code (and maybe
other technologies) require aligned memory... it would make much more sense.
  New functions would make more sense.

> 
> >+  /* outgoing buffer. We use av_malloc() to have properly aligned memory. */
> >+  *outbuf = gst_buffer_new ();
> >+  GST_BUFFER_DATA (*outbuf) = GST_BUFFER_MALLOCDATA (*outbuf) = av_malloc(AVCODEC_MAX_AUDIO_FRAME_SIZE);
> >+  GST_BUFFER_SIZE (*outbuf) = AVCODEC_MAX_AUDIO_FRAME_SIZE;
> 
> Don't we also need an GST_BUFFER_FREE_FUNC (*outbuf) = av_free; ?

  We could for safety, although it will not be doing anything fancy (apart from
calling free) if we have CONFIG_MEMALIGN_HACK deactivated (which is the case in
our supported builds).

> 
> What about the input buffer? Not sure how to read the docs, but it seems like
> the input buffer needs to be at least 4-byte aligned?
> 
  Indeed. I guess we could check if the input buffer is 32bit aligned, and if
not, create a temporary buffer with av_malloc(), pass that to the decoding
function, and then free it.

  Will create an updated patch for this and test it.


-- 
See http://bugzilla.gnome.org/page.cgi?id=email.html for more info about why you received
this email, why you can't respond via email, how to stop receiving
emails (or reduce the number you receive), and how to contact someone
if you are having problems with the system.

You can add comments to this bug at http://bugzilla.gnome.org/show_bug.cgi?id=572863.