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 767474 - vaapidecode: video corruption with vp9 resolution changes
vaapidecode: video corruption with vp9 resolution changes
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.9.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-06-09 22:23 UTC by Scott D Phillips
Modified: 2016-10-31 14:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] gstvaapicontext: control reset_on_resize with option (3.16 KB, patch)
2016-06-09 22:27 UTC, Scott D Phillips
committed Details | Review
[PATCH] decoder: vp9: Update comment about context resets (1.68 KB, patch)
2016-06-15 18:28 UTC, Scott D Phillips
committed Details | Review

Description Scott D Phillips 2016-06-09 22:23:45 UTC
Some vp9 videos with resolution changes are decoded incorrectly because the context is recreated when the size changes. The proper vaapi usage for vp9 is to keep using the same context but create new surfaces for the new resolution.
Comment 1 Scott D Phillips 2016-06-09 22:27:22 UTC
Created attachment 329520 [details] [review]
[PATCH] gstvaapicontext: control reset_on_resize with option
Comment 2 Víctor Manuel Jáquez Leal 2016-06-14 11:36:49 UTC
This context reset because of frame size changes is something written in the VA spec? Or is it something specific for intel driver? Or is it just to "play safe"?

I wonder, can we remove the context reset caused by the frame resize for all the vaapi entries?
Comment 3 Víctor Manuel Jáquez Leal 2016-06-14 12:36:44 UTC
According to this patch 

https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/commit/?id=906a561c26578e17cae77be665934b498a814695

The VA-API spec says if the surfaces change, the context needs to be reset.

I wonder if this is something to be fixed in the driver, rather than work-arounded in gstreamer-vaapi.
Comment 4 sreerenj 2016-06-14 14:34:38 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #3)
> According to this patch 
> 
> https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/commit/
> ?id=906a561c26578e17cae77be665934b498a814695
> 
> The VA-API spec says if the surfaces change, the context needs to be reset.
> 
> I wonder if this is something to be fixed in the driver, rather than
> work-arounded in gstreamer-vaapi.

Right now, all codecs except vp9 requires the context reset if surface size changes . This is how driver implemented, i would say it is a limitation.
At the same time, AFAIK VP9 is the only codec which allows resolution change even with out having a key/I frame before the change, and allowing inter-prediction between the frames which are having different resolutions.
Comment 5 sreerenj 2016-06-14 14:39:02 UTC
As per the patch, it seems even if the resolution changes to higher one (higher than already configured), there is no context change required.

Is that right Scot?
Could you please double check it?
 
If so you might need few other changes too,
My old comments need changes ;) :https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/tree/gst-libs/gst/vaapi/gstvaapidecoder_vp9.c#n599
Comment 6 Scott D Phillips 2016-06-15 18:20:13 UTC
(In reply to sreerenj from comment #5)
> As per the patch, it seems even if the resolution changes to higher one
> (higher than already configured), there is no context change required.
> 
> Is that right Scott?
> Could you please double check it?

Yes, that seems to be the case in my testing. A context reset is not required, even when resolution increases.

It looks to me like the code is correct (we will need new surfaces when size increases) but the comment isn't.
Comment 7 Scott D Phillips 2016-06-15 18:28:52 UTC
Created attachment 329872 [details] [review]
[PATCH] decoder: vp9: Update comment about context resets

Update the comment about context resets.