On Sun, Jan 3, 2021 at 1:25 PM Alexander Kapshuk
<alexander.kaps...@gmail.com> wrote:
>
> On Sun, Jan 3, 2021 at 7:20 PM Ilia Mirkin <imir...@alum.mit.edu> wrote:
> >
> > On Sun, Jan 3, 2021 at 3:18 AM Alexander Kapshuk
> > <alexander.kaps...@gmail.com> wrote:
> > >
> > > NVIDIA chip affected:
> > > 01:00.0 VGA compatible controller: NVIDIA Corporation GT216 [GeForce
> > > 210] (rev a1)
> > >
> > > The null pointer dereference occurs here:
> > > Thread 27 "vlc" received signal SIGSEGV, Segmentation fault.
> > > [Switching to Thread 0x7fff8f7c1640 (LWP 79292)]
> > > 0x00007fff8d59d1da in vl_compositor_yuv_deint_full (s=0x7fff980e8518,
> > > c=0x7fff980e83d8, src=0x7fff98670030, dst=0x0,
> > > src_rect=0x7fff8f7c0470,
> > > dst_rect=0x7fff8f7c0460, deinterlace=VL_COMPOSITOR_WEAVE) at
> > > ../mesa-20.3.2/src/gallium/auxiliary/vl/vl_compositor.c:689
> > > 689 dst_surfaces = dst->get_surfaces(dst); //dst==NULL
> > >
> > > => 0x00007fff8d5981da <+42>: call *0x38(%rcx) //rcx is dst
> > > (gdb) i r rcx
> > > rcx 0x0 0
> > >
> > > (gdb) bt
> > > #0 0x00007fff8d59d1da in vl_compositor_yuv_deint_full
> > > (s=0x7fff980e8518, c=0x7fff980e83d8, src=0x7fff98670030, dst=0x0,
> > > src_rect=0x7fff8f7c0470, dst_rect=0x7fff8f7c0460,
> > > deinterlace=VL_COMPOSITOR_WEAVE) at
> > > ../mesa-20.3.2/src/gallium/auxiliary/vl/vl_compositor.c:689
> > > #1 0x00007fff8d58a29b in vlVaDeriveImage (ctx=0x7fff980c1590,
> > > surface=<optimized out>, image=0x7fff8f7c05e0)
> > > at ../mesa-20.3.2/src/gallium/frontends/va/image.c:321
> > > #2 0x00007fff91485799 in vaDeriveImage () at /usr/lib/libva.so.2
> > > #3 0x00007fff8e2256d2 in () at
> > > /usr/lib/vlc/plugins/video_output/libglconv_vaapi_x11_plugin.so
> > > #4 0x00007fff8e224189 in () at
> > > /usr/lib/vlc/plugins/video_output/libglconv_vaapi_x11_plugin.so
> > > #5 0x00007fff8f6b1896 in () at
> > > /usr/lib/vlc/plugins/video_output/libgl_plugin.so
> > > #6 0x00007fff8f6b86db in () at
> > > /usr/lib/vlc/plugins/video_output/libgl_plugin.so
> > > #7 0x00007ffff7d07cee in () at /usr/lib/libvlccore.so.9
> > > #8 0x00007ffff7cfa019 in () at /usr/lib/libvlccore.so.9
> > > #9 0x00007ffff7cfbf9e in () at /usr/lib/libvlccore.so.9
> > > #10 0x00007ffff7f623e9 in start_thread () at /usr/lib/libpthread.so.0
> > > #11 0x00007ffff7e8a293 in clone () at /usr/lib/libc.so.6
> > >
> > > mesa-20.3.2/src/gallium/frontends/va/image.c:312,313
> > > VAStatus
> > > vlVaDeriveImage(VADriverContextP ctx, VASurfaceID surface, VAImage *image)
> > > {
> > > ...
> > > new_template.interlaced = false; //create_video_buffer
> > > returns NULL if new_template.interlaced is set to false See below.
> > > new_buffer = drv->pipe->create_video_buffer(drv->pipe,
> > > &new_template);
> > > ...
> > > vl_compositor_yuv_deint_full(&drv->cstate, &drv->compositor,
> > > surf->buffer, new_buffer,
> > > &src_rect, &dst_rect,
> > > VL_COMPOSITOR_WEAVE);
> > > ...
> > > }
> > >
> > > [Note]
> > > mesa-20.3.2/src/gallium/drivers/nouveau/nv50/nv84_video.c:618,621
> > > struct pipe_video_buffer *
> > > nv84_video_buffer_create(struct pipe_context *pipe,
> > > const struct pipe_video_buffer *template)
> > > {
> > > ...
> > > if (!template->interlaced) { //setting this to true in
> > > vlVaDeriveImage returns valid buffer
> > > debug_printf("Require interlaced video buffers\n");
> > > return NULL;
> > > }
> > > ...
> > > }
> > >
> > > Please advise on the further course of action.
> >
> > Figure out what change in mesa broke it, send a revert (or fix, if
> > you're able). There has been a bunch of activity in st/vl lately, and
> > the developers never take NVIDIA into account, only AMD (well, they're
> > AMD developers, so not entirely surprising).
> >
> > Cheers,
> >
> > -ilia
>
> The following commit,
> https://gitlab.freedesktop.org/mesa/mesa/-/commit/338745c6f4b7133d7b36f78562d46bc4e8d368f5,
> introduced derive_interlaced_allowlist, which currently comprises the
> 'vlc' media player. Which, as far as I could tell, was to make an
> exception for vlc, so a video buffer would be created when
> surf->buffer->interlaced was set to true.
> VA_STATUS_ERROR_OPERATION_FAILED is returned otherwise.
>
> Whereas, this commit,
> https://gitlab.freedesktop.org/mesa/mesa/-/commit/fcb558321e65b62244a11e0066bb8713b1854721,
> is the one responsible for new_buffer being set to NULL, because it
> explicitly sets new_template.interlaced to false. New_buffer ends up
> being passed as a dst parameter and dereferenced.
>
> As far as the patch goes, I've set new_template.interlaced to true in
> my local build, which fixes the null pointer dereference for me:
> mesa-20.3.2/src/gallium/frontends/va/image.c:311,313
> new_template = surf->templat;
> - new_template.interlaced = false;
> + new_template.interlaced = true;
> new_buffer = drv->pipe->create_video_buffer(drv->pipe,
> &new_template);
>
> What is the convention for submitting patches on this mailing list?
> Is it done via email, like LKML, which I'm more familiar with?
> Or is gitlab, or github the preferred way of submitting patches?
I believe gitlab, with the project over at
https://gitlab.freedesktop.org/mesa/mesa. This will require you to
create an account, create a clone, push a branch, open a merge
request. IMO it's too much to ask of one-time contributors (I find it
to be enough of a pain to have stopped contributing to mesa for the
most part since the gitlab migration), so you can also send mail in
the usual way to mesa-...@lists.freedesktop.org (although I'd add a
comment below the --- to pre-empt the "submit on gitlab" discussion).
That said, your patch is unlikely to be accepted -- the whole point of
the original commit was to auto-deinterlace (which seems like a horrid
idea, but perhaps vaDeriveImage is meant to do that?). The problem is
that nouveau only supports interlaced video buffers, hence the various
issues. From the vaDeriveImage docs,
"""
When the operation is not possible this interface will return
VA_STATUS_ERROR_OPERATION_FAILED. Clients should then fall back to
using vaCreateImage + vaPutImage to accomplish the same task in an
indirect manner.
"""
So I'd guess the proper solution is to detect whether the underlying
implementation supports non-interlaced video buffers, and bail with
the failed operation if it can't. Or to add support for non-interlaced
buffers to nouveau. While the hardware has no support for outputting
video to such buffers, one could imagine adding support for creating
these with the proper amounts of memory/etc backing the surfaces.
However I think the whole thing should just be reverted and
libva-using software fixed -- returning the operation failed is
totally acceptable behavior that they need to be able to deal with.
Cheers,
-ilia
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
