[FFmpeg-devel] [PATCH 6/9] lavfi/vf_pp: convert to the video_enc_params API

[Anton Khirnov]

Quoting Michael Niedermayer (2020-04-18 21:52:22)
> On Sat, Apr 18, 2020 at 09:07:09PM +0200, Marton Balint wrote:
> > 
> > 
> > On Sat, 18 Apr 2020, Michael Niedermayer wrote:
> > 
> > >On Sat, Apr 18, 2020 at 07:03:16PM +0200, Marton Balint wrote:
> > >>
> > >>
> > >>On Sat, 18 Apr 2020, Michael Niedermayer wrote:
> > >>
> > >>>On Sat, Apr 18, 2020 at 12:14:12PM +0200, Anton Khirnov wrote:
> > >>>[...]
> > >>>>diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
> > >>>>index 3d0d4969b8..cb7ce7a158 100644
> > >>>>--- a/tests/fate/filter-video.mak
> > >>>>+++ b/tests/fate/filter-video.mak
> > >>>>@@ -535,7 +535,7 @@ FATE_FILTER_PP = fate-filter-pp fate-filter-pp1 fate-filter-pp2 fate-filter-pp3
> > >>>>FATE_FILTER_VSYNTH-$(CONFIG_PP_FILTER) += $(FATE_FILTER_PP)
> > >>>>$(FATE_FILTER_PP): fate-vsynth1-mpeg4-qprd
> > >>>>
> > >>>>-fate-filter-pp:  CMD = framecrc -flags bitexact -idct simple -i $(TARGET_PATH)/tests/data/fate/vsynth1-mpeg4-qprd.avi -frames:v 5 -flags +bitexact -vf "pp=be/hb/vb/tn/l5/al"
> > >>>>+fate-filter-pp:  CMD = framecrc -flags bitexact -export_side_data venc_params -idct simple -i $(TARGET_PATH)/tests/data/fate/vsynth1-mpeg4-qprd.avi -frames:v 5 -flags +bitexact -vf "pp=be/hb/vb/tn/l5/al"
> > >>>
> > >>>ffmpeg / ffplay should automatically enable the exportation of the parameters
> > >>>when theres a filter downstream that needs such parameters
> > >>>
> > >>>Otherwise the use of these filters (and other filters that need any kind of
> > >>>information thats unavailable by default) would become a bit akward to use
> > >>
> > >>Why? It is not unusual at all that you need to specify certain extra
> > >>parameters to export some kind of metadata.
> > >
> > >in addition to what carl said.
> > >It seems you consider that "normal", i wouldnt really consider it normal
> > >to have to specify this by hand.
> > >All the information is there for the code to do this automatically and its
> > >not really a A vs B alternative. Instead "-export_side_data venc_params"
> > >with a filter using this afterwards is the only functional choice
> > 
> > And how will you decide if a pp filter in a complex filtergraph is actually
> > processing your video input or not? Or how will you know that the pp filter
> > is parametrized in a way that it wants to use motion vectors?
> > 
> > This not looks to me easy at all. Automagically enabling this would look a
> > lot more like a heuristic to me, a case when the application wants to be
> > smarter than it actually is.
> 
> the filter requests the metadata types from its input from which it wants it.
> the prior filter passes the request on exactly the same but in the opposit
> direction to how it will pass frames metadata.
> 
> In reality for just pp* its much simpler actually as pp filters will be after the decoder
> no scale or crop even will be between because these would cause problems.
> one could imagine a split before a pp but thats very easy to pass back.
> there would not be something like a overlay before pp as again this wouldnt
> work without compensating the passed metadata (which is not done ATM).
> 
> You can also do it in a completely other way
> 1. Enable all export initially
> 2. Pass in AVFrames a way so that every subsequent consumer can mark each
>    data type as used or not used.
> 3. if frames get deallocated with some side/metadata never used the decoder
>    can stop exporting that in future frames
> 
> Theres also the simpler solution of simply giving each filter a flag for each
> exportable type of data and then have every decoder feeding a graph with a
> set flag to export that type of data.
> This would export more than needed in case of complex graphs 
> 
> Iam sure there are more solutions and iam sure ive missed some aspects of
> what i suggested above. But it seems to me this is a useful feature beyond
> this case here

I agree with Marton that this would be the case of a program trying to
be smarter than it is, which is IMO not appropriate here.

ffmpeg.c is suppposed to be, inasmuch as possible, a generic transcoder.
That design choice means that it is very powerful, but the corresponding
drawback is that it requires a bit more effort to make it do exactly
what you want it to do. I believe that trying to make it work
"automagically" for all imaginable use cases
- will not work anyway because there are more of those than anyone can
  handle
- will cause the code to become a convoluted impossible-to-understand
  mess that breaks all the time for inexplicable reasons

In this case specifically, there is no guarantee that the mere presence
of the pp filter implies that the user wants the decoder to export this
metadata. Maybe the user wants to generate the tables manually with the
qp filter. Or load them from some other source that will be added in the
future. Other possibilities are also imaginable.
Furthermore, pp is not the only user of the QP tables. Is ffmpeg.c to
maintain a constantly-changing list of filters and magically conjure up
decoder flags based on those? All my experience tells me this will only
lead to pain.

-- 
Anton Khirnov