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 748184 - vaapipostproc 1920x1080
vaapipostproc 1920x1080
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Linux
: Urgent critical
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on:
Blocks: 743569
 
 
Reported: 2015-04-20 14:21 UTC by Thomas Scheuermann
Modified: 2015-07-03 11:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
vaapipostproc: Fix wrong selection of passthrough mode (1.17 KB, patch)
2015-06-29 09:33 UTC, sreerenj
committed Details | Review
vaapipostproc: no format convert on GL tex upload meta (1.46 KB, patch)
2015-07-02 15:50 UTC, Víctor Manuel Jáquez Leal
committed Details | Review
vaapipostroc: GLTextureUploadMeta in sink template (1.80 KB, patch)
2015-07-03 07:37 UTC, Víctor Manuel Jáquez Leal
none Details | Review

Description Thomas Scheuermann 2015-04-20 14:21:51 UTC
I've got a problem with a pipeline using vaapipostproc with output from vaapidecode (decodebin). If the input of the video stream is 1920x1080 and as output is also 1920x1080 wanted, the pipeline fails. Here is an example pipeline, which fails 1920x1080 -> 1920x1080:

gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12,width=1920,height=1080 ! vaapiencode_h264 ! vaapiparse_h264 ! decodebin ! vaapipostproc width=1920 height=1080 ! video/x-raw,format=BGRx,width=1920,height=1080 ! ximagesink

if the size is changed slightly to output 1921x1080 it works 1920x1080 -> 1921x1080:

gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12,width=1920,height=1080 ! vaapiencode_h264 ! vaapiparse_h264 ! decodebin ! vaapipostproc width=1921 height=1080 ! video/x-raw,format=BGRx,width=1921,height=1080 ! ximagesink

Also these pipelines work
1921x1080 -> 1921x1080:

gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12,width=1921,height=1080 ! vaapiencode_h264 ! vaapiparse_h264 ! decodebin ! vaapipostproc width=1921 height=1080 ! video/x-raw,format=BGRx,width=1921,height=1080 ! ximagesink

1280x720 -> 1280x720:
gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12,width=1280,height=720 ! vaapiencode_h264 ! vaapiparse_h264 ! decodebin ! vaapipostproc width=1280 height=720 ! video/x-raw,format=BGRx,width=1280,height=720 ! ximagesink

1280x720 -> 1920x1080
gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12,width=1280,height=720 ! vaapiencode_h264 ! vaapiparse_h264 ! decodebin ! vaapipostproc width=1920 height=1080 ! video/x-raw,format=BGRx,width=1920,height=1080 ! ximagesink

Regards,

Thomas
Comment 1 Thomas Scheuermann 2015-04-21 11:20:02 UTC
It seems that the error is caused only by the horizontal size of 1920. The following pipeline also does not work:
1920x900 -> 1920x900

gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12,width=1920,height=900 ! vaapiencode_h264 ! vaapiparse_h264 ! decodebin ! vaapipostproc width=1920 height=900 ! video/x-raw,format=BGRx,width=1920,height=900 ! ximagesink

Regards,

Thomas
Comment 2 Thomas Scheuermann 2015-04-21 11:47:34 UTC
Multiple horizontal sizes of 240 don't work
240x180 ->240x180 fails:

gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12,width=240,height=180 ! vaapiencode_h264 ! vaapiparse_h264 ! decodebin ! vaapipostproc width=240 height=180 ! video/x-raw,format=BGRx,width=240,height=180 ! ximagesink

I've tested it for 240, 480, 720, 960, 1200, 1440, 1680, 1920, 3840
Comment 3 Víctor Manuel Jáquez Leal 2015-04-21 15:21:21 UTC
Thanks for your report.

