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 785085 - multiple vaapi encoding pipelines on radeonsi segfault
multiple vaapi encoding pipelines on radeonsi segfault
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-07-18 20:20 UTC by Tomas Rataj
Modified: 2017-08-14 13:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
simple test app (985 bytes, text/x-c++src)
2017-07-18 20:20 UTC, Tomas Rataj
  Details
thread backtrace (50.31 KB, text/plain)
2017-07-18 20:27 UTC, Tomas Rataj
  Details
dmesg after 3 crashes (56.48 KB, text/plain)
2017-07-18 21:20 UTC, Tomas Rataj
  Details
GST_DEBUG=5 (563.54 KB, application/zip)
2017-07-18 21:23 UTC, Tomas Rataj
  Details
glxinfo output (20.82 KB, text/plain)
2017-07-18 23:05 UTC, Tomas Rataj
  Details
vagrind (8.97 KB, application/zip)
2017-07-20 12:19 UTC, Tomas Rataj
  Details
st/va: change frame_idx from array to hash table (4.97 KB, patch)
2017-07-21 09:40 UTC, Julien Isorce
reviewed Details | Review
vaapi append_formats change pointers to indexes (2.11 KB, patch)
2017-07-27 10:54 UTC, Tomas Rataj
committed Details | Review

Description Tomas Rataj 2017-07-18 20:20:49 UTC
Created attachment 355885 [details]
simple test app

Hello,

I am writing and application which creates up to 12 encoding pipelines using vaapih264enc. It is working fine on Intel with i965 driver, but it is segfaulting on AMD using mesa gallium radeonsi driver. 

I have attached simple testcase application which simply start multiple pipelines using gst_parse_launch:

pipelines[i] = gst_parse_launch("videotestsrc is-live=true ! video/x-raw,width=1280,height=1024,framerate=20/1 ! vaapih264enc ! testsink", &error);
		
I am able to run up to 6 pipelines with very unstable operation. When used with single pipeline per process I am able to run up to 16 pipelines until I get:

[ 6791.275650] [drm:radeon_vce_cs_parse [radeon]] *ERROR* No more free VCE handles!
[ 6791.277803] [drm:radeon_cs_ioctl [radeon]] *ERROR* Invalid command stream !

When more then 6 pipelines are run as a single process it ends with segfault:

Thread 48 "videotestsrc7:s" received signal SIGSEGV, Segmentation fault.

Thread 140736100189952 (LWP 4962)

  • #0 vlVaEndPicture
    at picture.c line 596
  • #1 vaEndPicture
  • #2 gst_vaapi_enc_picture_encode
    at gstvaapiencoder_objects.c line 581
  • #3 gst_vaapi_encoder_h264_encode
    at gstvaapiencoder_h264.c line 2581
  • #4 gst_vaapi_encoder_put_frame
    at gstvaapiencoder.c line 407
  • #5 gst_vaapiencode_handle_frame
    at gstvaapiencode.c line 643
  • #6 gst_video_encoder_chain
    at gstvideoencoder.c line 1438
  • #7 gst_pad_chain_data_unchecked
    at gstpad.c line 4205
  • #8 gst_pad_push_data
    at gstpad.c line 4457
  • #9 gst_pad_push
    at gstpad.c line 4576
  • #10 gst_base_transform_chain
    at gstbasetransform.c line 2312
  • #11 gst_pad_chain_data_unchecked
    at gstpad.c line 4205
  • #12 gst_pad_push_data
    at gstpad.c line 4457
  • #13 gst_pad_push
    at gstpad.c line 4576
  • #14 gst_base_src_loop
    at gstbasesrc.c line 2913
  • #15 gst_task_func
    at gsttask.c line 332
  • #16 g_thread_pool_thread_proxy
    at ././glib/gthreadpool.c line 307
  • #17 g_thread_proxy
    at ././glib/gthread.c line 784
  • #18 start_thread
    at pthread_create.c line 333
  • #19 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 97

I am using latest mesa from git master.

