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

Rémi Denis-Courmont remi at remlab.net
Thu Nov 26 17:21:51 CET 2020


Le torstaina 26. marraskuuta 2020, 9.31.25 EET Steve Lhomme a écrit :
> On 2020-11-25 19:47, Rémi Denis-Courmont wrote:
> > Le mercredi 25 novembre 2020, 09:05:01 EET Steve Lhomme a écrit :
> >> 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.
> > 
> > Fair enough. But IMO, it should not. As long as the chroma is the same,
> > the
> > change should be handled with source-change controls, and if the chroma
> > changes, then we should first try to adjust osys->converter and only if
> > that fails, restart the display.
> 
> Or a broader format_update callback which the display could decide to
> accept or reject. Just like they are allowed to reject a
> VOUT_DISPLAY_CHANGE_xxx control.
> 
> In the end there's a balance between controls and a callback. With a
> control you call tell exactly the one thing that changed. If many
> parameters change you need to make many control changes. Further
> delaying the handling of source changes.

Controls and dedicated callbacks have their respective pros and cons. I *tend* 
to prefer dedicated callbacks in new code but that's really a preference. If 
we would add an source orientation change requirement, it would most likely be 
better done with a control so as to share code with crop and aspect change 
control.

But that's really not the problem here.

The problem is that, unlike changing crop or aspect, changing pixel format in 
a display is much more involved. It's also not a straight success/failure 
situation, because we have to consider the fallback chromas and intermediate 
chroma for conversion. For comparison, displays are *required* to handle crop 
and aspect - that's as simple as it gets.

> >> This definition of change is completely arbitrary.
> > 
> > No. It's a convention, and all conventions have a level of arbitrary. But
> > that
> Yes, the converter is a special kind of filter, it's a "video converter"
> rather than a "video filter". That differentiation will still be needed
> no matter what. Not all display will be able to handle all pixel
> formats. This patchset doesn't change that. The update_format callback
> updated the converter according to the new preferred input of the display.
> 
> > convention is informed by years of experience and several rewrites of the
> > video output - especially the previous one by Laurent and myself.
> 
> You're talking about a time when there was no "push" and hardly any GPU
> handling.

No. We've had GPU path supported in master for 6 years now. More time has 
elapsed since then than had elapsed from the previous rework.

> We're in unknown territory (in the context of VLC).

Uh, no. Push was planned designed, not organic evolution. We even discussed 
this specific question while designing push.

> > So far the rules are that changing the source crop or A/R must be
> > supported by the display, and nothing else. We could probably consider
> > adding more change
> It's not a hard *must*.

It actually is a hard must. There are deliberately no provisions for displays 
to reject such change.

> > 3) I fear that too wild and drastic change of pixel formats will just
> > break
> > the conversion chain and expose bugs in displays.
> 
> IMO we already have all the tools to handle this.

We don't even have a ruleset for how to "break down" chroma conversion if 
there is no direct conversion from one to another. And the fallback chroma 
code is, well, simplistic and designed with only 8-bit components in mind.


-- 
Реми Дёни-Курмон
http://www.remlab.net/





More information about the vlc-devel mailing list