[FFmpeg-devel] [PATCH] lavfi test for 1-1 filters pixel format output

[Måns Rullgård]

Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:

> On date Monday 2010-06-14 18:50:30 +0200, Michael Niedermayer encoded:
>> On Mon, Jun 14, 2010 at 02:04:49PM +0100, M?ns Rullg?rd wrote:
>> > Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
>> > 
>> > > On date Monday 2010-06-14 12:28:51 +0100, M?ns Rullg?rd encoded:
>> > >> Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
>> > >> 
>> > >> > On date Sunday 2010-06-06 01:30:16 +0200, Stefano Sabatini encoded:
>> > >> >> On date Wednesday 2010-06-02 23:42:32 +0200, Stefano Sabatini encoded:
>> > >> >> > On date Monday 2010-05-24 22:12:25 +0100, M?ns Rullg?rd encoded:
>> > >> >> > > Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:
>> > >> >> > [...]
>> > >> >> > > >> Did I review this?  Of course not, or I would have told you it breaks
>> > >> >> > > >> cross-builds (only if the test is run of course).
>> > >> >> > > >
>> > >> >> > > > Michael asked to commit, I did since the test is disabled so people
>> > >> >> > > > can start to work out the lavfi bugs.
>> > >> >> > > 
>> > >> >> > > He also asked me to review it.
>> > >> >> > > 
>> > >> >> > > > Feel free to fix it yourself or point out the problem it has.
>> > >> >> > > 
>> > >> >> > > I'll get to it eventually, but the problem is that the executable you
>> > >> >> > > build has to be run with $TARGET_EXEC and $TARGET_PATH.
>> > >> >> > 
>> > >> >> > Uhm, if that is true why tiny_psnr in regression-funcs.sh is not
>> > >> >> > defined like that.
>> > >> >> > 
>> > >> >> > Anyway my main problem seems that the lavfi-showfiltfmts I'm using
>> > >> >> > needs to include and link against the libav* libraries, in this being
>> > >> >> > different from the other testprogs, so it is failing to compile and I
>> > >> >> > don't even know if this crossbuilding thing is even able to support
>> > >> >> > that requirement.
>> > >> >> > 
>> > >> >> > Your help is welcome, regards.
>> > >> >> 
>> > >> >> M?ns-ping.
>> > >> >
>> > >> > I'll apply this patch or a variant in three days if I get no reply.
>> > >> 
>> > >> You will do no such thing.  The patch is WRONG.
>> > >
>> > > I know that,
>> > 
>> > Then this discussion is pointless.
>> 
>> So how exactly will this be resolved?
>> Can you elaborate on what is wrong?
>> Can you document how the cross compilation system you designed works
>> so other people than you can modify the scripts without breaking it?
>> Do we have some volunteer who is less busy and who could help maintain
>> the scripts in ffmpeg?
>
> Patch updated, don't know if this is the correct solution, anyway it
> works for me. It still misses the BE tests, I don't have such a
> machine so I need either someone to try them either an an account on
> such a machine.

You can have a login on my PPC if you like.  Send me an ssh key and
desired username.

> I'm still waiting for a useful reply from Mans, what's sure is that
> I'm not going to wait forever (note that this thread started in
> March).

I already told you to fix it for non-cross builds.  I'll do the rest
once it works locally.

> diff --git a/Makefile b/Makefile
> index 4b945f4..67a7a02 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -14,7 +14,7 @@ PROGS_G     = $(addsuffix _g$(EXESUF), $(PROGS-yes))
>  OBJS        = $(addsuffix .o,          $(PROGS-yes)) cmdutils.o
>  MANPAGES    = $(addprefix doc/, $(addsuffix .1, $(PROGS-yes)))
>  TOOLS       = $(addprefix tools/, $(addsuffix $(EXESUF), cws2fws pktdumper probetest qt-faststart trasher))
> -HOSTPROGS   = $(addprefix tests/, audiogen videogen rotozoom tiny_psnr)
> +HOSTPROGS   = $(addprefix tests/, audiogen videogen rotozoom tiny_psnr lavfi-showfiltfmts)