libva info: VA-API version 0.40.0
libva info: va_getDriverName() returns 0
libva info: User requested driver 'radeonsi'
libva info: Trying to open /usr/lib/x86_64-linux-gnu/dri/radeonsi_drv_video.so
libva info: Found init function __vaDriverInit_0_40
libva info: va_openDriver() returns 0
vainfo: VA-API version: 0.40 (libva )
vainfo: Driver version: mesa gallium vaapi
vainfo: Supported profile and entrypoints
      VAProfileMPEG2Simple            :	VAEntrypointVLD
      VAProfileMPEG2Main              :	VAEntrypointVLD
      VAProfileVC1Simple              :	VAEntrypointVLD
      VAProfileVC1Main                :	VAEntrypointVLD
      VAProfileVC1Advanced            :	VAEntrypointVLD
      VAProfileH264ConstrainedBaseline:	VAEntrypointVLD
      VAProfileH264ConstrainedBaseline:	VAEntrypointEncSlice
      VAProfileH264Main               :	VAEntrypointVLD
      VAProfileH264Main               :	VAEntrypointEncSlice
      VAProfileH264High               :	VAEntrypointVLD
      VAProfileH264High               :	VAEntrypointEncSlice
      VAProfileNone                   :	VAEntrypointVideoProc

Thanks. Tomas
Comment 1 Tomas Rataj 2017-07-18 20:27:09 UTC
Created attachment 355886 [details]
thread backtrace
Comment 2 Tomas Rataj 2017-07-18 21:20:12 UTC
Created attachment 355890 [details]
dmesg after 3 crashes
Comment 3 Tomas Rataj 2017-07-18 21:23:12 UTC
Created attachment 355892 [details]
GST_DEBUG=5
Comment 4 Julien Isorce 2017-07-18 22:31:11 UTC
What mesa version are you using ? I wonder what line picture.c::596 is exactly.

Also I can see there is no nullity check of 'surf' and 'coded_buf' here https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/va/picture.c?id=e58a1e8f68b3b740d915468012573a4d7befb875#n588
Comment 5 Tomas Rataj 2017-07-18 23:05:24 UTC
Created attachment 355893 [details]
glxinfo output

If I install it correctly it should be latest (few days old) mesa from master.
Comment 6 Tomas Rataj 2017-07-20 12:19:07 UTC
Created attachment 356028 [details]
vagrind

Four consequent runs under valgrind. Hopefully it will shed some light on it. Thanks.
Comment 7 Julien Isorce 2017-07-20 13:17:15 UTC
Also can you try adding ",format=RGBA ! vaapipostproc" like this:

videotestsrc is-live=true ! video/x-raw,width=1280,height=1024,framerate=20/1,format=RGBA ! vaapipostproc ! vaapih264enc ! testsink
Also try RGBx, BGRx and BGRA.

Also can you try to comment the block (while keeping the original pipeline, so without adding the format/vaapipostproc):

  /* enable direct upload if upstream requests raw video */
  if (gst_caps_is_video_raw (caps)) {
    usage_flag = GST_VAAPI_IMAGE_USAGE_FLAG_DIRECT_UPLOAD;
    GST_INFO_OBJECT (plugin, "enabling direct upload in sink allocator");
  }

here https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/tree/gst/vaapi/gstvaapipluginbase.c#n519

Also can you check if coded_buf is null here too https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/va/picture.c#n424 . Btw, have you checked coded_buf and surf nullity as per comment #4 ?

And can you compile mesa with --enabl-debug flag.
Comment 8 Tomas Rataj 2017-07-20 20:58:16 UTC
I dig a little deeper and find out that the problem which is causing a segfault is here:

https://cgit.freedesktop.org/mesa/mesa/tree/src/gallium/state_trackers/va/picture.c#n430

The h264->CurrPic.picture_id is sometimes higher than 31. When I add a check for that it is working fine. I can see faulty picture_id only when the pipeline is starting.

Any idea what could be causing it? Thanks.
Comment 9 deathsimple@vodafone.de 2017-07-20 21:14:42 UTC
Thanks for tracking this down.

The problem is that Mesa thinks that the picture_id is a frame number which is in the range 0-31.

But the gst vaapi plugin is using it's surface handles as identifier and that can easily be larger than 31.

We need to change the array in mesa into a hash table to fix this I think.
Comment 10 Julien Isorce 2017-07-21 09:40:48 UTC
Created attachment 356100 [details] [review]
st/va: change frame_idx from array to hash table

Thx Christian for the suggestion. Hi Tomas, please have a go with the attached patch.
Comment 11 Tomas Rataj 2017-07-22 14:58:52 UTC
Thank you guys. Problem with picture_id is gone but there might be still something wrong. With the original pipeline I am getting invalid read here:

https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/tree/gst-libs/gst/vaapi/gstvaapidisplay.c#n272

Which might be related to the fact that the hw is supporting NV12 only.

