[vlc-devel] [PATCH 1/5] vout: update the format after filters

[Rémi Denis-Courmont]

Le keskiviikkona 21. lokakuuta 2020, 18.50.51 EEST Romain Vimont a écrit :
> On Wed, Oct 21, 2020 at 05:26:01PM +0300, Rémi Denis-Courmont wrote:
> > Le keskiviikkona 21. lokakuuta 2020, 15.51.03 EEST Romain Vimont a écrit :
> > > Meanwhile, this patchset is far less ambitious (and quite independent),
> > > it's just a local optimization/fix: instead of inserting a converter
> > > unconditionally on filters/vout format mismatch, the core just requests
> > > the vout to accept the new format directly. If it can't, it just inserts
> > > a converter like before, but if it can, this avoids an additional
> > > converter (and a failure for opaque formats).
> > 
> > And yet it still breaks in the ways that were already pointed out when
> > this
> > was proposed and rejected at the winter 2019 workshop.
> 
> I'm sorry, I don't understand how it breaks.
> 
> It just adds a possibility for the vout to accept a format change due to
> filters, in order to avoid the insertion of a converter. If the vout
> refuses, a converter is inserted as before.

That's pretty convoluted just for one single display module and naughty 
filters. Normally, we take the output of the previous filter and give that as 
input to the next, which is both simple and locally optimal.

I don't see why we should create a display with the wrong format, and then 
change the format. And it's exactly what we said not to do at the workshop.

> > But as was pointed out at the workshop, and as Steve already pointed
> > out again, if you need to change the pixel format, you need to go
> > through the display capability modules to re-apply priorities and
> > fallback chroma search.
> 
> For a "pure ultimate push model", maybe, but here this is not the case:
>  - if the vout can handle the format change, it does,
>  - otherwise, it refuses and a converter is inserted as before.

In other words, it breaks priorities and fallback chroma search, since you 
picked the display based on the wrong chroma.

> In any case, this is never worse (and it's sometimes better) than the
> current unconditional insertion of a converter

What you're doing is optimising for badly behaving, border-line broken, 
filters. For obvious performance reasons, filters should not change the format. 
And indeed, most current filters will conditionally fail depending on the 
format, rather than request conversions.

For audio, it's easy. We assume float and fail only on pass-through. But for 
video, we need to be realistic. Conversion is just too slow. Normally, there 
should be exactly one scaler and one to-RGB chroma converter in the pipeline 
(preferably within the display plugin itself rather than as a separate 
module).

-- 
Rémi Denis-Courmont
http://www.remlab.net/