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 775203 - validate: transcode failure with vaapi: vaapi-bufferpool is dropped in videorate
validate: transcode failure with vaapi: vaapi-bufferpool is dropped in videorate
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 774241
 
 
Reported: 2016-11-28 06:47 UTC by Hyunjun Ko
Modified: 2017-01-10 12:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
plugins: set 1 as minimum buffer to allocator query (1.02 KB, patch)
2016-11-29 06:30 UTC, Hyunjun Ko
none Details | Review
vaapivideobufferpool: adds default minimum size of vaapi bufferpool (1.13 KB, patch)
2017-01-10 06:24 UTC, Hyunjun Ko
committed Details | Review
plugins: use the default value as minimum buffer of vaapibufferpool (1.48 KB, patch)
2017-01-10 06:25 UTC, Hyunjun Ko
committed Details | Review

Description Hyunjun Ko 2016-11-28 06:47:24 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
Comment 1 Hyunjun Ko 2016-11-29 06:30:51 UTC
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.
Comment 2 Hyunjun Ko 2016-11-29 06:33:05 UTC
(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?
Comment 3 Víctor Manuel Jáquez Leal 2017-01-06 11:50:46 UTC
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.
Comment 4 Hyunjun Ko 2017-01-10 06:24:40 UTC
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.
Comment 5 Hyunjun Ko 2017-01-10 06:25:11 UTC
Created attachment 343213 [details] [review]
plugins: use the default value as minimum buffer of  vaapibufferpool
Comment 6 Hyunjun Ko 2017-01-10 06:36:49 UTC
(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) :)
Comment 7 Víctor Manuel Jáquez Leal 2017-01-10 12:54:53 UTC
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>