0:00:13.324039431   802     0x189023e0 ERROR       vaapivideomemory gstvaapivideomemory.c:736:gst_video_info_update_from_surface: Cannot create a VA derived image from surface 0x19603570
==802== Thread 10 videotestsrc0:sr:
==802== Invalid read of size 4
==802==    at 0xADACD31: append_formats (gstvaapidisplay.c:272)
==802==    by 0xADACEFF: ensure_image_formats (gstvaapidisplay.c:721)
==802==    by 0xADAE4EB: gst_vaapi_display_has_image_format (gstvaapidisplay.c:1573)
==802==    by 0xADB23CF: _gst_vaapi_image_create (gstvaapiimage.c:138)
==802==    by 0xADB2C48: gst_vaapi_image_create (gstvaapiimage.c:169)
==802==    by 0xADB2C48: gst_vaapi_image_new (gstvaapiimage.c:257)
==802==    by 0xAD9443D: new_image (gstvaapivideomemory.c:118)
==802==    by 0xAD9443D: allocator_configure_image_info (gstvaapivideomemory.c:829)
==802==    by 0xAD9443D: allocator_params_init (gstvaapivideomemory.c:871)
==802==    by 0xAD9443D: gst_vaapi_video_allocator_new (gstvaapivideomemory.c:910)
==802==    by 0xAD84860: ensure_sinkpad_allocator (gstvaapipluginbase.c:524)
==802==    by 0xAD84860: ensure_sinkpad_buffer_pool (gstvaapipluginbase.c:783)
==802==    by 0xAD84AC5: gst_vaapi_plugin_base_set_caps (gstvaapipluginbase.c:834)
==802==    by 0xAD96410: gst_vaapiencode_set_format (gstvaapiencode.c:566)
==802==    by 0x8293E15: gst_video_encoder_setcaps (gstvideoencoder.c:594)
==802==    by 0x8293E15: gst_video_encoder_sink_event_default (gstvideoencoder.c:946)
==802==    by 0xAD952D7: gst_vaapiencode_sink_event (gstvaapiencode.c:781)
==802==    by 0x5559C76: gst_pad_send_event_unchecked (gstpad.c:5608)
==802==  Address 0x196068f4 is 20 bytes inside a block of size 32 free'd
==802==    at 0x4C2DDCF: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==802==    by 0x5AA60E7: g_realloc (gmem.c:159)
==802==    by 0x5A73908: g_array_maybe_expand (garray.c:788)
==802==    by 0x5A73C9C: g_array_append_vals (garray.c:421)
==802==    by 0xADACC8D: append_format (gstvaapidisplay.c:229)
==802==    by 0xADACC8D: append_formats (gstvaapidisplay.c:252)
==802==    by 0xADACEFF: ensure_image_formats (gstvaapidisplay.c:721)
==802==    by 0xADAE4EB: gst_vaapi_display_has_image_format (gstvaapidisplay.c:1573)
==802==    by 0xADB23CF: _gst_vaapi_image_create (gstvaapiimage.c:138)
==802==    by 0xADB2C48: gst_vaapi_image_create (gstvaapiimage.c:169)
==802==    by 0xADB2C48: gst_vaapi_image_new (gstvaapiimage.c:257)
==802==    by 0xAD9443D: new_image (gstvaapivideomemory.c:118)
==802==    by 0xAD9443D: allocator_configure_image_info (gstvaapivideomemory.c:829)
==802==    by 0xAD9443D: allocator_params_init (gstvaapivideomemory.c:871)
==802==    by 0xAD9443D: gst_vaapi_video_allocator_new (gstvaapivideomemory.c:910)
==802==    by 0xAD84860: ensure_sinkpad_allocator (gstvaapipluginbase.c:524)
==802==    by 0xAD84860: ensure_sinkpad_buffer_pool (gstvaapipluginbase.c:783)
==802==    by 0xAD84AC5: gst_vaapi_plugin_base_set_caps (gstvaapipluginbase.c:834)
==802==  Block was alloc'd at
==802==    at 0x4C2DDCF: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==802==    by 0x5AA60E7: g_realloc (gmem.c:159)
==802==    by 0x5A73908: g_array_maybe_expand (garray.c:788)
==802==    by 0x5A73C9C: g_array_append_vals (garray.c:421)
==802==    by 0xADACC8D: append_format (gstvaapidisplay.c:229)
==802==    by 0xADACC8D: append_formats (gstvaapidisplay.c:252)
==802==    by 0xADACEFF: ensure_image_formats (gstvaapidisplay.c:721)
==802==    by 0xADAE4EB: gst_vaapi_display_has_image_format (gstvaapidisplay.c:1573)
==802==    by 0xADB23CF: _gst_vaapi_image_create (gstvaapiimage.c:138)
==802==    by 0xADB2C48: gst_vaapi_image_create (gstvaapiimage.c:169)
==802==    by 0xADB2C48: gst_vaapi_image_new (gstvaapiimage.c:257)
==802==    by 0xAD9443D: new_image (gstvaapivideomemory.c:118)
==802==    by 0xAD9443D: allocator_configure_image_info (gstvaapivideomemory.c:829)
==802==    by 0xAD9443D: allocator_params_init (gstvaapivideomemory.c:871)
==802==    by 0xAD9443D: gst_vaapi_video_allocator_new (gstvaapivideomemory.c:910)
==802==    by 0xAD84860: ensure_sinkpad_allocator (gstvaapipluginbase.c:524)
==802==    by 0xAD84860: ensure_sinkpad_buffer_pool (gstvaapipluginbase.c:783)
==802==    by 0xAD84AC5: gst_vaapi_plugin_base_set_caps (gstvaapipluginbase.c:834)


