[vlc-devel] [PATCH v2 09/20] deinterlace: implement draining of extra pictures

[Alexandre Janniaux]

Hi,

On Thu, Oct 15, 2020 at 10:19:16AM +0300, Rémi Denis-Courmont wrote:
> Hi,
>
> Extra pictures are chained. That's how it always worked or has been supposed to work, to the extent that multiple pictures are supported.
>
> Adding a different way to handle more than one picture vs zero or one picture is making things both more complicated and less consistent.
>
> Draining is used to forcefully process buffers at EOS; this is also how audio works. It's just that nobody bothered to convert the filter_video(NULL) to drain_video(NULL). I disagree with changing the meaning/scope of drain.

In my work related to that (which is why I requested the ownership of
the ticket linked to this discussion from you), I extended filter(NULL)
to drain extra pictures without adding a EOS.

This was needed because my filter was spending 9ms per picture, and
being a frame doubler deinterlacer, it needed to generate two pictures
before this, for a final framerate of 60fps. By putting the second
frame into the same filter scope as the first, the first always arrived
a few ms late while the second was already done. Thus, I changed opengl
filters and vlc filter_t to move the backpressure to the beginning of
the filter pipeline and filter the filter chain starting from the last
filter able to output extra pictures, thus delaying the push of new
frames in this filter pipeline until everything has been processed.

It's particularily interesting with GPU buffers because we don't know
how many we need them currently and outputting two buffers might just
stall the pipeline since we cannot really dynamically allocate them, or
at least not without changing the video context and not in a general
way in my work, it also leads to the deinterlace filter to account for
a fixed processing time for each output frames instead of each input
frames, providing a regularization effect in the pipeline.

It was originally made to match the decoder's behaviour, but filters
are obviously different. Each input frames can produce multiple output
frames but the common case is that they are output in the same order
as the input order. In addition, decoders are running in their own
thread and hve the capability to proactively push pictures from there
whereas filters are asynchronous beasts waiting to be called to return
a new (currently, set of) picture, so it's currently a all or nothing
situation and typically if returning only one picture, drain() could
not even «block» until all pictures are available (without even
mentioning the computational cost of processing all missing pictures
at once).

The current system is also completely taylored for frame doubler
filters at the begining of the static chain, or said otherwise,
deinterlace filters, which led to a lot of pictures drop in almost
all other cases, even not-paced conversion. If you fix this and then
chain frame doublers, you have an even worse combinatorial explosion
for extra memory and computational power consumption with the current
system.

Thus, I'd suggest to remove the previous way to handle more than one
picture which even had no equivalent, and build a new way respecting
the backpressure right after the decoder and before the filters,
allowing a more robust filter pipeline.

Draining in this case was taken from the comments in the current code
but we can provide different naming. As far as I've understood this
part, you could separate this into two kinds of draining which were
draining with discontinuity or draining without discontinuity.

Being here, either we choose a model where discontinuity is signalled
in-band with the input (filter(NULL) for example) delegating its
handling to the module, and use a common drain() method, or we choose
a drain(bool discontinuity) which signals the state to the filter when
draining.

IMHO, the first is better because most filters don't differentiate
between discontinuity and extra picture drain, but we could also add
a dedicated info boolean for specific behaviour (which was originally
how I choosed to do it, so as to incrementally support filter(NULL) in
the different filters).

Personnally, since it was re-done here, I don't care about the method
choosed as long as the backpressure is moved adequately like I
described for my use case.

Regards,
--
Alexandre Janniaux
Videolabs

> Le 15 octobre 2020 08:34:55 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
> >On 2020-10-14 18:25, Rémi Denis-Courmont wrote:
> >> Le keskiviikkona 14. lokakuuta 2020, 15.39.09 EEST Steve Lhomme a
> >écrit :
> >>> Each filter using the common deinterlacing code now calls DoDrain
> >(similar
> >>> to DoDeinterlacing) to do its draining.
> >>
> >> That does not sound like what is usually called draining, which means
> >toIt might be tricky to make Mesa robust against this... While the GLX spec says this should be handled gracefully, I suspect it might be easier for VLC to destroy the GLX context before the window.
> >> process all buffers *before* an end or a discontinuity.
> >
> >Happy to use another term if there is one.
> >
> >>> We no longer return pictures chained using
> >vlc_picture_chain_AppendChain().
> >>
> >> This seems odd too.
> >
> >That's my whole goal with this "draining". To handle extra pictures
> >explicitly and consistently, forced by the API. Not have some rare
> >places (display for deinterlace, transcode for deinterlace/fps) handle
> >it manually and leave everything else up to
> >interpretation/guessing/parsing hundreds of modules.
> >
> >As a transition I could do #16-#20 with just the picture and
> >picture_chain API's that can do this properly. And then we can
> >transition them to "drain" calls.
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel