[vlc-devel] [PATCH 12/14] filter_chain: add a default implementation for the video_allocator

[Steve Lhomme]

On 2019-07-25 19:08, Rémi Denis-Courmont wrote:
> Le torstaina 25. heinäkuuta 2019, 13.34.11 EEST Steve Lhomme a écrit :
>> At some point NULL should not be allowed there.
> 
> I don't think we should have allocation callbacks in filters at all anymore. We
> got rid of them for audio years ago.

It cannot work for filters outputting GPU surfaces. You cannot use a 
generic picture_NewFromFormat here. You have to use at least the 
(output) video context and some video context specific callback. This 
callback may be attached to the video context but there might be cases 
where the filter needs to do its own tweaks (for example in D3D11 you 
may decide to allocate pictures that are mappable to the CPU or not, in 
D3D9 decoder and filter surfaces have a different type than display 
surfaces).

> First, and it's always been a problem, the last filter in the chain may well
> just pass on picture buffers from upstream, without allocating anything. So we
> cannot rely on the allocation callback.

Not all display modules can deal with pictures they did not allocate 
(the majority of cases). So either we force a copy there or we allow the 
last filter to use pictures from the display pool. This is what we do 
now and I don't think we should change this.

There is indeed a problem for filter that pass pictures from the input 
directly to the output. They don't do filter_NewPicture() and thus 
bypass the whole "filter chain last filter" trick. For that we may have 
to keep the ugly picture_pool_OwnPic() function.

> And, unlike for decoders, we don't need flow control either. None of the filters
> return more than O(n) output pictures per input picture. In fact, I think the
> "worst" case is just 2 outputs per input (deinterlace).

It's true for now as in-flight pictures from filters are always handle 
late, just before displaying. If we ever want to do some filtering 
earlier and/or in threads this may change. Plus there's nothing stopping 
a filter from holding 8 pictures from upstream to display all of them at 
once.

>> ---
>>   src/misc/filter_chain.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/src/misc/filter_chain.c b/src/misc/filter_chain.c
>> index 6a7b8d7a44..d4652ebf93 100644
>> --- a/src/misc/filter_chain.c
>> +++ b/src/misc/filter_chain.c
>> @@ -180,6 +180,11 @@ void filter_chain_Reset( filter_chain_t *p_chain, const
>> es_format_t *p_fmt_in, }
>>   }
>>
>> +static picture_t *DefaultNewPicture( filter_t *filter )
>> +{
>> +    return picture_NewFromFormat( &filter->fmt_out.video );
>> +}
>> +
>>   static filter_t *filter_chain_AppendInner( filter_chain_t *chain,
>>       const char *name, const char *capability, config_chain_t *cfg,
>>       const es_format_t *fmt_in, const es_format_t *fmt_out )
>> @@ -232,6 +237,12 @@ static filter_t *filter_chain_AppendInner(
>> filter_chain_t *chain, if( filter->p_module == NULL )
>>           goto error;
>>
>> +    if ( filter->filter_allocator.buffer_new == NULL )
>> +    {
>> +        // legacy filters not setting a callback to create their output
>> pictures +        filter->filter_allocator.buffer_new = DefaultNewPicture;
>> +    }
>> +
>>       if( filter->b_allow_fmt_out_change )
>>       {
>>           es_format_Clean( &chain->fmt_out );
> 
> 
> -- 
> Реми Дёни-Курмон
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>