Another weird thing is that running the app under valgrind always ends with SIGBUS.

  ==2999== Process terminating with default action of signal 7 (SIGBUS)
==2999==  Non-existent physical address at address 0x333EE000
==2999==    at 0x4C30143: memcpy@GLIBC_2.2.5 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2999==    by 0xF07D0E9: util_copy_rect (u_surface.c:105)
==2999==    by 0xF07D267: util_copy_box (u_surface.c:131)
==2999==    by 0xF081FFE: u_default_texture_subdata (u_transfer.c:67)
==2999==    by 0xF004321: vlVaPutImage (image.c:557)
==2999==    by 0xADB65FB: gst_vaapi_surface_put_image (gstvaapisurface.c:741)
==2999==    by 0xAD92712: ensure_surface_is_current.part.2 (gstvaapivideomemory.c:228)
==2999==    by 0xAD8FED4: gst_vaapi_video_meta_get_surface_proxy (gstvaapivideometa.c:559)
==2999==    by 0xAD95ECA: gst_vaapiencode_handle_frame (gstvaapiencode.c:634)
==2999==    by 0x8294C1E: gst_video_encoder_chain (gstvideoencoder.c:1438)
==2999==    by 0x555AF91: gst_pad_chain_data_unchecked (gstpad.c:4205)
==2999==    by 0x555AF91: gst_pad_push_data (gstpad.c:4457)
==2999==    by 0x5563421: gst_pad_push (gstpad.c:4576)

With following messages in dmesg:

[  359.907512] radeon 0000:00:01.0: ring 0 stalled for more than 10256msec
[  359.907578] radeon 0000:00:01.0: GPU lockup (current fence id 0x0000000000000ea7 last fence id 0x0000000000000eaa on ring 0)
[  360.414134] [TTM] Buffer eviction failed

[  638.166189] radeon 0000:00:01.0: Syncing to a disabled ring!
[  638.166218] radeon 0000:00:01.0: failed to sync rings (-22)
[  639.562933] radeon 0000:00:01.0: couldn't schedule ib
[  639.562994] [drm:radeon_cs_ioctl [radeon]] *ERROR* Failed to schedule IB !
[  639.563312] radeon 0000:00:01.0: couldn't schedule ib

I have to say that I was not able to crash it without valgrid. Is it something to be worry about? Thanks
Comment 12 Julien Isorce 2017-07-25 14:55:54 UTC
(In reply to Tomas Rataj from comment #11)
> Thank you guys. Problem with picture_id is gone 
Thx for confirming. I submited the patch for review: https://patchwork.freedesktop.org/patch/168921/

> but there might be still
> something wrong. With the original pipeline I am getting invalid read here:
> 
> https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/tree/gst-libs/gst/
> vaapi/gstvaapidisplay.c#n272
> 

Could be indeed, also the "*fipp = &g_array_index(" looks dangerous. Can you add some traces to figure out what is wrong exactly ?
Do you also get it if you force RGBA with "RGBA ! vaapipostproc" (see previous msgs) ?

About the gpu lockup it does not look good :) Can you attach the full dmesg when this happens and also the GST_DEBUG=*vaapi*:6 traces. I wonder if mesa:"vlVaTerminate" is called in between. Since you mentioned that it happens when terminating the app. And maybe also are more traces around mesa::vlVaPutImage (image.c:557).
Comment 13 Tomas Rataj 2017-07-25 22:05:01 UTC
Lets start with the invalid read. I am not very familiar with g_array but it seems that it is reallocating the data when new elements are added. Than the access using pointers YV12_fip and I420_fip is wrong. It can be done using array indexes for example. Please take a look on it.

