[vlc-devel] [PATCH v2 09/20] deinterlace: implement draining of extra pictures
Steve Lhomme
robux4 at ycbcr.xyz
Thu Oct 15 12:56:24 CEST 2020
On 2020-10-15 12:16, Rémi Denis-Courmont wrote:
> Hi,
>
> You can't drain pictures without EOS. A deinterlacing filter with one
> frame delay (i.e looking one frame into the future) needs to know if
> there will be another frame or not. Draining is only if there will not
> be another frame. That's what my ticket is all about.
>
> It's really the exact same logic and the same name as with audio
> filters. Nothing new here.
>
> Filters have to return all their processed pictures as a chain. It would
> be easier of we had an owner.queue_picture() callback but we don't.
I don't think it would solve the issue Alexandre mentions: computing one
picture at a time to spread the computation time among calls. If the
filter decides when to compute, then the owner loses the option to
choose and we're back to the late frame issue.
> Le 15 octobre 2020 12:46:41 GMT+03:00, Alexandre Janniaux
> <ajanni at videolabs.io> a écrit :
>
> 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
>
> ------------------------------------------------------------------------
> 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
>
More information about the vlc-devel
mailing list