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 797277 - create surface fail when reset context.
create surface fail when reset context.
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-10-11 08:47 UTC by Fei
Modified: 2018-10-15 15:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch for this issue. (1.14 KB, patch)
2018-10-11 08:52 UTC, Fei
none Details | Review
patch for this issue. (1.34 KB, patch)
2018-10-15 14:26 UTC, Fei
committed Details | Review

Description Fei 2018-10-11 08:47:56 UTC
In gst_vaapi_context_reset, context will be destroyed if config need to reset. Make sure context be created again before use it to create surface.
Comment 1 Fei 2018-10-11 08:52:59 UTC
Created attachment 373895 [details] [review]
patch for this issue.
Comment 2 Víctor Manuel Jáquez Leal 2018-10-11 14:12:43 UTC
@Fei

Do you have a test that shows the problem?

What's the problem exactly?
Comment 3 Fei 2018-10-12 01:45:14 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #2)
> @Fei
> 
> Do you have a test that shows the problem?
> 
> What's the problem exactly?

Hi Victor, due to the licence we can't public the stream, and I put it into our public google drive(gstreamervaapi@gmail.com) gstreamer_bug_stream folder. If you forget the passwd, please ping me through fei.w.wang@intel.com. :)

The stream have 2 different resolution, when the resolution change, the context and surfaces will be destroyed and re-created. This patch makes sure context is created and available before use it to create surface. Otherwise the pipeline will be stuck with some error. I believe any streams with 2 or more resolutions can be reproduce this issue.

error log:
root@T:/media/# gst-launch-1.0 filesrc location=test.mpeg2 ! mpegvideoparse ! vaapimpeg2dec ! fakesink
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
Got context from element 'vaapidecode_mpeg2-0': gst.gl.GLDisplay=context, gst.gl.GLDisplay=(GstGLDisplay)"\(GstGLDisplayX11\)\ gldisplayx11-0";
Got context from element 'vaapidecode_mpeg2-0': gst.vaapi.Display=context, gst.vaapi.Display=(GstVaapiDisplay)"\(GstVaapiDisplayDRM\)\ vaapidisplaydrm1";
Redistribute latency...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
Redistribute latency...
0:00:00.228859761 24841      0x12204f0 ERROR                  vaapi gstvaapidecoder_mpeg2.c:1509:gst_vaapi_decoder_mpeg2_start_frame: failed to reset context
0:00:00.228904080 24841      0x12204f0 ERROR            vaapidecode gstvaapidecode.c:755:gst_vaapidecode_handle_frame: decode error -1
Comment 4 Fei 2018-10-15 13:15:49 UTC
@Victor,any other comments on this patch?
Comment 5 Víctor Manuel Jáquez Leal 2018-10-15 13:45:45 UTC
Just tested it. I already had the sample. And it is a regression. If you test with release 1.14 the works.

I would like to know on which commit this regression appeared.
Comment 6 Víctor Manuel Jáquez Leal 2018-10-15 13:54:20 UTC
It seems the commit that pop up the error is 

commit 82872f42344506c4f5e31c0a485a68a5f40f98cf (HEAD, refs/bisect/bad)
Author: Wangfei <fei.w.wang@intel.com>
Date:   Mon Oct 1 09:26:05 2018 +0800

    libs: context: query surface format before context to create surface.
    
    Before using context to create surface, the supported surface format
    should be checked first.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797222
Comment 7 Víctor Manuel Jáquez Leal 2018-10-15 14:04:39 UTC
This is logical since with commit 82872f4 now it check the formats available in the VAConfig (part of the GstVaapiContext), thus now we need to create first the context and then the surfaces.

@Fei, please add all this information in the commit log:

"""
In gst_vaapi_context_reset(), if the context has to be destroyed, make sure to create it first before allocating its associated surfaces.

This patch fixes a regression introduced in commit 82872f4 because the formats available in the current context now are ensured before creating the context's surfaces.
"
Comment 8 Fei 2018-10-15 14:26:25 UTC
Created attachment 373928 [details] [review]
patch for this issue.
Comment 9 Fei 2018-10-15 14:26:48 UTC
(In reply to Víctor Manuel Jáquez Leal from comment #7)
> This is logical since with commit 82872f4 now it check the formats available
> in the VAConfig (part of the GstVaapiContext), thus now we need to create
> first the context and then the surfaces.
> 
> @Fei, please add all this information in the commit log:
> 
> """
> In gst_vaapi_context_reset(), if the context has to be destroyed, make sure
> to create it first before allocating its associated surfaces.
> 
> This patch fixes a regression introduced in commit 82872f4 because the
> formats available in the current context now are ensured before creating the
> context's surfaces.
> "

Thanks Victor. Added.
Comment 10 Víctor Manuel Jáquez Leal 2018-10-15 15:00:47 UTC
attachment 373928 [details] [review] pushed as commit ecdfc623 - libs: context: create context first before using it to create surface.