When I run pipeline with vaapipostproc as you suggested:

videotestsrc is-live=true ! video/x-raw,width=1280,height=1024,framerate=20/1,format=BGRA ! vaapipostproc ! vaapih264enc ! testsink

I am getting following error and SEGV right after that:

0x15ef37c0 ERROR          vaapipostproc gstvaapipostproc.c:810:gst_vaapipostproc_process_vpp:<vaapipostproc0> failed to apply VPP filters (error 2)
==925== Thread 10 videotestsrc2:sr:
==925== Invalid write of size 8
==925==    at 0x41E8199: ??? (in /run/user/1000/orcexec.teOKWB (deleted))
==925==    by 0x82A44DA: video_orc_pack_BGRA (tmp-orc.c:4653)
==925==    by 0xA3689E5: convert_hline_generic (videotestsrc.c:1289)
==925==    by 0xA368697: videotestsrc_convert_tmpline (videotestsrc.c:273)
==925==    by 0xA368FAD: gst_video_test_src_smpte (videotestsrc.c:351)
==925==    by 0xA3663E3: gst_video_test_src_fill (gstvideotestsrc.c:1152)
==925==    by 0x6E055DE: gst_base_src_default_create (gstbasesrc.c:1474)
==925==    by 0x6E0896E: gst_base_src_get_range (gstbasesrc.c:2453)
==925==    by 0x6E0A2EE: gst_base_src_loop (gstbasesrc.c:2729)
==925==    by 0x558E240: gst_task_func (gsttask.c:332)
==925==    by 0x5AC8EED: g_thread_pool_thread_proxy (gthreadpool.c:307)
==925==    by 0x5AC84F4: g_thread_proxy (gthread.c:784)
==925==  Address 0x4035000 is not stack'd, malloc'd or (recently) free'd
==925== 
==925== 
==925== Process terminating with default action of signal 11 (SIGSEGV)
==925==  Access not within mapped region at address 0x4035000
==925==    at 0x41E8199: ??? (in /run/user/1000/orcexec.teOKWB (deleted))
==925==    by 0x82A44DA: video_orc_pack_BGRA (tmp-orc.c:4653)
==925==    by 0xA3689E5: convert_hline_generic (videotestsrc.c:1289)
==925==    by 0xA368697: videotestsrc_convert_tmpline (videotestsrc.c:273)
==925==    by 0xA368FAD: gst_video_test_src_smpte (videotestsrc.c:351)
==925==    by 0xA3663E3: gst_video_test_src_fill (gstvideotestsrc.c:1152)
==925==    by 0x6E055DE: gst_base_src_default_create (gstbasesrc.c:1474)
==925==    by 0x6E0896E: gst_base_src_get_range (gstbasesrc.c:2453)
==925==    by 0x6E0A2EE: gst_base_src_loop (gstbasesrc.c:2729)
==925==    by 0x558E240: gst_task_func (gsttask.c:332)
==925==    by 0x5AC8EED: g_thread_pool_thread_proxy (gthreadpool.c:307)
==925==    by 0x5AC84F4: g_thread_proxy (gthread.c:784)

When I comment out direct upload flag as you suggested in comment#7 it starts working and it also stops printing this error that was always there:

0x9c76500 ERROR       vaapivideomemory gstvaapivideomemory.c:736:gst_video_info_update_from_surface: Cannot create a VA derived image from surface 0x9ee5b60

