After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 321662 - reenable our get_buffer function in gst-ffmpeg
reenable our get_buffer function in gst-ffmpeg
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-libav
git master
Other Linux
: Normal normal
: 0.10.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-11-16 22:49 UTC by Luca Ognibene
Modified: 2008-01-22 16:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for our get/release buffer functions in gstffmpeggdecoders (2.06 KB, patch)
2006-02-13 12:58 UTC, Edward Hervey
none Details | Review

Description Luca Ognibene 2005-11-16 22:49:29 UTC
When seeking in a pipeline using ffdec_* elements it can happens an error like:
(lt-totem:7911): GLib-GObject-WARNING **: invalid uninstantiatable type
`<invalid>' in cast to `GstMiniObject'
and some segfaults. I've tried to fix it but my fix doesn't really works..

Some info:
 - a buffer is allocated in _get_buffer. The buffer exits this function with a
ref count of 2.
 - a buffer is deallocated in _release_buffer and in _frame. It can also be
deallocated when another element unrefs it. _release_buffer is called when we
call avcodec_decode_video or avcodec_flush_buffers. 

So, every buffer has a refcount of 2 when it's created using our _geT_buffer
function. Every buffer should also be unreffed exactly two times :) 
This is not the case right now so i've disabled our _get_buffer function
(ffdec_* elements will use more cpu but they should work fine).
Comment 1 Ronald Bultje 2005-11-16 23:11:23 UTC
Make sure to set our external picture reference in GstFFMpegDec to NULL after calling __flush_buffers(), 
that is the most likely culprit.
Comment 2 Edward Hervey 2006-02-13 12:56:04 UTC
After looking a bit further into it, the problem is when ffmpeg calls our get_buffer when the src pad is flushing.
When we return -1, ffmpeg assumes the previous allocated buffer is to be re-used.
And all hell breaks loose, because that means ffmpeg will call our release_buffer functions twice !

A quick fix for this is to always allocate a valid buffer in get_buffer(). It works fine with the attached patch.
Comment 3 Edward Hervey 2006-02-13 12:58:58 UTC
Created attachment 59255 [details] [review]
patch for our get/release buffer functions in gstffmpeggdecoders
Comment 4 Luca Ognibene 2006-02-13 15:00:30 UTC
I've not tested the patch but i think you should leave FORCE_OUR_GET_BUFFER disabled. I remember there were some problems with some qt video in 0.8 with that flag enabled. 
To enable our get_buffer function on supported buffer size you should just remove the "|| 1" before the "#ifdef FORCE_OUR_GET_BUFFER". 
Comment 5 Wim Taymans 2006-02-16 17:45:14 UTC
ping?
Comment 6 Sebastian Dröge (slomo) 2007-07-11 07:19:20 UTC
ping²?
Comment 7 Edward Hervey 2007-12-18 10:45:31 UTC
Still has issues.

ffmpeg is STILL writing outside of the allocated memory.
Comment 8 Wim Taymans 2008-01-22 16:33:43 UTC
        * ext/ffmpeg/gstffmpegdec.c: (gst_ffmpegdec_class_init),
        (gst_ffmpegdec_init), (gst_ffmpegdec_close), (gst_ffmpegdec_open),
        (gst_ffmpegdec_setcaps), (gst_ffmpegdec_get_buffer),
        (gst_ffmpegdec_release_buffer), (get_output_buffer),
        (gst_ffmpegdec_video_frame), (gst_ffmpegdec_audio_frame),
        (gst_ffmpegdec_frame), (gst_ffmpegdec_change_state),
        (gst_ffmpegdec_set_property), (gst_ffmpegdec_get_property):
        Reenable pad_alloc, seem to work now.
        Added property to easily disable it later on.
        Remove some old code that tried hard to break the get_buffer
        functions. Fixes #321662.