<html><head></head><body>Hi,<br><br>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.<br><br>It's really the exact same logic and the same name as with audio filters. Nothing new here.<br><br>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.<br><br><div class="gmail_quote">Le 15 octobre 2020 12:46:41 GMT+03:00, Alexandre Janniaux <ajanni@videolabs.io> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">Hi,<br><br>On Thu, Oct 15, 2020 at 10:19:16AM +0300, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"> Hi,<br><br> Extra pictures are chained. That's how it always worked or has been supposed to work, to the extent that multiple pictures are supported.<br><br> Adding a different way to handle more than one picture vs zero or one picture is making things both more complicated and less consistent.<br><br> 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.<br></blockquote><br>In my work related to that (which is why I requested the ownership of<br>the ticket linked to this discussion from you), I extended filter(NULL)<br>to drain extra pictures without adding a EOS.<br><br>This was needed because my filter was spending 9ms per picture, and<br>being a frame doubler deinterlacer, it needed to generate two pictures<br>before this, for a final framerate of 60fps. By putting the second<br>frame into the same filter scope as the first, the first always arrived<br>a few ms late while the second was already done. Thus, I changed opengl<br>filters and vlc filter_t to move the backpressure to the beginning of<br>the filter pipeline and filter the filter chain starting from the last<br>filter able to output extra pictures, thus delaying the push of new<br>frames in this filter pipeline until everything has been processed.<br><br>It's particularily interesting with GPU buffers because we don't know<br>how many we need them currently and outputting two buffers might just<br>stall the pipeline since we cannot really dynamically allocate them, or<br>at least not without changing the video context and not in a general<br>way in my work, it also leads to the deinterlace filter to account for<br>a fixed processing time for each output frames instead of each input<br>frames, providing a regularization effect in the pipeline.<br><br>It was originally made to match the decoder's behaviour, but filters<br>are obviously different. Each input frames can produce multiple output<br>frames but the common case is that they are output in the same order<br>as the input order. In addition, decoders are running in their own<br>thread and hve the capability to proactively push pictures from there<br>whereas filters are asynchronous beasts waiting to be called to return<br>a new (currently, set of) picture, so it's currently a all or nothing<br>situation and typically if returning only one picture, drain() could<br>not even «block» until all pictures are available (without even<br>mentioning the computational cost of processing all missing pictures<br>at once).<br><br>The current system is also completely taylored for frame doubler<br>filters at the begining of the static chain, or said otherwise,<br>deinterlace filters, which led to a lot of pictures drop in almost<br>all other cases, even not-paced conversion. If you fix this and then<br>chain frame doublers, you have an even worse combinatorial explosion<br>for extra memory and computational power consumption with the current<br>system.<br><br>Thus, I'd suggest to remove the previous way to handle more than one<br>picture which even had no equivalent, and build a new way respecting<br>the backpressure right after the decoder and before the filters,<br>allowing a more robust filter pipeline.<br><br>Draining in this case was taken from the comments in the current code<br>but we can provide different naming. As far as I've understood this<br>part, you could separate this into two kinds of draining which were<br>draining with discontinuity or draining without discontinuity.<br><br>Being here, either we choose a model where discontinuity is signalled<br>in-band with the input (filter(NULL) for example) delegating its<br>handling to the module, and use a common drain() method, or we choose<br>a drain(bool discontinuity) which signals the state to the filter when<br>draining.<br><br>IMHO, the first is better because most filters don't differentiate<br>between discontinuity and extra picture drain, but we could also add<br>a dedicated info boolean for specific behaviour (which was originally<br>how I choosed to do it, so as to incrementally support filter(NULL) in<br>the different filters).<br><br>Personnally, since it was re-done here, I don't care about the method<br>choosed as long as the backpressure is moved adequately like I<br>described for my use case.<br><br>Regards,<br>--<br>Alexandre Janniaux<br>Videolabs<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Le 15 octobre 2020 08:34:55 GMT+03:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">On 2020-10-14 18:25, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">Le keskiviikkona 14. lokakuuta 2020, 15.39.09 EEST Steve Lhomme a<br></blockquote>écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;">Each filter using the common deinterlacing code now calls DoDrain<br></blockquote></blockquote>(similar<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;">to DoDeinterlacing) to do its draining.<br></blockquote>That does not sound like what is usually called draining, which means<br></blockquote>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.<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">process all buffers *before* an end or a discontinuity.<br></blockquote>Happy to use another term if there is one.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;">We no longer return pictures chained using<br></blockquote></blockquote>vlc_picture_chain_AppendChain().<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">This seems odd too.<br></blockquote>That's my whole goal with this "draining". To handle extra pictures<br>explicitly and consistently, forced by the API. Not have some rare<br>places (display for deinterlace, transcode for deinterlace/fps) handle<br>it manually and leave everything else up to<br>interpretation/guessing/parsing hundreds of modules.<br><br>As a transition I could do #16-#20 with just the picture and<br>picture_chain API's that can do this properly. And then we can<br>transition them to "drain" calls.<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote>--<br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.<br></blockquote><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;"><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>