[FFmpeg-cvslog] r25165 - in trunk: Changelog configure doc/filters.texi libavfilter/Makefile libavfilter/allfilters.c libavfilter/avfilter.h libavfilter/vf_frei0r.c

[Stefano Sabatini]

On date Friday 2010-09-24 10:04:20 +0100, M?ns Rullg?rd wrote:
> Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
> 
> > On date Friday 2010-09-24 08:13:51 +0100, M?ns Rullg?rd wrote:
> >> stefano <subversion at mplayerhq.hu> writes:
> >> 
> >> > Author: stefano
> >> > Date: Fri Sep 24 02:32:22 2010
> >> > New Revision: 25165
> >> >
> >> > Log:
> >> > Add frei0r filter.
> >> >
> >> > See thread:
> >> > Subject: [FFmpeg-devel] [POC] frei0r wrapper
> >> > Date: Tue, 24 Aug 2010 21:37:32 +0200
> >> >
> >> > Added:
> >> >    trunk/libavfilter/vf_frei0r.c
> >> > Modified:
> >> >    trunk/Changelog
> >> >    trunk/configure
> >> >    trunk/doc/filters.texi
> >> >    trunk/libavfilter/Makefile
> >> >    trunk/libavfilter/allfilters.c
> >> >    trunk/libavfilter/avfilter.h
> >> >
> >> > Modified: trunk/Changelog
> >> > ==============================================================================
> >> > --- trunk/Changelog	Thu Sep 23 23:33:29 2010	(r25164)
> >> > +++ trunk/Changelog	Fri Sep 24 02:32:22 2010	(r25165)
> >> > @@ -36,6 +36,7 @@ version <next>:
> >> >  - G.722 ADPCM audio encoder/decoder
> >> >  - R10k video decoder
> >> >  - ocv_smooth filter
> >> > +- frei0r wrapper filter
> >> >
> >> >  version 0.6:
> >> >
> >> > Modified: trunk/configure
> >> > ==============================================================================
> >> > --- trunk/configure	Thu Sep 23 23:33:29 2010	(r25164)
> >> > +++ trunk/configure	Fri Sep 24 02:32:22 2010	(r25165)
> >> > @@ -162,6 +162,7 @@ Configuration options:
> >> >  External library support:
> >> >    --enable-avisynth        enable reading of AVISynth script files [no]
> >> >    --enable-bzlib           enable bzlib [autodetect]
> >> > +  --enable-frei0r          enable frei0r video filtering
> >> >    --enable-libopencore-amrnb enable AMR-NB de/encoding via libopencore-amrnb [no]
> >> >    --enable-libopencore-amrwb enable AMR-WB decoding via libopencore-amrwb [no]
> >> >    --enable-libopencv       enable video filtering via libopencv [no]
> >> > @@ -873,6 +874,7 @@ CONFIG_LIST="
> >> >      ffprobe
> >> >      ffserver
> >> >      fft
> >> > +    frei0r
> >> >      golomb
> >> >      gpl
> >> >      gray
> >> > @@ -1050,6 +1052,7 @@ HAVE_LIST="
> >> >      poll_h
> >> >      setrlimit
> >> >      strerror_r
> >> > +    strtok_r
> >> >      struct_addrinfo
> >> >      struct_ipv6_mreq
> >> >      struct_sockaddr_in6
> >> 
> >> I told you not to check for strtok_r.  It exists on all POSIX systems,
> >> and frei0r doesn't work anywhere else (dlopen).  Please remove this
> >> pointless check, and also please respect the review comments in future.
> >
> > POSIX => dlopen
> > but not:
> > dlopen => POSIX
> >
> > so if dlopen is present you can't infer that strtok_r exists.
> >
> > And note that we already have a check for strerror_r, which is
> > supported by POSIX but is not supported in MinGW, following the same
> > logic it makes sense to keep the check for strtok_r.
> 
> We check for strerror_r because it causes an actual build failure if
> we do not.  That is not the case for strtok_r.  Gratuitous checks only
> make configure run slower and bloat the build system for no gain.
> 
> For just about any function we use, no matter how standard, there
> surely exists some system, somewhere that doesn't have it.

Check attached.
 
[...]
> >> You have changed this substantially from what was reviewed, and not in
> >> the way I requested.  What is the point of reviews if you are going to
> >> ignore them?
> >
> > Basically I rewrote it to be more strict with the frei0r specs.
> 
> Without bothering to respond to my comments, and without posting it
> for review.

I believed I addressed to your comments with that change, and the
patch was posted since more than a week (09-13).

Regards.
-- 
FFmpeg = Faithless and Friendly Mysterious Prodigious Elastic Ghost