[vlc-devel] Display update/restart

[Steve Lhomme]

On 2020-11-16 12:54, Alexandre Janniaux wrote:
> Hi,
> 
> I actually lean towards a mix of #1 and #2, ie. creating the display
> after the filters (meaning that the only hint filters have is the
> decoder device that should be video device) and then potentially
> allowing the format to change for some metadatas.

Makes sense. Although it's a bigger change from what we use now. The 
vout_Request will not actually create a display, only when a picture 
arrives ? The display probing and loading might take too much time and 
the first picture will be late.

Also in some interlaced samples the first frame is not marked as 
interlaced so some transition will be needed anyway. But in a clean 
source the first picture is marked as interlaced so we can start cleanly 
with the format "pushed" by the deinterlacer (in the case of interlaced 
HEVC double height, half frame rate). Hence the importance of trigerring 
the deinterlacer *before* displaying a picture (pushed code that 
mysteriously disappeared).

> #2 is much easier to do (it's done and has been submitted by Romain)
> and fits the current model, but #1 is much cleaner. However, we must
> take care of the different factors that might restart a display:
> 
>   - a chroma change is likely to restart filters/display anyway
>   - a colorimetry metadata change shouldn't restart the display if the
>     display supports metadata
>   - an aspect ratio change shouldn't restart the display too if the
>     display is able to scale the picture

That's a bit messy right now. The source SAR is never actually passed to 
the display module but a mix of source SAR+user DAR. And the display 
module can only refuse it if it handles the reset_picture callback 
(which BTW shouldn't exist in scenario #1 either).

> That's more or less splitting the video format information from the
> video metadata information, but there's multiple way to define this.
> 
> About the interlacing part, wouldn't it be easier to have the correct
> size for interlaced content anyway, but considering interlaced content
> like a different format (meaning it needs a filter or a display able
> to deinterlace in one way or another)? 

I'm not sure what you mean by "correct size". Do you mean that pictures 
with only 1 field should have double the height anyway ? IMO that's a 
waste of resource since they shouldn't be displayed that way. It does 
help to create the window at the proper size. For now the window is 
created on vout_Request() from the video_format_t information. It 
doesn't carry (yet) the fact there is only one field per picture (patch 
incoming soon).

Once you have that information in the video format, a video format with 
1 or 2 field per picture is a different source as far as the video 
ouptut code is concerned (memcmp on all fields of the video format 
except for the SAR).

> It would mean that we need a
> way to enforce interlacing format check which might be harder than
> what it seems by just saying that, 

The interlacing (b_progressive) is currently only in the picture (as 
well as the number of fields) it is checked in a different place than 
the video format. But it possible to move it to the video format and 
merge everything.

> but size doesn't really mean
> something when you typically have half the lines that might not be used
> contiguously when displaying. Did you investigate such design?

I tried moving the interlaced flag to the video format but not just 
checking the video format for changes. But there's a side note to this. 
The video output needs 2 ways to handle format change: the 
decoder_UpdateVideoOutput way and checking fields didn't change in 
incoming pictures. They are complementary but don't have the same 
effect. They would both be source of display update/creation.

> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> On Mon, Nov 16, 2020 at 12:24:33PM +0100, Steve Lhomme wrote:
>> One of the last missing part in push is the possibility to update/restart
>> the display module when the source changes. A corollary of this is when we
>> decide to use a converter filter or not.
>>
>> The current way of handling source changes is to redo the filter chain(s)
>> and add an extra filter if the last filter doesn't output what it was
>> supposed to. So the source change is effectively canceled by this last
>> filter.
>>
>> It's even worse because instead of updating osys->converter we add an extra
>> filter to the interactive chain. And then there will still be the old
>> osys->convereter. I have patches to fix this and update the osys->converter
>> instead (the extra added filter never needed to be "interactive").
>>
>> No matter what, we will still need the osys->converter because not all
>> display modules can handle all chroma/colorimetry/360 formats we send them.
>>
>> So when a new format should we
>> - create a new display no matter what ?
>> - reuse the display if possible ? And if not create a new display ?
>>
>>  From previous mail it doesn't seem clear what we should do. It seems Remi is
>> leaning towards #1 while Alexandre, Romain and myself towards #2.
>>
>> This is becoming critical because we have to handle interlaced HEVC which
>> decoders output field by field, rather than 2 fields at a time for all other
>> interlaced codecs. So when deinterlacing kicks in (patches coming soon), the
>> dimension coming out of filter is double the height. To display it properly
>> we need to update the display or create a new one. We should not use a
>> filter to reduce by half the height and display it in the old display. And
>> given the picture surface is actually double the size, it's quite a big
>> change for the display module.
>>
>> Even if we try to reuse the display with the new format, there will always
>> be cases where the display refuses. So we have to handle the
>> restart/recreate no matter what.
>>
>> There is another issue arising from this. We only know about this format
>> change after the picture is deinterlaced. With the current code, that
>> happens when preparing (pre-rendering) a picture. Except this is when
>> deinterlacing the picture.next which is not the one that will be sent to the
>> display right away (picture.current). So this is not the right moment to
>> restart the display. We may need to change how we handle this.
>>
>> I think it is better to reuse the display module if possible. In the end
>> people are only going to use opengl/direct3d11 99% of the time and they are
>> capable of handling such texture size changes, even without recompiling
>> shaders. It's a lot less time sensitive as well. We don't have to worry if
>> we'll have time to restart a display to display the frame that has the new
>> format.
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>