Just to be clear, what version of gstreamer are you using? I suppose you are using gstreamer-vaapi master, am I right?
Comment 4 Thomas Scheuermann 2015-04-22 08:10:18 UTC
Yes, I have tested this with the master branch (commit dedbbdd41b9d510f8c1a667182b796d5cbcbbf65)
Comment 5 Víctor Manuel Jáquez Leal 2015-04-22 09:19:41 UTC
(In reply to Thomas Scheuermann from comment #4)
> Yes, I have tested this with the master branch (commit
> dedbbdd41b9d510f8c1a667182b796d5cbcbbf65)

Ok. Thanks. But what version of gstreamer are you using? I mean, 1.2, 1.4, 1.5??

The thing is that gstreamer-vaapi behaves slightly different for each of the gstreamer version, and I would like to replicate your test in the same environment.
Comment 6 Thomas Scheuermann 2015-04-22 09:59:59 UTC
I use gstreamer 1.4.4 in a Debian Jessie environment.
Comment 7 sreerenj 2015-06-29 04:07:16 UTC
Issue is not because of multiples of x... The current code path will fall-back to passthrough mode unless you set the format  property of vaapipostproc(or at least one property of vaapipostproc to a different value than what provided in sinkpad caps) explicitly..
Your pipeline will work if you set "format=bgrx" in vaapipostproc.. Anyway I will prepare a patch to avoid passthrough if negotiated src caps have a different color space than sink pad caps , even though the user didn't set the format property explicitly.
Comment 8 sreerenj 2015-06-29 09:33:37 UTC
Created attachment 306269 [details] [review]
vaapipostproc: Fix wrong selection of passthrough mode
Comment 9 sreerenj 2015-06-29 09:35:10 UTC
(In reply to Thomas Scheuermann from comment #6)
> I use gstreamer 1.4.4 in a Debian Jessie environment.

Could you please have some tests with the attached patch..?
Comment 10 sreerenj 2015-06-29 10:38:20 UTC
Pushed,
commit d14a201699356d1a0fcb140d0700e15df52cc52e
Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Date:   Mon Jun 29 13:35:59 2015 +0300

    vaapipostproc: Fix wrong selection of passthrough mode.
    
    The Current code path is falling back to passthorugh mode if there is no
    vpp property set by the user explictily. But we should not use the
    passthrough mode if the negotiated src pad caps have a differnt color space
    format than sink pad caps (Even though the user didn't set the format property
    explicitly).
    
    https://bugzilla.gnome.org/show_bug.cgi?id=748184
    
    Signed-off-by: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Comment 11 sreerenj 2015-06-29 10:39:19 UTC
Closing this based on my testings...
Please feel free to re-open if issue persists.
Comment 12 Víctor Manuel Jáquez Leal 2015-07-02 14:12:09 UTC
This landed patch breaks clutterautovideosink rendering (which is used by totem!!)
Comment 13 sreerenj 2015-07-02 14:19:24 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #12)
> This landed patch breaks clutterautovideosink rendering (which is used by
> totem!!)

ouch!! may be a separate passthrough-or-not flag is needed....
Sorry, Could you please have a look ??? 
I am on behalf of another blocker #751831....
Comment 14 sreerenj 2015-07-02 14:21:47 UTC
Changing to "critical"!
Comment 15 Víctor Manuel Jáquez Leal 2015-07-02 14:45:01 UTC
reverting commit d14a201 (vaapipostproc: Fix wrong selection of passthrough mode) the cluttetautovideosink works.

I'm looking at it.
Comment 16 Víctor Manuel Jáquez Leal 2015-07-02 15:50:32 UTC
Created attachment 306646 [details] [review]
vaapipostproc: no format convert on GL tex upload meta

When GL texture upload meta is negotiated, vaapipostproc shall not modify the
color format of the buffer.
Comment 17 sreerenj 2015-07-03 06:27:30 UTC
Review of attachment 306646 [details] [review]:

lgtm,
Also i think we need to advertise the GLTextureUploadMeta in sinkcaps too...
Comment 18 sreerenj 2015-07-03 06:28:24 UTC
(In reply to sreerenj from comment #17)
> Review of attachment 306646 [details] [review] [review]:
> 
> lgtm,
> Also i think we need to advertise the GLTextureUploadMeta in sinkcaps too...
:s/sinkcaps/sink template caps
Comment 19 Víctor Manuel Jáquez Leal 2015-07-03 06:35:30 UTC
(In reply to sreerenj from comment #17)
> Review of attachment 306646 [details] [review] [review]:
> 
> lgtm,
> Also i think we need to advertise the GLTextureUploadMeta in sinkcaps too...

In order to enable passthrough mode? Let me try it.
Comment 20 sreerenj 2015-07-03 06:52:47 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #19)
> (In reply to sreerenj from comment #17)
> > Review of attachment 306646 [details] [review] [review] [review]:
> > 
> > lgtm,
> > Also i think we need to advertise the GLTextureUploadMeta in sinkcaps too...
> 
> In order to enable passthrough mode? Let me try it.

vaapipostproc is able is passthrough all GstVideoGLTextureUploadMeta-enabled buffers from vaapideocde for eg..So we should advertise it in sink pad template too...right now sinkpad only exposing video/x-raw(memory:VASurface) and video/x-raw...
Comment 21 Víctor Manuel Jáquez Leal 2015-07-03 07:37:11 UTC
Created attachment 306679 [details] [review]
vaapipostroc: GLTextureUploadMeta in sink template

Advertise GLTextureUploadMeta in sink caps template.
Comment 22 Víctor Manuel Jáquez Leal 2015-07-03 07:43:15 UTC
(In reply to sreerenj from comment #20)
> (In reply to Víctor Manuel Jáquez Leal from comment #19)
> > (In reply to sreerenj from comment #17)
> > > Review of attachment 306646 [details] [review] [review] [review] [review]:
> > > 
> > > lgtm,
> > > Also i think we need to advertise the GLTextureUploadMeta in sinkcaps too...
> > 
> > In order to enable passthrough mode? Let me try it.
> 
> vaapipostproc is able is passthrough all GstVideoGLTextureUploadMeta-enabled
> buffers from vaapideocde for eg..So we should advertise it in sink pad
> template too...right now sinkpad only exposing video/x-raw(memory:VASurface)
> and video/x-raw...

I'm not sure of this since vaapidecode will always choose video/x-raw(memory:VASurface) when negotiating with vaapipostroc (it is the first to try). Nonetheless, if the videosink request a buffer pool with GstVideoGLTextureUploadMeta, the vaapidecode will add it, hence the vaapipostroc can negotiate it.

I mean, I'm still not convinced about this patch: Imagine a non-vaapi video decoder that negotiate GstVideoGLTextureUploadMeta, the vaapipostroc, in the best case, will be passthrough since it cannot do anything else.
Comment 23 sreerenj 2015-07-03 08:51:44 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #22)
> (In reply to sreerenj from comment #20)
> > (In reply to Víctor Manuel Jáquez Leal from comment #19)
> > > (In reply to sreerenj from comment #17)
> > > > Review of attachment 306646 [details] [review] [review] [review] [review] [review]:
> > > > 
> > > > lgtm,
> > > > Also i think we need to advertise the GLTextureUploadMeta in sinkcaps too...
> > > 
> > > In order to enable passthrough mode? Let me try it.
> > 
> > vaapipostproc is able is passthrough all GstVideoGLTextureUploadMeta-enabled
> > buffers from vaapideocde for eg..So we should advertise it in sink pad
> > template too...right now sinkpad only exposing video/x-raw(memory:VASurface)
> > and video/x-raw...
> 
> I'm not sure of this since vaapidecode will always choose
> video/x-raw(memory:VASurface) when negotiating with vaapipostroc (it is the
> first to try). Nonetheless, if the videosink request a buffer pool with
> GstVideoGLTextureUploadMeta, the vaapidecode will add it, hence the
> vaapipostroc can negotiate it.
> 

How about a capsfilter(to enable GstVideoGlTextureUploadMeta) in the middle of vaapidecode and vaapipostproc? will it pass the negotiation phase if there is no sinkpad template caps??

> I mean, I'm still not convinced about this patch: Imagine a non-vaapi video
> decoder that negotiate GstVideoGLTextureUploadMeta, the vaapipostroc, in the
> best case, will be passthrough since it cannot do anything else.
Comment 24 Víctor Manuel Jáquez Leal 2015-07-03 11:33:36 UTC
Attachment 306646 [details] pushed as 9984678 - vaapipostproc: no format convert on GL tex upload meta
Comment 25 Víctor Manuel Jáquez Leal 2015-07-03 11:35:20 UTC
I'll open a new bug regarding the caps template