[vlc-devel] [PATCH 17/17] video_output: restart display after filter change

[Steve Lhomme]

On 2020-11-24 16:37, Rémi Denis-Courmont wrote:
> Le lundi 23 novembre 2020, 14:21:29 EET Steve Lhomme a écrit :
>> On 2020-11-22 10:22, Rémi Denis-Courmont wrote:
>>> Le perjantaina 20. marraskuuta 2020, 16.45.07 EET Steve Lhomme a écrit :
>>>> This is done in the vout thread loop, immediately after the filter output
>>>> change is detected, before processing pictures coming out of the filters.
>>>> The filters are untouched (not flushed).
>>>>
>>>> The new display module is created with the format coming out of the last
>>>> filter.
>>>
>>> As you have pointed out yourself, this causes glitches if the windowing
>>> system is not capable of retaining the window content while restarting
>>> the display. AFAICT; without this patch, it works just fine: the
>>> conversion filter chain is adjusted and the display has nothing to worry
>>> about.
>>
>> It doesn't work *just fine*, an extra filter is added to match the
>> original display format.
> 
> Itss removing the old conversion and adding a new conversion. That's working
> fine and as intended.

No. The osys->converter is untouched. The added converter is added in 
filter.chain_interactive (as you can see from this patch). So when 
adding a filter in the chain(s) that changes the format, an *extra* 
converter is added, before osys->converter. Just read the code.

The fact that canceling a format chain is *intended* is completely bogus.

>> You said yourself that the display should be restarted when the source
>> changes and now you're against it because it causes glitches ?
> 
> I wrote that, if it is somehow necessary to change the video format of a
> display in a way that is not currently permitted by the change controls, the
> display should be restarted, e.g. the "chroma" or pixel format.

This definition of change is completely arbitrary. Even a chroma/pixel 
format will be handled just fine by OpenGL/D3D11 with small changes in 
the pixel shaders. Changing from rectangular to 360 video on the other 
hand is just one flag changed but may not be handled properly (dunno if 
Vulkan handles it yet for example).

> Still you should avoid getting into a situation where the display needs to be
> restarted at all.

We agree on that.

> In particular, I don't challenge that it might cause glitches within the
> window. Those glitches are more tolerable than window glitches which
> potentially steal the focus, move the window some place else, etc.

Just as restarting the display in some cases is inevitable, some 
glitches are bound to happen as well. But there are tons of cases where 
it can be avoided and should be.

>>> Of course, that assumes the display is sensibly implemented when it comes
>>> to picking its input format. In particular a CPU display should always
>>> request CPU formats and a GPU display GPU formats - memory transfer is
>>> best handled by conversion filters.
> 
> -- 
> Rémi Denis-Courmont
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>