This can never work.  lavfi-showfiltfmts has to link against lavfi,
which is built for the target, not the host.

>  $(HOSTOBJS): %.o: %.c
> -	$(HOSTCC) $(HOSTCFLAGS) -c -o $@ $<
> +	$(HOSTCC) $(CPPFLAGS) $(HOSTCFLAGS) -c -o $@ $<

Using CPPFLAGS with HOSTCC is a very bad idea.

>  $(HOSTPROGS): %$(HOSTEXESUF): %.o
> -	$(HOSTCC) $(HOSTLDFLAGS) -o $@ $< $(HOSTLIBS)
> +	$(HOSTCC) $(FF_LDFLAGS) $(HOSTLDFLAGS) -o $@ $< $(FF_EXTRALIBS) $(HOSTLIBS)

Likewise for LDFLAGS.

>  depend dep: $(DEPS)
> diff --git a/tests/lavfi-regression.sh b/tests/lavfi-regression.sh
> index ebbc0f3..8dc92b0 100755
> --- a/tests/lavfi-regression.sh
> +++ b/tests/lavfi-regression.sh
> @@ -67,20 +67,26 @@ scale=200:100
>  vflip
>  "
>  
> +showfiltfmts="$target_exec ${target_path}/tests/lavfi-showfiltfmts"

Now you're trying to run a host executable on the target.  This
command is probably correct if you remove the brokenness above.

>  if [ -n "$do_lavfi_pix_fmts" ]; then
>      # exclude pixel formats which are not supported as input
>      excluded_pix_fmts="$(ffmpeg -pix_fmts list 2>/dev/null | sed -ne '9,$p' | grep '^\..\.' | cut -d' ' -f2)"
>  
> -    scale_out_pix_fmts=$(tools/lavfi-showfiltfmts scale | grep "^OUTPUT" | cut -d: -f2)
> +    scale_out_pix_fmts=$($showfiltfmts scale | grep "^OUTPUT" | cut -d: -f2)
>      scale_out_pix_fmts=$(get_exclusive_elements "$scale_out_pix_fmts" "$excluded_pix_fmts")
>  
> +    ref_file=tests/ref/lavfi/lavfi_pix_fmts
> +    rm -f $ref_file

This looks unrelated to cross-builds.

>      for filter_args in $filters_args; do
>          filter=$(echo $filter_args | sed -e 's/\([^=]\+\)=.*/\1/')
> -        in_pix_fmts=$(tools/lavfi-showfiltfmts $filter | grep "^INPUT" | cut -d: -f2)
> +        in_pix_fmts=$($showfiltfmts $filter | grep "^INPUT" | cut -d: -f2)
>          pix_fmts=$(get_common_elements "$in_pix_fmts" "$scale_out_pix_fmts")
>  
>          for pix_fmt in $pix_fmts; do
> -            do_video_encoding "${pix_fmt}-${filter}.nut" "" \
> +            cat $ref_file-${filter}-$pix_fmt >> $ref_file
> +            do_video_encoding "lavfi_pix_fmts-${filter}-${pix_fmt}.nut" "" \

Ditto.

>                  "-vf slicify=random,format=$pix_fmt,$filter_args -vcodec rawvideo -pix_fmt $pix_fmt"
>          done
>      done
> diff --git a/tools/lavfi-showfiltfmts.c b/tests/lavfi-showfiltfmts.c
> similarity index 100%
> rename from tools/lavfi-showfiltfmts.c
> rename to tests/lavfi-showfiltfmts.c

Why?

> diff --git a/tests/ref/lavfi/lavfi_pix_fmts-crop-abgr b/tests/ref/lavfi/lavfi_pix_fmts-crop-abgr
> new file mode 100644
> index 0000000..ec6fc9a

Adding the ref files surely has nothing to do with cross-builds.

Now _PLEASE_ commit the non-cross bits as I've asked you repeatedly
and stop trying to claim I'm holding you back.

-- 
M?ns Rullg?rd
mans at mansr.com