[vlc-devel] [PATCH 2/2][RFC] VAAPI XCB video output

[Jean-Baptiste Kempf]

On 25 Aug, Petri Hintukainen wrote :
> > > +include hw/va/Makefile.am
> > >  include hw/vdpau/Makefile.am
> > Do we want more Makefile.am here? Rémi?
> > 
> > (btw, why is not hw/mmal/ listed?)
> 
> mmal is in SUBDIRS

Which way is better ? :)

> > > +static int OutputOpen(vlc_object_t *obj)
> > > +{
> > > +    if (filter->fmt_in.video.orientation != filter-
> > > >fmt_out.video.orientation) {
> > > +        return VLC_EGENERIC;
> > Any simple way of handling rotations?
> 
> Rotation is handled in output module. It could be handled in filter
> too, but it is simpler this way (at least until some "real" filtering
> is added).

OK. Sorry for my misunderstanding then.

> Maybe other transforms should be handled here (there's only rotation
> support in VA-API). But how common are those in "real life" ?

Not existing.

> > > +    msg_Info(o, "VA-API: v%d.%d (%s)\n", major, minor, vendor);
> > I'd argue this requires a better message.
> 
> Um... Like "Trying to use ... for decoding and/or rendering" ?

Yes. Msg_Info are always visible.

> > > +#if 0
> > > +    /* too late to test here ? */
> > > +    status = vlc_va_TestPutImage(va_dpy, &va_format,
> > > +                                 va_surface_ids[0], NULL,
> > > +                                 fmt->i_visible_width, fmt-
> > > >i_visible_height);
> > > +#endif
> > Why would it be too late?
> 
> Picture pool is not created in Open(), so vout is already successfully
> created when we enter here.
> 
> Currently the test is in upload filter, but that won't be used with HW
> decoding.

OK.

> > > +    plane_t dst_planes[3];
> > If so, don't we need to assert on src->i_planes to be <= 3?
> 
> Right.
> Or check the format first.

Or both :)

> > > +    for (int i = 0; i < src->i_planes; i++) {
> > > +        plane_CopyPixels(&dst_planes[i], &src->p[i]);
> > > +    }
> > memcpy is murder :)
> 
> Even double one here. vaDeriveImage() may return mapped GPU memory, so
> "normal" memcpy is really slow. I can add optimized memcpy later.

modules/video_chroma/copy.h|.c ?

> But there's no way to avoid copying here. Maybe it would be possible
> when filters can provide picture pool. But, still, decoding directly to
> GPU memory would kill SW decoder performance.

I think elenril has somehting to say about this topic, though...

> > subpicture_region_t ?
> 
> subpicture_region_t is over-complex, and I didn't find any (existing)
> simpler alternatives ...

OK

> > Can't we merge this with the classical XCB output?
> 
> Should be possible with some ifdefs and runtime checks. Most of va
> specific code is in the helper functions. It should be quite "easy" to
> drop to other vout plugins too (ex. OpenGL).

Well, this is the core question.

We have now too many video outputs, and they are going to be more
complex, because all of them will need to support 360, HDR and possibly
stereo. Therefore, we should focus on a few:
 - OpenGL will be used for Linux, BSD, iOS and MacOS
 - Direct3D11 on the modern Windows (and D3D9 won't be upgraded too
   much)
 - AndroidSurface on Android, but OpenGL will take more and more space
   here.
 - XCB is a fallback we must maintain for a bit.

So, for hw accelerations, we should focus, as much as possible, to map
the textures to an existing vout. With stuff like
getXcbTextureFromVA(picture_t *), getRGBTextureFromCVPixBuf( picture_t *),
etc.

Of course, I speak about the code source level.

> But that will add runtime dependency for libva.

I don't see how, for the normal module. (note that this could be a
submodule with the special conversion function)

> > Also, shouldn't this be a different patch?
> 
> If you prefer so.

Yep.

> But note that those modules are useless alone (can't be even tested).

Yes.

> I'll send new version after I've tested how well HW decoding integrates
> to all this.

Btw, perf?


-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device