GNOME Bugzilla – Bug 783599
plugins: refactor the handling of negotiated caps vs allocation caps
Last modified: 2017-07-10 17:56:44 UTC
Since bug 781577 a regression were introduced: when playing back videos without using vaapipostproc and software based renderers, the frames cannot be mapped because the resolutions doesn't match. These patches are big refactoring for handling caps when the negotiated and allocation are different.
Created attachment 353467 [details] [review] vaapivideobufferpool: rename video info structures Renamed private GstVideoInfo structure video_info to allocation_vinfo and alloc_info to negotiated_vinfo. The purpose of these renaming is to clarify the origin and purpose of these private variables: video_info (now allocation_vinfo) comes from the bufferpool configuration. It describes the physical video resolution to be allocated by the allocator, which may be different from the negotiated one. alloc_info (now vmeta_vinfo) comes from the negotiated caps in the pipeline. It represents how the frame is going to be mapped using the video meta. In Intel's VA-API backend, the allocation_vinfo resolution is bigger than the negotiated_info.
Created attachment 353468 [details] [review] vaapivideobufferpool: rename local variables Renamed local video info structure names in set_config() vitual method. The purpose of their renaming is to clarify the origin of those structures, whether come from passed caps parameter (new_allocation_vinfo) or from the configured allocator (allocator_vinfo).
Created attachment 353469 [details] [review] vaapivideomemory: rename qdata quarks and ids Also the parameter names were renamed to reflect their origin and purpose.
Created attachment 353470 [details] [review] plugins: distinguish allocation and negotiation caps The vaapi video decoders might have different allocation caps from the negotiation caps, thus the GstVideoMeta shall use the negotiation caps, not the allocation caps. This was done before reusing gst_allocator_get_vaapi_video_info(), storing there the negotiation caps if they differ from the allocation ones, but this strategy felt short when the allocator had to be reset in the vaapi buffer pool, since we need both. This patch adds gst_allocator_set_vaapi_negotiated_video_info() and gst_allocator_get_vaapi_negotiated_video_info() to store the negotiated video info in the allocator, and distinguish it from the allocation video info.
Created attachment 353471 [details] [review] vaapivideobufferpool: refactor set_config() Refactor the set_config() virtual method considering a cleaner approach to allocator instanciation, if it it not set or if it is not valid for the pool.
Created attachment 353472 [details] [review] vaapivideobufferpool: remove allocation_vinfo private attribute There is no need to keep this attribute internally since it is already managed by the allocator.
So if I understand well, in VAAPI we have allocation caps != negotiated caps. So using this method, we are forced to have a buffer pool that is aware of the GstElement in order to receive the negotiated caps, is that right ? Was there any reason why using only negotiated caps (or croppped caps) combined with gst_buffer_pool_config_set_video_alignment() didn't work ?
(In reply to Nicolas Dufresne (stormer) from comment #7) > So if I understand well, in VAAPI we have allocation caps != negotiated > caps. So using this method, we are forced to have a buffer pool that is > aware of the GstElement in order to receive the negotiated caps, is that > right ? > > Was there any reason why using only negotiated caps (or croppped caps) > combined with gst_buffer_pool_config_set_video_alignment() didn't work ? Most of this debate was carried on Bug 753914 and it is related with Bug 746087. But, no, if I understand correctly you question, the pool doesn't have to be aware of the GstElement, just of the configured allocator.
Created attachment 353611 [details] [review] vaapivideobufferpool: rename video info structures Renamed private GstVideoInfo structure video_info to allocation_vinfo and alloc_info to negotiated_vinfo. The purpose of these renaming is to clarify the origin and purpose of these private variables: video_info (now allocation_vinfo) comes from the bufferpool configuration. It describes the physical video resolution to be allocated by the allocator, which may be different from the negotiated one. alloc_info (now vmeta_vinfo) comes from the negotiated caps in the pipeline. It represents how the frame is going to be mapped using the video meta. In Intel's VA-API backend, the allocation_vinfo resolution is bigger than the negotiated_info.
Created attachment 353612 [details] [review] vaapivideobufferpool: rename local variables Renamed local video info structure names in set_config() vitual method. The purpose of their renaming is to clarify the origin of those structures, whether come from passed caps parameter (new_allocation_vinfo) or from the configured allocator (allocator_vinfo).
Created attachment 353613 [details] [review] vaapivideomemory: rename qdata quarks and ids Also the parameter names were renamed to reflect their origin and purpose.
Created attachment 353614 [details] [review] plugins: distinguish allocation and negotiation caps The vaapi video decoders might have different allocation caps from the negotiation caps, thus the GstVideoMeta shall use the negotiation caps, not the allocation caps. This was done before reusing gst_allocator_get_vaapi_video_info(), storing there the negotiation caps if they differ from the allocation ones, but this strategy felt short when the allocator had to be reset in the vaapi buffer pool, since we need both. This patch adds gst_allocator_set_vaapi_negotiated_video_info() and gst_allocator_get_vaapi_negotiated_video_info() to store the negotiated video info in the allocator, and distinguish it from the allocation video info.
Created attachment 353615 [details] [review] vaapivideobufferpool: refactor set_config() Refactor the set_config() virtual method considering a cleaner approach to allocator instanciation, if it it not set or if it is not valid for the pool.
Created attachment 353616 [details] [review] vaapivideobufferpool: remove allocation_vinfo private attribute There is no need to keep this attribute internally since it is already managed by the allocator.
Attachment 353611 [details] pushed as dce7a6f - vaapivideobufferpool: rename video info structures Attachment 353612 [details] pushed as 45faeb2 - vaapivideobufferpool: rename local variables Attachment 353613 [details] pushed as 36cf510 - vaapivideomemory: rename qdata quarks and ids Attachment 353614 [details] pushed as 7a20692 - plugins: distinguish allocation and negotiation caps Attachment 353615 [details] pushed as 60158c3 - vaapivideobufferpool: refactor set_config() Attachment 353616 [details] pushed as bd02092 - vaapivideobufferpool: remove allocation_vinfo private attribute
*** Bug 781756 has been marked as a duplicate of this bug. ***