But it is still quite unstable under heavy load (>10 pipelines) with random crashes on different places. I will put here some traces after I will analyze it a bit more. What about the direct upload flag? Should I leave it commented out? Thanks a lot.
Comment 14 Julien Isorce 2017-07-26 22:38:51 UTC
(In reply to Tomas Rataj from comment #13)
> Lets start with the invalid read. I am not very familiar with g_array but it
> seems that it is reallocating the data when new elements are added. Than the
> access using pointers YV12_fip and I420_fip is wrong. It can be done using
> array indexes for example. Please take a look on it.

According to the doc https://developer.gnome.org/glib/stable/glib-Arrays.html#g-array-index it looks ok so I am not sure what could be wrong.

I have not got the chance to try but since you are at it could you please try to see what happen if you change the for loop end condition from i < n to i < (n > 0) ? 1 : 0 ? Also can you check if ->len is > 0 ?
And what value has n when returning from vaQueryImageFormats in that same file ? Thx!

> 
> When I run pipeline with vaapipostproc as you suggested:
> 
> videotestsrc is-live=true !
> video/x-raw,width=1280,height=1024,framerate=20/1,format=BGRA !
> vaapipostproc ! vaapih264enc ! testsink
> 
> I am getting following error and SEGV right after that:
> 
> 0x15ef37c0 ERROR          vaapipostproc
> gstvaapipostproc.c:810:gst_vaapipostproc_process_vpp:<vaapipostproc0> failed
> to apply VPP filters (error 2)
> ==925== Thread 10 videotestsrc2:sr:
> ==925== Invalid write of size 8
> ==925==    at 0x41E8199: ??? (in /run/user/1000/orcexec.teOKWB (deleted))
> ==925==    by 0x82A44DA: video_orc_pack_BGRA (tmp-orc.c:4653)
> ==925==    by 0xA3689E5: convert_hline_generic (videotestsrc.c:1289)
> ==925==    by 0xA368697: videotestsrc_convert_tmpline (videotestsrc.c:273)
> ==925==    by 0xA368FAD: gst_video_test_src_smpte (videotestsrc.c:351)
> ==925==    by 0xA3663E3: gst_video_test_src_fill (gstvideotestsrc.c:1152)
> ==925==    by 0x6E055DE: gst_base_src_default_create (gstbasesrc.c:1474)
> ==925==    by 0x6E0896E: gst_base_src_get_range (gstbasesrc.c:2453)
> ==925==    by 0x6E0A2EE: gst_base_src_loop (gstbasesrc.c:2729)
> ==925==    by 0x558E240: gst_task_func (gsttask.c:332)
> ==925==    by 0x5AC8EED: g_thread_pool_thread_proxy (gthreadpool.c:307)
> ==925==    by 0x5AC84F4: g_thread_proxy (gthread.c:784)
> ==925==  Address 0x4035000 is not stack'd, malloc'd or (recently) free'd

This other issue looks like https://bugzilla.gnome.org/show_bug.cgi?id=779642

> When I comment out direct upload flag as you suggested in comment#7 it
> starts working and it also stops printing this error that was always there:
> 
> 0x9c76500 ERROR       vaapivideomemory
> gstvaapivideomemory.c:736:gst_video_info_update_from_surface: Cannot create
> a VA derived image from surface 0x9ee5b60

Could this is because the tiled format does not get mapped as linear as it is on intel/vaapi. Maybe we can force linear when using mesa/vaapi. I think for the download case there is an env var something like that. So yes you can leave it as linear.

> 
> But it is still quite unstable under heavy load (>10 pipelines) with random
> crashes on different places. I will put here some traces after I will
> analyze it a bit more. What about the direct upload flag? Should I leave it
> commented out? Thanks a lot.

I think it has not been tested with many decoder and encoder instances in the same process so we will have to solve all issues one by one I am afraid.
Any chance that you can compile mesa and gstreamer-vaapi with less optimizations, like -O0.
Comment 15 Tomas Rataj 2017-07-27 06:49:31 UTC
(In reply to Julien Isorce from comment #14)
> (In reply to Tomas Rataj from comment #13)
> > Lets start with the invalid read. I am not very familiar with g_array but it
> > seems that it is reallocating the data when new elements are added. Than the
> > access using pointers YV12_fip and I420_fip is wrong. It can be done using
> > array indexes for example. Please take a look on it.
> 
> According to the doc
> https://developer.gnome.org/glib/stable/glib-Arrays.html#g-array-index it
> looks ok so I am not sure what could be wrong.
> 
> I have not got the chance to try but since you are at it could you please
> try to see what happen if you change the for loop end condition from i < n
> to i < (n > 0) ? 1 : 0 ? Also can you check if ->len is > 0 ?
> And what value has n when returning from vaQueryImageFormats in that same
> file ? Thx!
>
I will describe it a bit more. The problem is that we are saving the location of the element inside the for loop here:

https://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/tree/gst-libs/gst/vaapi/gstvaapidisplay.c#n240

But when we are adding more elements to the array the elements could be reallocated to the different place and then the original location is not valid. Than accessing the flags on line 246 gives an invalid read. Solution could be for example use of index of the element instead of pointer (or simply storing the YV12 and I420 flags directly):

static void
append_formats (GArray * formats, const VAImageFormat * va_formats,
    guint * flags, guint n)
{
  GstVideoFormat format;
  int YV12_idx = -1;
  int I420_idx = -1;
  const GstVaapiFormatInfo *fip;
  guint i;

  for (i = 0; i < n; i++) {
    const VAImageFormat *const va_format = &va_formats[i];

    format = gst_vaapi_video_format_from_va_format (va_format);
    if (format == GST_VIDEO_FORMAT_UNKNOWN) {
      GST_DEBUG ("unsupported format %" GST_FOURCC_FORMAT,
          GST_FOURCC_ARGS (va_format->fourcc));
      continue;
    }
    append_format (formats, format, flags ? flags[i] : 0);

    switch (format) {
      case GST_VIDEO_FORMAT_YV12:
        YV12_idx = formats->len - 1;
        break;
      case GST_VIDEO_FORMAT_I420:
        I420_idx = formats->len - 1;
        break;
      default:
        break;
    }
  }

  /* Append I420 (resp. YV12) format if YV12 (resp. I420) is not
     supported by the underlying driver */
  if ((YV12_idx != -1) && (I420_idx == -1)) {
    fip = &g_array_index(formats, GstVaapiFormatInfo,YV12_idx);
    append_format (formats, GST_VIDEO_FORMAT_I420, fip->flags);
  } else if ((I420_idx != -1) && (YV12_idx == -1)) {
    fip = &g_array_index(formats, GstVaapiFormatInfo,I420_idx);
    append_format (formats, GST_VIDEO_FORMAT_YV12, fip->flags);
  }

> > 
> > When I run pipeline with vaapipostproc as you suggested:
> > 
> > videotestsrc is-live=true !
> > video/x-raw,width=1280,height=1024,framerate=20/1,format=BGRA !
> > vaapipostproc ! vaapih264enc ! testsink
> > 
> > I am getting following error and SEGV right after that:
> > 
> > 0x15ef37c0 ERROR          vaapipostproc
> > gstvaapipostproc.c:810:gst_vaapipostproc_process_vpp:<vaapipostproc0> failed
> > to apply VPP filters (error 2)
> > ==925== Thread 10 videotestsrc2:sr:
> > ==925== Invalid write of size 8
> > ==925==    at 0x41E8199: ??? (in /run/user/1000/orcexec.teOKWB (deleted))
> > ==925==    by 0x82A44DA: video_orc_pack_BGRA (tmp-orc.c:4653)
> > ==925==    by 0xA3689E5: convert_hline_generic (videotestsrc.c:1289)
> > ==925==    by 0xA368697: videotestsrc_convert_tmpline (videotestsrc.c:273)
> > ==925==    by 0xA368FAD: gst_video_test_src_smpte (videotestsrc.c:351)
> > ==925==    by 0xA3663E3: gst_video_test_src_fill (gstvideotestsrc.c:1152)
> > ==925==    by 0x6E055DE: gst_base_src_default_create (gstbasesrc.c:1474)
> > ==925==    by 0x6E0896E: gst_base_src_get_range (gstbasesrc.c:2453)
> > ==925==    by 0x6E0A2EE: gst_base_src_loop (gstbasesrc.c:2729)
> > ==925==    by 0x558E240: gst_task_func (gsttask.c:332)
> > ==925==    by 0x5AC8EED: g_thread_pool_thread_proxy (gthreadpool.c:307)
> > ==925==    by 0x5AC84F4: g_thread_proxy (gthread.c:784)
> > ==925==  Address 0x4035000 is not stack'd, malloc'd or (recently) free'd
> 
> This other issue looks like https://bugzilla.gnome.org/show_bug.cgi?id=779642
> 
> > When I comment out direct upload flag as you suggested in comment#7 it
> > starts working and it also stops printing this error that was always there:
> > 
> > 0x9c76500 ERROR       vaapivideomemory
> > gstvaapivideomemory.c:736:gst_video_info_update_from_surface: Cannot create
> > a VA derived image from surface 0x9ee5b60
> 
> Could this is because the tiled format does not get mapped as linear as it
> is on intel/vaapi. Maybe we can force linear when using mesa/vaapi. I think
> for the download case there is an env var something like that. So yes you
> can leave it as linear.
> 
Could be indeed something with the memory mappings because:

This one is working:
gst-launch-1.0 videotestsrc ! video/x-raw,format=NV12 ! vaapipostproc ! video/x-raw,format=RGBA ! fakesink

This one is not:
gst-launch-1.0 videotestsrc ! video/x-raw,format=RGBA ! vaapipostproc ! video/x-raw,format=NV12 ! fakesink

I would like to fix that a bit better than commenting the direct upload flag. Could you guide me where to look?

> > 
> > But it is still quite unstable under heavy load (>10 pipelines) with random
> > crashes on different places. I will put here some traces after I will
> > analyze it a bit more. What about the direct upload flag? Should I leave it
> > commented out? Thanks a lot.
> 
> I think it has not been tested with many decoder and encoder instances in
> the same process so we will have to solve all issues one by one I am afraid.
> Any chance that you can compile mesa and gstreamer-vaapi with less
> optimizations, like -O0.

I will do that. Thanks.
Comment 16 Víctor Manuel Jáquez Leal 2017-07-27 08:43:12 UTC
(In reply to Tomas Rataj from comment #15)
> static void
> append_formats (GArray * formats, const VAImageFormat * va_formats,
>     guint * flags, guint n)
> {
>   GstVideoFormat format;
>   int YV12_idx = -1;
>   int I420_idx = -1;
>   const GstVaapiFormatInfo *fip;
>   guint i;
> 
>   for (i = 0; i < n; i++) {
>     const VAImageFormat *const va_format = &va_formats[i];
> 
>     format = gst_vaapi_video_format_from_va_format (va_format);
>     if (format == GST_VIDEO_FORMAT_UNKNOWN) {
>       GST_DEBUG ("unsupported format %" GST_FOURCC_FORMAT,
>           GST_FOURCC_ARGS (va_format->fourcc));
>       continue;
>     }
>     append_format (formats, format, flags ? flags[i] : 0);
> 
>     switch (format) {
>       case GST_VIDEO_FORMAT_YV12:
>         YV12_idx = formats->len - 1;
>         break;
>       case GST_VIDEO_FORMAT_I420:
>         I420_idx = formats->len - 1;
>         break;
>       default:
>         break;
>     }
>   }
> 
>   /* Append I420 (resp. YV12) format if YV12 (resp. I420) is not
>      supported by the underlying driver */
>   if ((YV12_idx != -1) && (I420_idx == -1)) {
>     fip = &g_array_index(formats, GstVaapiFormatInfo,YV12_idx);
>     append_format (formats, GST_VIDEO_FORMAT_I420, fip->flags);
>   } else if ((I420_idx != -1) && (YV12_idx == -1)) {
>     fip = &g_array_index(formats, GstVaapiFormatInfo,I420_idx);
>     append_format (formats, GST_VIDEO_FORMAT_YV12, fip->flags);
>   }

Your proposal looks sensible. Can you provide a patch?

Thanks.
Comment 17 Tomas Rataj 2017-07-27 10:54:42 UTC
Created attachment 356464 [details] [review]
vaapi append_formats change pointers to indexes

Fix invalid read when YV12 or I420 is not supported by the driver
Comment 18 Julien Isorce 2017-07-31 22:35:07 UTC
(In reply to Tomas Rataj from comment #17)
> Created attachment 356464 [details] [review] [review]
> vaapi append_formats change pointers to indexes
> 
> Fix invalid read when YV12 or I420 is not supported by the driver

Victor, gentle ping ?
Comment 19 Víctor Manuel Jáquez Leal 2017-08-01 14:28:09 UTC
Review of attachment 356464 [details] [review]:

excepting that there is no commit log, the patch lgtm. I'm going to edit the commit log and push it.
Comment 20 Víctor Manuel Jáquez Leal 2017-08-01 14:45:53 UTC
Comment on attachment 356464 [details] [review]
vaapi append_formats change pointers to indexes

Attachment 356464 [details] pushed as commit c1813089
Comment 21 Víctor Manuel Jáquez Leal 2017-08-01 14:48:00 UTC
Comment on attachment 356100 [details] [review]
st/va: change frame_idx from array to hash table

rejected because it doesn't belong to gstreamer-vaapi but to mesa
Comment 22 Julien Isorce 2017-08-01 15:47:46 UTC
Comment on attachment 356100 [details] [review]
st/va: change frame_idx from array to hash table

Hi Tomas, Victor has closed the bug because this is for gstreamer-vaapi and he pushed your patch. Thx!

The mesa patch has been reviewed and accepted on mesa-dev mailing list so I prefer to leave it as reviewed until:

Please open a bug against mesa (using https://bugs.freedesktop.org/enter_bug.cgi?product=DRI this is a different site) and attach the patch.

Then for any other new bug please open a bug either against gstreamer-vaapi or mesa (make sure this is not a duplicate of https://bugzilla.gnome.org/show_bug.cgi?id=779642)

Thx for your patience on chasing these bugs !
Comment 23 Julien Isorce 2017-08-14 13:02:51 UTC
Comment on attachment 356100 [details] [review]
st/va: change frame_idx from array to hash table

Committed in Mesa https://bugs.freedesktop.org/show_bug.cgi?id=102006