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 785054 - if downstream doesn't support GstVideoMeta, VAAPI has to copy to another buffer and convert the stride
if downstream doesn't support GstVideoMeta, VAAPI has to copy to another buff...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal major
: 1.13.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-07-18 10:07 UTC by Víctor Manuel Jáquez Leal
Modified: 2018-02-21 16:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vaapibufferpool: don't change config when forcing video meta (2.68 KB, patch)
2018-02-20 13:28 UTC, Víctor Manuel Jáquez Leal
none Details | Review
vaapibufferpool: add gst_vaapi_video_buffer_pool_copy_buffer() (2.73 KB, patch)
2018-02-20 13:28 UTC, Víctor Manuel Jáquez Leal
none Details | Review
plugins: store the first downstream allocator if available (2.59 KB, patch)
2018-02-20 13:28 UTC, Víctor Manuel Jáquez Leal
none Details | Review
plugins: add COPY_OUTPUT_FRAME flag (2.15 KB, patch)
2018-02-20 13:28 UTC, Víctor Manuel Jáquez Leal
none Details | Review
plugins: add gst_vaapi_copy_va_buffer() (3.39 KB, patch)
2018-02-20 13:28 UTC, Víctor Manuel Jáquez Leal
none Details | Review
vaapipostproc: handle system allocated buffers when required (5.62 KB, patch)
2018-02-20 13:28 UTC, Víctor Manuel Jáquez Leal
none Details | Review
vaapidecode: generate system allocated buffers (2.79 KB, patch)
2018-02-20 13:28 UTC, Víctor Manuel Jáquez Leal
none Details | Review
vaapibufferpool: don't change config when forcing video meta (2.68 KB, patch)
2018-02-20 17:17 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapibufferpool: add gst_vaapi_video_buffer_pool_copy_buffer() (2.73 KB, patch)
2018-02-20 17:17 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
plugins: store the first downstream allocator if available (2.59 KB, patch)
2018-02-20 17:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
plugins: add COPY_OUTPUT_FRAME flag (2.15 KB, patch)
2018-02-20 17:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
plugins: add gst_vaapi_copy_va_buffer() (3.39 KB, patch)
2018-02-20 17:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapipostproc: handle system allocated buffers when required (5.54 KB, patch)
2018-02-20 17:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapidecode: generate system allocated buffers (2.79 KB, patch)
2018-02-20 17:18 UTC, Víctor Manuel Jáquez Leal
committed Details | Review

Description Víctor Manuel Jáquez Leal 2017-07-18 10:07:53 UTC
VAAPI decoder and postprocessor have a historical problem with downstream elements without GstVideoMeta support, since VA surfaces use special strides.

In bug 766184 we added a wrong solution, forcing the GstVideoMeta even if it is not requested.

The correct solution is to copy the VA surface onto another buffer (provided perhaps by downstream).

For sake of completion I add the IRC discussion:

