GNOME Bugzilla – Bug 790149
vaapipostproc: Does not maintain aspect ratio when scaling
Last modified: 2018-11-03 15:52:08 UTC
When vaapipostproc is being used (notably through vaapidecodebin), the vaapipostproc may endup scaling the video. The problem is that the scaling does not maintain the aspect ratio, which is totally unexpected default behaviour. On can illustrace this with a non scaling display sink. gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 ! vaapipostproc ! ximagesink In comparison with: gst-launch-1.0 videotestsrc ! video/x-raw,width=320,height=240 ! videoscale ! ximagesink
I don't deny there might be a problem handling the display-aspect-ratio with vaapipostproc, but I can't reproduce it, in my setup, with those examples.
I didn't check yet, but Wich layer of abstraction take care of that ? If it's not reliable it might be something inside the driver, with different bug depending on the driver version ?
oops! I can reproduce it!
Just to keep the ball rolling, can vaapipostproc add black bars, top/bottom or left/right depending ? I'm asking because scalers have two options: a) they maintain aspect ratio by negotiating output caps that do respect it b) scale to the negotiated output size, and add black borders as needed
Any answer to my question ?
(In reply to Nicolas Dufresne (stormer) from comment #4) > Just to keep the ball rolling, can vaapipostproc add black bars, top/bottom > or left/right depending ? > > I'm asking because scalers have two options: > > a) they maintain aspect ratio by negotiating output caps that do respect it > b) scale to the negotiated output size, and add black borders as needed Honestly, I don't know for sure, I've never tried. But, reading the gstvaapifilter code, and the VA-API, it seems possible to do option b): https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/tree/gst-libs/gst/vaapi/gstvaapifilter.c#n1444 Since it is possible to set a destine rectangle with a specified background color.
Created attachment 367036 [details] [review] vaapipostproc: if no p-a-r in out caps define a range Instead of copying the pixel-aspect-ratio from the sink caps, define an open range for the src caps pixel-aspect-ratio. Later it will be defined.
Created attachment 367037 [details] [review] vaapipostproc: remove spurious code This assignation is dead code, since gst_video_info_from_caps() set to 1 by default.
Created attachment 367038 [details] [review] vaapipostproc: keep the display's pixel aspect ratio Before vaapipostproc will resize the video frame to the window size, but that broke the display's pixel aspect ratio form the original video. This patch will honor the original pixel-aspect-ratio by setting a target rectangle within the output surface, that means black borders.
(In reply to Víctor Manuel Jáquez Leal from comment #9) > Created attachment 367038 [details] [review] [review] > vaapipostproc: keep the display's pixel aspect ratio > > Before vaapipostproc will resize the video frame to the window size, > but that broke the display's pixel aspect ratio form the original > video. > > This patch will honor the original pixel-aspect-ratio by setting a > target rectangle within the output surface, that means black borders. This patch works regular with intel-vaapi-driver, it shows some artifacts and the image is not centered. In the case of mesa amd driver, it is worse since it crashes. Dunno, perhaps it is a code path no quite tested by drivers. Still, it seems the correct way to do it.
Attachment 367036 [details] pushed as ea9c52e - vaapipostproc: if no p-a-r in out caps define a range Attachment 367037 [details] pushed as da77fd5 - vaapipostproc: remove spurious code
Review of attachment 367038 [details] [review]: I think we should get that in before 1.14.
Ok, after testing more myself, I agree with you, we'll need to debug the driver, I had a complete system hang and had to reboot: vainfo: VA-API version: 0.40 (libva ) vainfo: Driver version: Intel i965 driver for Intel(R) Ivybridge Mobile - 1.8.3
Review of attachment 367038 [details] [review]: Again, I don't see anything ovbiously wrong in the this code though ...
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/issues/74.