diff mbox

[FFmpeg-devel] configure: deprecate libpostproc and pp filter

Message ID 20180426230816.56958-1-josh@itanimul.li
State New
Headers show

Commit Message

Josh Dekker April 26, 2018, 11:08 p.m. UTC
The postproc library is only used in a single filter, so should be moved into the filter itself if the filter was to stay, but the filter has all of its internal filters now in lavfi itself. (Also it's a bit weird to have a separate library of filters which is used in a filter in the filter library).
---
 configure | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

James Almer April 26, 2018, 11:15 p.m. UTC | #1
On 4/26/2018 8:08 PM, Josh de Kock wrote:
> The postproc library is only used in a single filter, so should be moved into the filter itself if the filter was to stay, but the filter has all of its internal filters now in lavfi itself. (Also it's a bit weird to have a separate library of filters which is used in a filter in the filter library).

If libpostproc can't be merged into the filter (or nobody bothers to do
it), then it could be moved to a separate repository, much like the
nvidia headers.

> ---
>  configure | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 9fa1665496..1e00c3e7a4 100755
> --- a/configure
> +++ b/configure
> @@ -130,7 +130,7 @@ Component options:
>    --disable-avformat       disable libavformat build
>    --disable-swresample     disable libswresample build
>    --disable-swscale        disable libswscale build
> -  --disable-postproc       disable libpostproc build
> +  --enable-postproc        disable libpostproc build (deprecated) [no]
>    --disable-avfilter       disable libavfilter build
>    --enable-avresample      enable libavresample build (deprecated) [no]
>    --disable-pthreads       disable pthreads [autodetect]
> @@ -3529,7 +3529,7 @@ intrinsics="none"
>  enable $PROGRAM_LIST
>  enable $DOCUMENT_LIST
>  enable $EXAMPLE_LIST
> -enable $(filter_out avresample $LIBRARY_LIST)
> +enable $(filter_out postproc $(filter_out avresample $LIBRARY_LIST))
>  enable stripping
>  
>  enable asm
> @@ -6674,6 +6674,8 @@ check_deps $CONFIG_LIST       \
>  
>  enabled threads && ! enabled pthreads && ! enabled atomics_native && die "non pthread threading without atomics not supported, try adding --enable-pthreads or --cpu=i486 or higher if you are on x86"
>  enabled avresample && warn "Building with deprecated library libavresample"
> +enabled postproc && warn "Building with deprecated library libpostproc"
> +enabled pp_filter && warn "Building with deprecated filter pp"

Superfluous warning, the first one is enough. You don't see a warning
for resample_filter below the one for libavresample.

>  
>  if test $target_os = "haiku"; then
>      disable memalign
>
Carl Eugen Hoyos April 26, 2018, 11:19 p.m. UTC | #2
2018-04-27 1:08 GMT+02:00, Josh de Kock <josh@itanimul.li>:
> The postproc library is only used in a single filter

Is libswscale used in more filters? Are you planning to
deprecate it?

Seriously: The library is needed for compliance with
multimedia standards, it should not be deprecated.

Carl Eugen
Josh Dekker April 26, 2018, 11:28 p.m. UTC | #3
On 2018/04/27 0:19, Carl Eugen Hoyos wrote:
> 2018-04-27 1:08 GMT+02:00, Josh de Kock <josh@itanimul.li>:
>> The postproc library is only used in a single filter
> 
> Is libswscale used in more filters? Are you planning to
> deprecate it?

No, libswscale does not suffer from the same issue of being a secondary 
filter library. See for example how libavresample is the secondary 
resampling library, which has been deprecated for being regarded as 
worse than libswscale. libpostproc is just another filtering library 
(which was shoved into lavfi instead of its filters being moved into 
lavfi). Some of the postproc filters seemingly have equivalents in lavfi 
(just going off of names), so I think removal of postproc while making 
sure equivalent filters will be available is the best way to solve this.

> Seriously: The library is needed for compliance with
> multimedia standards, it should not be deprecated.

I'm unsure of which multimedia standards you refer to.
Josh Dekker April 26, 2018, 11:30 p.m. UTC | #4
On 2018/04/27 0:15, James Almer wrote:
> On 4/26/2018 8:08 PM, Josh de Kock wrote:
>> The postproc library is only used in a single filter, so should be moved into the filter itself if the filter was to stay, but the filter has all of its internal filters now in lavfi itself. (Also it's a bit weird to have a separate library of filters which is used in a filter in the filter library).
> 
> If libpostproc can't be merged into the filter (or nobody bothers to do
> it), then it could be moved to a separate repository, much like the
> nvidia headers.
> 

This is another 'solution', though I think it would just be better to 
redirect users to existing equivalent functionality in lavfi itself (see 
my other mail replying to carl).

>> [...]
>>   enabled threads && ! enabled pthreads && ! enabled atomics_native && die "non pthread threading without atomics not supported, try adding --enable-pthreads or --cpu=i486 or higher if you are on x86"
>>   enabled avresample && warn "Building with deprecated library libavresample"
>> +enabled postproc && warn "Building with deprecated library libpostproc"
>> +enabled pp_filter && warn "Building with deprecated filter pp"
> 
> Superfluous warning, the first one is enough. You don't see a warning
> for resample_filter below the one for libavresample.
> 

Fair, I had it like that initially, was unsure if they should be separate.

>>   
>>   if test $target_os = "haiku"; then
>>       disable memalign
>>
Carl Eugen Hoyos April 26, 2018, 11:38 p.m. UTC | #5
2018-04-27 1:28 GMT+02:00, Josh de Kock <josh@itanimul.li>:
> On 2018/04/27 0:19, Carl Eugen Hoyos wrote:
>> 2018-04-27 1:08 GMT+02:00, Josh de Kock <josh@itanimul.li>:
>>> The postproc library is only used in a single filter
>>
>> Is libswscale used in more filters? Are you planning to
>> deprecate it?
>
> No, libswscale does not suffer from the same issue of being a secondary
> filter library. See for example how libavresample is the secondary
> resampling library, which has been deprecated for being regarded as
> worse than libswscale. libpostproc is just another filtering library
> (which was shoved into lavfi instead of its filters being moved into
> lavfi). Some of the postproc filters seemingly have equivalents in lavfi
> (just going off of names), so I think removal of postproc while making
> sure equivalent filters will be available is the best way to solve this.

I am a little surprised:
Could you explain which filters in libavfilter are the equivalents for the
functionality of libpostproc?

Carl Eugen
Josh Dekker April 26, 2018, 11:57 p.m. UTC | #6
On 2018/04/27 0:38, Carl Eugen Hoyos wrote:
> 2018-04-27 1:28 GMT+02:00, Josh de Kock <josh@itanimul.li>:
>> On 2018/04/27 0:19, Carl Eugen Hoyos wrote:
>>> 2018-04-27 1:08 GMT+02:00, Josh de Kock <josh@itanimul.li>:
>>>> The postproc library is only used in a single filter
>>>
>>> Is libswscale used in more filters? Are you planning to
>>> deprecate it?
>>
>> No, libswscale does not suffer from the same issue of being a secondary
>> filter library. See for example how libavresample is the secondary
>> resampling library, which has been deprecated for being regarded as
>> worse than libswscale. libpostproc is just another filtering library
>> (which was shoved into lavfi instead of its filters being moved into
>> lavfi). Some of the postproc filters seemingly have equivalents in lavfi
>> (just going off of names), so I think removal of postproc while making
>> sure equivalent filters will be available is the best way to solve this.
> 
> I am a little surprised:
> Could you explain which filters in libavfilter are the equivalents for the
> functionality of libpostproc?

deblocking filters -> vf_deblock
deinterlacing filters -> vf_yadif
autolevels -> eq (with some auto-value code)

But as I said, 'some' 'seemingly' 'going off names'. There was just a 
discussion on irc to just use 
http://git.videolan.org/?p=libpostproc.git;a=summary as an external 
library and drop in-tree postproc (this still requires deprecation of 
postproc in-tree however afaik).
Carl Eugen Hoyos April 27, 2018, 12:12 a.m. UTC | #7
2018-04-27 1:57 GMT+02:00, Josh de Kock <josh@itanimul.li>:
> On 2018/04/27 0:38, Carl Eugen Hoyos wrote:

>> Could you explain which filters in libavfilter are the
>> equivalents for the functionality of libpostproc?
>
> deblocking filters -> vf_deblock
> deinterlacing filters -> vf_yadif
> autolevels -> eq (with some auto-value code)

I don't think the filters are equivalent.

> But as I said, 'some' 'seemingly' 'going off names'. There
> was just a discussion on irc to just use
> http://git.videolan.org/?p=libpostproc.git;a=summary as an
> external library

That sounds like a particularly bad idea.

Carl Eugen
Michael Niedermayer April 27, 2018, 12:36 a.m. UTC | #8
On Fri, Apr 27, 2018 at 12:08:16AM +0100, Josh de Kock wrote:
> The postproc library is only used in a single filter, so should be moved into the filter itself if the filter was to stay, but the filter has all of its internal filters now in lavfi itself. (Also it's a bit weird to have a separate library of filters which is used in a filter in the filter library).
> ---
>  configure | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

That patch is a bit surprising.
What prompted you to write this ?

Also, dont you think that before suggesting to deprecate a lib it would make
sense to talk with the author and maintainer of the library in question ?

Either way, the first question to ask would be
"why was/is it a seperate library"
I think, you do not know, 
And you should understand why things are as they are before suggesting to
deprecate, move or otherwise change them


[...]
Michael Niedermayer April 27, 2018, 10:21 a.m. UTC | #9
On Fri, Apr 27, 2018 at 02:36:05AM +0200, Michael Niedermayer wrote:
> On Fri, Apr 27, 2018 at 12:08:16AM +0100, Josh de Kock wrote:
> > The postproc library is only used in a single filter, so should be moved into the filter itself if the filter was to stay, but the filter has all of its internal filters now in lavfi itself. (Also it's a bit weird to have a separate library of filters which is used in a filter in the filter library).
> > ---
> >  configure | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> That patch is a bit surprising.
> What prompted you to write this ?
> 
> Also, dont you think that before suggesting to deprecate a lib it would make
> sense to talk with the author and maintainer of the library in question ?
> 
> Either way, the first question to ask would be
> "why was/is it a seperate library"
> I think, you do not know, 
> And you should understand why things are as they are before suggesting to
> deprecate, move or otherwise change them

Elaborating on this a bit

The first step is to understand the current state of things, why is 
libpostproc where it is. (it seems noone asked the maintainer / author)

after its understood, next is to discuss whats the best thing for the project
Some discussion occured on IRC but people neither understood why its in a
seperate lib currently nor where anyone who maintains or who wrote it involved
in this discussion, making this discussion a bit headless and blind
I also think IRC is not ideal for this discussion as only a small subset
of people will be there when something is discussed ...

Once a consensus is reached what is best to do, that should then be implemented.
We have not reached this point nor has anything that was suggested been
implemented.

The last step then is to deprecate any previous places that are no longer
relevant. So this patch is premature, other steps have to be done first

Thanks


[...]
Paul B Mahol April 27, 2018, 10:39 a.m. UTC | #10
On 4/27/18, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Apr 27, 2018 at 02:36:05AM +0200, Michael Niedermayer wrote:
>> On Fri, Apr 27, 2018 at 12:08:16AM +0100, Josh de Kock wrote:
>> > The postproc library is only used in a single filter, so should be moved
>> > into the filter itself if the filter was to stay, but the filter has all
>> > of its internal filters now in lavfi itself. (Also it's a bit weird to
>> > have a separate library of filters which is used in a filter in the
>> > filter library).
>> > ---
>> >  configure | 6 ++++--
>> >  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> That patch is a bit surprising.
>> What prompted you to write this ?
>>
>> Also, dont you think that before suggesting to deprecate a lib it would
>> make
>> sense to talk with the author and maintainer of the library in question ?
>>
>> Either way, the first question to ask would be
>> "why was/is it a seperate library"
>> I think, you do not know,
>> And you should understand why things are as they are before suggesting to
>> deprecate, move or otherwise change them
>
> Elaborating on this a bit
>
> The first step is to understand the current state of things, why is
> libpostproc where it is. (it seems noone asked the maintainer / author)
>
> after its understood, next is to discuss whats the best thing for the
> project
> Some discussion occured on IRC but people neither understood why its in a
> seperate lib currently nor where anyone who maintains or who wrote it
> involved
> in this discussion, making this discussion a bit headless and blind
> I also think IRC is not ideal for this discussion as only a small subset
> of people will be there when something is discussed ...
>
> Once a consensus is reached what is best to do, that should then be
> implemented.
> We have not reached this point nor has anything that was suggested been
> implemented.
>
> The last step then is to deprecate any previous places that are no longer
> relevant. So this patch is premature, other steps have to be done first

Libpostproc was removed from Libav, you failed somehow merge that change?


They moved it to abandoned repo.


We should not follow that.


Libpostproc is very small in size and trivial GPL code and all funcionality
should be moved to pp filter in libavfilter.

No point to have separate library nobody really uses.
Nicolas George April 27, 2018, 12:11 p.m. UTC | #11
Paul B Mahol (2018-04-27):
> Libpostproc was removed from Libav, you failed somehow merge that change?

Many useful things were removed from libav.

I find somewhat disturbing the amount of time and effort that some
people (not you) spend trying to remove harmless code instead of
producing useful new code.

Regards,
Paul B Mahol April 27, 2018, 12:28 p.m. UTC | #12
On 4/27/18, Nicolas George <george@nsup.org> wrote:
> Paul B Mahol (2018-04-27):
>> Libpostproc was removed from Libav, you failed somehow merge that change?
>
> Many useful things were removed from libav.
>
> I find somewhat disturbing the amount of time and effort that some
> people (not you) spend trying to remove harmless code instead of
> producing useful new code.

Perhaps you forgot, but I removed libmpcodecs + wrapper.
Nicolas George April 27, 2018, 12:29 p.m. UTC | #13
Paul B Mahol (2018-04-27):
> Perhaps you forgot, but I removed libmpcodecs + wrapper.

Yes, but you also spend time writing (probably) useful code.

Regards,
Jean-Baptiste Kempf April 27, 2018, 12:40 p.m. UTC | #14
Hello,

On Fri, 27 Apr 2018, at 12:39, Paul B Mahol wrote:
> No point to have separate library nobody really uses.

VLC uses libprostproc, but we see many crashes in it, and the usefulness is not very obvious/visible depending on the codec.

A general library/filter to "smooth" would be very very welcome.

Thx.
Paul B Mahol April 27, 2018, 12:43 p.m. UTC | #15
On 4/27/18, Jean-Baptiste Kempf <jb@videolan.org> wrote:
> Hello,
>
> On Fri, 27 Apr 2018, at 12:39, Paul B Mahol wrote:
>> No point to have separate library nobody really uses.
>
> VLC uses libprostproc, but we see many crashes in it, and the usefulness is
> not very obvious/visible depending on the codec.

Crashes? Have you reported such cases?

>
> A general library/filter to "smooth" would be very very welcome.

There are numerous filters in lavfi that "smooth" stuff.

>
> Thx.
Carl Eugen Hoyos April 27, 2018, 1:54 p.m. UTC | #16
2018-04-27 14:40 GMT+02:00, Jean-Baptiste Kempf <jb@videolan.org>:
> Hello,
>
> On Fri, 27 Apr 2018, at 12:39, Paul B Mahol wrote:
>> No point to have separate library nobody really uses.
>
> VLC uses libprostproc, but we see many crashes in it

Please point us to the bug reports.

Thank you, Carl Eugen
Michael Niedermayer April 27, 2018, 3:20 p.m. UTC | #17
On Fri, Apr 27, 2018 at 12:39:13PM +0200, Paul B Mahol wrote:
> On 4/27/18, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Apr 27, 2018 at 02:36:05AM +0200, Michael Niedermayer wrote:
> >> On Fri, Apr 27, 2018 at 12:08:16AM +0100, Josh de Kock wrote:
> >> > The postproc library is only used in a single filter, so should be moved
> >> > into the filter itself if the filter was to stay, but the filter has all
> >> > of its internal filters now in lavfi itself. (Also it's a bit weird to
> >> > have a separate library of filters which is used in a filter in the
> >> > filter library).
> >> > ---
> >> >  configure | 6 ++++--
> >> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> That patch is a bit surprising.
> >> What prompted you to write this ?
> >>
> >> Also, dont you think that before suggesting to deprecate a lib it would
> >> make
> >> sense to talk with the author and maintainer of the library in question ?
> >>
> >> Either way, the first question to ask would be
> >> "why was/is it a seperate library"
> >> I think, you do not know,
> >> And you should understand why things are as they are before suggesting to
> >> deprecate, move or otherwise change them
> >
> > Elaborating on this a bit
> >
> > The first step is to understand the current state of things, why is
> > libpostproc where it is. (it seems noone asked the maintainer / author)
> >
> > after its understood, next is to discuss whats the best thing for the
> > project
> > Some discussion occured on IRC but people neither understood why its in a
> > seperate lib currently nor where anyone who maintains or who wrote it
> > involved
> > in this discussion, making this discussion a bit headless and blind
> > I also think IRC is not ideal for this discussion as only a small subset
> > of people will be there when something is discussed ...
> >
> > Once a consensus is reached what is best to do, that should then be
> > implemented.
> > We have not reached this point nor has anything that was suggested been
> > implemented.
> >
> > The last step then is to deprecate any previous places that are no longer
> > relevant. So this patch is premature, other steps have to be done first
> 
> Libpostproc was removed from Libav, you failed somehow merge that change?
> 
> 
> They moved it to abandoned repo.
> 
> 
> We should not follow that.
> 
> 

> Libpostproc is very small in size and trivial GPL code and all funcionality
> should be moved to pp filter in libavfilter.
> 
> No point to have separate library nobody really uses.

Well, thats one direction it could go.
But still noone asks why its a seperate lib ?

Its long ago and there were multiple reasons, the one still relevant today
is that lossy decompressed moving pictures and images is very widespread.
And postprocessing that often improves its subjective and
objective quality

So there are a lot of applications and libraries which would benefit
from using "postprocessing".

And libavfilter is quite big, a small lib that is just for postprocessing
lossy decompressed video would fit this usecase better.

I think a big problem we had was that alot of software which would benefit from
this code isnt aware of it.
But i may be wrong, libpostproc was never "advertised" by anyone as being
usefull to improve the quality of low bitrate videos, and jpegs and other
similar lossy compression schemes.

I dont know what is the best way forward but simply moving it into
a filter in libavfilter alone seems to make it much harder to use for
many outside ffmpeg.

Of course that can be a great idea together with making libavfilter more
modular. If for example an application which wants to use just one filter
from libavfilter could do this without pulling anything else in not even
most of the core and just use that one filter then moving postproc
into a filter in libavfilter would be possibly even better than having it
in a seperate lib (which is less modular as it would contain multiple
related filters)

Its really a question what people prefer what makes most sense and what 
has volunteers to do the actual work.


[...]
diff mbox

Patch

diff --git a/configure b/configure
index 9fa1665496..1e00c3e7a4 100755
--- a/configure
+++ b/configure
@@ -130,7 +130,7 @@  Component options:
   --disable-avformat       disable libavformat build
   --disable-swresample     disable libswresample build
   --disable-swscale        disable libswscale build
-  --disable-postproc       disable libpostproc build
+  --enable-postproc        disable libpostproc build (deprecated) [no]
   --disable-avfilter       disable libavfilter build
   --enable-avresample      enable libavresample build (deprecated) [no]
   --disable-pthreads       disable pthreads [autodetect]
@@ -3529,7 +3529,7 @@  intrinsics="none"
 enable $PROGRAM_LIST
 enable $DOCUMENT_LIST
 enable $EXAMPLE_LIST
-enable $(filter_out avresample $LIBRARY_LIST)
+enable $(filter_out postproc $(filter_out avresample $LIBRARY_LIST))
 enable stripping
 
 enable asm
@@ -6674,6 +6674,8 @@  check_deps $CONFIG_LIST       \
 
 enabled threads && ! enabled pthreads && ! enabled atomics_native && die "non pthread threading without atomics not supported, try adding --enable-pthreads or --cpu=i486 or higher if you are on x86"
 enabled avresample && warn "Building with deprecated library libavresample"
+enabled postproc && warn "Building with deprecated library libpostproc"
+enabled pp_filter && warn "Building with deprecated filter pp"
 
 if test $target_os = "haiku"; then
     disable memalign