2017-07-18 11:26:58	slomo	ceyusa: is vaapidec always adding GstVideoMeta, or first checking if downstream can actually handle that and if not do something else (copying)?
2017-07-18 11:31:35	ceyusa	slomo: when the vaapi buffer pool is configured, if the configuation doesn't request GST_BUFFER_POOL_OPTION_VIDEO_META, it adds it
2017-07-18 11:32:12	ceyusa	then, when the buffer pool allocs a buffer, checks for the configuration and if it is, the meta is added
2017-07-18 11:33:04	slomo	ceyusa: what if downstream does not provide any buffer pool?
2017-07-18 11:35:10	ceyusa	slomo: vaapi always use its own buffer pool (except for sink pad in decoders)
2017-07-18 11:35:24	slomo	ok, so that's wrong
2017-07-18 11:35:43	ceyusa	why?
2017-07-18 11:36:05	slomo	you must not put videometa on buffers unless downstream claims to support videometa in the response to the allocation query
2017-07-18 11:37:41	ceyusa	slomo it was added because of https://bugzilla.gnome.org/show_bug.cgi?id=766184
2017-07-18 11:38:29	slomo	ceyusa: if downstream does not support it, you have to use the default strides/offsets. and if you can't, you have to copy to another buffer and convert the stride
2017-07-18 11:40:47	slomo	ceyusa: without that, people using vaapi will get corrupted data if they use appsink, fakesink, or their own sink that does not support videometa
2017-07-18 11:41:03	slomo	if videometa is not supported, you have to use the defaults
Comment 1 Nicolas Dufresne (ndufresne) 2017-11-23 21:07:34 UTC
Any progress on this ? Btw, as long as you can map the buffers, gst_video_frame_copy() will do the right thing in converting strides and offsets, so it should not be that difficult.
Comment 2 Víctor Manuel Jáquez Leal 2017-11-27 13:29:11 UTC
(In reply to Nicolas Dufresne (stormer) from comment #1)
> Any progress on this ? Btw, as long as you can map the buffers,
> gst_video_frame_copy() will do the right thing in converting strides and
> offsets, so it should not be that difficult.

I haven't started on in this issue. I wonder from which buffer pool should I get the "plain" buffer (without video meta): downstream? create a new one?
Comment 3 Víctor Manuel Jáquez Leal 2018-02-20 13:28:26 UTC
Created attachment 368638 [details] [review]
vaapibufferpool: don't change config when forcing video meta

VA-API based buffer might need a video meta because of different
strides. But when donwstream doesn't support video meta we need to
force the usage of video meta.

Before we changed the buffer pool configuration, but actually this
is a hack and we cannot rely on that for downstream.

This patch add a check fo raw video caps and allocator is VA-API,
then the option is enabled without changing the pool configuration.
In this case the element is responsible to copy the frame to a
simple buffer with the expected strides.
Comment 4 Víctor Manuel Jáquez Leal 2018-02-20 13:28:31 UTC
Created attachment 368639 [details] [review]
vaapibufferpool: add gst_vaapi_video_buffer_pool_copy_buffer()

This function will inform the element if it shall copy the generated
buffer by the pool to a system allocated buffer before pushing it
to downstream.
Comment 5 Víctor Manuel Jáquez Leal 2018-02-20 13:28:36 UTC
Created attachment 368640 [details] [review]
plugins: store the first downstream allocator if available

The allocator will be required if we need to allocate a buffer
to store the frame with the expected strides.
Comment 6 Víctor Manuel Jáquez Leal 2018-02-20 13:28:40 UTC
Created attachment 368641 [details] [review]
plugins: add COPY_OUTPUT_FRAME flag

This patch add the member copy_output_frame and set it TRUE when
when downstream didn't request GstVideoMeta API, the caps are raw
and the internal allocator is the VA-API one.
Comment 7 Víctor Manuel Jáquez Leal 2018-02-20 13:28:46 UTC
Created attachment 368642 [details] [review]
plugins: add gst_vaapi_copy_va_buffer()

This helper function aims to copy buffers with VA memory to dumb
buffers, when GstVideoMeta is not available dowstream.
Comment 8 Víctor Manuel Jáquez Leal 2018-02-20 13:28:50 UTC
Created attachment 368643 [details] [review]
vaapipostproc: handle system allocated buffers when required

When downstream can't handle GstVideoMeta it is required to send
system allocated buffers.

The system allocated buffers are produced in prepare_output_buffer()
vmethod if downstream can't handl GstVideoMeta.

At transform() vmethod if the buffer is a system allocated buffer,
a VA buffer is instanciated and replaces the out buffer. Later
the VA buffer is copied to the system allocate buffer and it
replaces the output buffer.
Comment 9 Víctor Manuel Jáquez Leal 2018-02-20 13:28:55 UTC
Created attachment 368644 [details] [review]
vaapidecode: generate system allocated buffers

Generate system allocated output buffers when downstream doesn't
support GstVideoMeta.

The VA buffer content is copied to the new output buffer, and it
replaces the VA buffer.
Comment 10 Víctor Manuel Jáquez Leal 2018-02-20 17:17:50 UTC
Created attachment 368665 [details] [review]
vaapibufferpool: don't change config when forcing video meta

VA-API based buffer might need a video meta because of different
strides. But when donwstream doesn't support video meta we need to
force the usage of video meta.

Before we changed the buffer pool configuration, but actually this
is a hack and we cannot rely on that for downstream.

This patch add a check fo raw video caps and allocator is VA-API,
then the option is enabled without changing the pool configuration.
In this case the element is responsible to copy the frame to a
simple buffer with the expected strides.
Comment 11 Víctor Manuel Jáquez Leal 2018-02-20 17:17:55 UTC
Created attachment 368666 [details] [review]
vaapibufferpool: add gst_vaapi_video_buffer_pool_copy_buffer()

This function will inform the element if it shall copy the generated
buffer by the pool to a system allocated buffer before pushing it
to downstream.
Comment 12 Víctor Manuel Jáquez Leal 2018-02-20 17:18:00 UTC
Created attachment 368667 [details] [review]
plugins: store the first downstream allocator if available

The allocator will be required if we need to allocate a buffer
to store the frame with the expected strides.
Comment 13 Víctor Manuel Jáquez Leal 2018-02-20 17:18:06 UTC
Created attachment 368668 [details] [review]
plugins: add COPY_OUTPUT_FRAME flag

This patch add the member copy_output_frame and set it TRUE when
when downstream didn't request GstVideoMeta API, the caps are raw
and the internal allocator is the VA-API one.
Comment 14 Víctor Manuel Jáquez Leal 2018-02-20 17:18:11 UTC
Created attachment 368669 [details] [review]
plugins: add gst_vaapi_copy_va_buffer()

This helper function aims to copy buffers with VA memory to dumb
buffers, when GstVideoMeta is not available dowstream.
Comment 15 Víctor Manuel Jáquez Leal 2018-02-20 17:18:17 UTC
Created attachment 368670 [details] [review]
vaapipostproc: handle system allocated buffers when required

When downstream can't handle GstVideoMeta it is required to send
system allocated buffers.

The system allocated buffers are produced in prepare_output_buffer()
vmethod if downstream can't handl GstVideoMeta.

At transform() vmethod if the buffer is a system allocated buffer,
a VA buffer is instanciated and replaces the out buffer. Later
the VA buffer is copied to the system allocate buffer and it
replaces the output buffer.
Comment 16 Víctor Manuel Jáquez Leal 2018-02-20 17:18:23 UTC
Created attachment 368671 [details] [review]
vaapidecode: generate system allocated buffers

Generate system allocated output buffers when downstream doesn't
support GstVideoMeta.

The VA buffer content is copied to the new output buffer, and it
replaces the VA buffer.
Comment 17 Víctor Manuel Jáquez Leal 2018-02-21 16:03:26 UTC
Attachment 368665 [details] pushed as ad705cc - vaapibufferpool: don't change config when forcing video meta
Attachment 368666 [details] pushed as bcc480b - vaapibufferpool: add gst_vaapi_video_buffer_pool_copy_buffer()
Attachment 368667 [details] pushed as f0fd2ae - plugins: store the first downstream allocator if available
Attachment 368668 [details] pushed as 5842e9c - plugins: add COPY_OUTPUT_FRAME flag
Attachment 368669 [details] pushed as 2c36610 - plugins: add gst_vaapi_copy_va_buffer()
Attachment 368670 [details] pushed as 188434f - vaapipostproc: handle system allocated buffers when required
Attachment 368671 [details] pushed as 9827ab0 - vaapidecode: generate system allocated buffers