[Bug 785085] multiple vaapi encoding pipelines on radeonsi segfault

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=785085

--- Comment #15 from Tomas Rataj <rataj28 at gmail.com> ---
(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.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.