GNOME Bugzilla – Bug 775203
validate: transcode failure with vaapi: vaapi-bufferpool is dropped in videorate
Last modified: 2017-01-10 12:54:53 UTC
Failures: validate.file.transcode.to_vorbis_and_h264_in_mkv.raw_video_avi validate.file.transcode.to_vorbis_and_vp8_in_webm.raw_video_avi This occurs since the commit https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/commit/?id=acefc7e384bd0112a4406f1062cbe9c95b9feed7 is landed. In the propose_allocation of videorate, it drops vaapi-bufferpool when min==max while vaapi allocator is passed to upstream. See this https://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst/videorate/gstvideorate.c#n1095 We can reproduce with this simple pipeline. gst-launch-1.0 videotestsrc ! videorate ! vaapih264enc ! fakesink
Created attachment 340960 [details] [review] plugins: set 1 as minimum buffer to allocator query Set 1 as minimum buffer for other element to ensure to keep 1 buffer at least.
(In reply to Hyunjun Ko from comment #1) > Created attachment 340960 [details] [review] [review] > plugins: set 1 as minimum buffer to allocator query > > Set 1 as minimum buffer for other element to ensure to keep 1 buffer at > least. This fixes this issue, but not sure. I don't know what's difference between setting 0 and setting 1 as minimum. Any thought or info about this?
Review of attachment 340960 [details] [review]: ::: gst/vaapi/gstvaapipluginbase.c @@ +793,1 @@ gst_query_add_allocation_param (query, plugin->sinkpad_allocator, NULL); Interesting. It seems that videorate, if an allocator is proposed, it tries to use it, regardless the buffer pool. And our vaapi allocator cannot be used outside of the vaapi video buffer poll. And, if an allocator is proposed, it needs at least one spare buffer. I propose two things: 1\ instead of one buffer as minimum, set 2, which is more safe 2\ if you change this, you have to instantiate 2 buffers when configuring the buffer pool. You have to change that in ensure_sinkpad_buffer_pool() And please test this change intensively with vaapisink.
Created attachment 343212 [details] [review] vaapivideobufferpool: adds default minimum size of vaapi bufferpool Adds default value of minimum buffer. This would be used when creating and proposing vaapi bufferpool, so that other elements ensure to keep 2 buffers at least.
Created attachment 343213 [details] [review] plugins: use the default value as minimum buffer of vaapibufferpool
(In reply to Víctor Manuel Jáquez Leal from comment #3) > And please test this change intensively with vaapisink. As far as I tested, no problem happens by this patch. (with playback testcases and some pipelines that I usually test) :)
I have merge both patch in a single one, without exporting a new symbol in the header file since it just used locally to pluginbase. commit bca2b1680bf07a1f7c593eb3aa1b04a354c47e6f Author: Hyunjun Ko <zzoon@igalia.com> Date: Tue Jan 10 15:15:31 2017 +0900 plugins: provide at least two buffers in sink pool Adds two buffers as the default value of minimum buffer. This would be used when creating and proposing vaapi bufferpool for sink pad, hence the upstream element will keep, at least, these two buffers. https://bugzilla.gnome.org/show_bug.cgi?id=775203 Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>