GNOME Bugzilla – Bug 301783
[PATCH] enable our get/release_buffer functions in ffmpegdec
Last modified: 2005-05-06 07:48:05 UTC
This patch enable our get/release_buffer functions in ffmpegdec. There are two define in the code: - ENABLE_OUR_GET_BUFFER This define enables the patch. - FORCE_OUR_GET_BUFFER This define forces the usage of our get/release functions even if ffmpeg needs to align width or height. The performance gain is something like 20% (ex. from 25% to 19%) when playing an "aligned" video, it's ~5% (ex. from 43% to 40%) when width or height needs to be aligned. Right now there is no codec whitelist/blacklist because i don't know how to test it :) I have only xvid or divx files and they all works fine..
Created attachment 45607 [details] [review] enable our get/release buffer functions
You can remove the ENABLE_OUR_GET_BUFFER, the compile will fail without it and I don't think we want that. The FORCE_OUR_GET_BUFFER macro is probably a good idea, though. Is there a reason why you've enabled it by default? I'd recommend the opposite, since ffmpeg probably won't work (test, e.g., castaway.mov in our testsutie, http://gstreamer.freedesktop.org/media/medium/).
Oh, also, please update the gst-core requirement in configure.ac, since you use recent API.
Created attachment 45712 [details] [review] disable force_our_get_buffer by default Better patch. I've tested it with svq1 files (like castaway.mov).
Comment on attachment 45712 [details] [review] disable force_our_get_buffer by default ops, too much disabled
Created attachment 45714 [details] [review] don't disable my own patch
+ if (!gst_ffmpegdec_negotiate (ffmpegdec)) + g_assert (0); Bad, bad, bad, bad, bad you. :). Please make it use the default implementations in that case and throw an error, so we will be able to safely exit the chainfunction.
Created attachment 45981 [details] [review] better error handling (oooops)
Comment on attachment 45981 [details] [review] better error handling (oooops) this patch breaks negotiation (grrr)
Created attachment 46008 [details] [review] fix negotiation and a memory leak Q) What's last_buffer and why is it needed? A) I've tried using this patch with a program (that uses ffdec_mjpeg) and i've seen a big memory leak, debugging this problem i've found that: - ffdec calls avcodec_decode_video - ffmpeg calls release_buffer - ffmpeg calls get_buffer - ffdec push the buffer So if i change ffdec status from PLAYING to READY the last buffer is not unreffed. This patch just unrefs it.
Done, thanks a lot for the patch! We're almost completely on-par with mplayer now. :).