[FFmpeg-devel] [PATCH 2/5] fate: add concat demuxer tests

[Marton Balint]

On Sun, 25 Oct 2015, Nicolas George wrote:

[...]

>> --- a/tests/fate-run.sh
>> +++ b/tests/fate-run.sh
>> @@ -249,6 +249,20 @@ gapless(){
>>      do_md5sum $decfile3
>>  }
>>
>> +concat(){
>
>> +    template=$(target_path $1)
>
> Should it really be target_path? The template is in the source.

You are right, target path is unneeded here.

>> +    sample=$(target_path $2)
>> +
>> +    concatfile="${outdir}/${test}.ffconcat"
>> +    packetfile="${outdir}/${test}.ffprobe"
>> +    cleanfiles="$concatfile $packetfile"
>> +
>> +    awk "{gsub(/%SRCFILE%/, \"$sample\"); print}" $template > $concatfile
>> +    run ffprobe${PROGSUF} -show_streams -show_packets -v 0 -fflags keepside -f concat $concatfile > $packetfile
>> +
>
>> +    do_md5sum $packetfile
>
> Since the output files are text, I would prefer it being the reference as
> is, without a hash. With a hash, you need two extra steps to know what you
> just broke.

I used the md5 hash because the packet files are quite big - around 
3000-10000 lines each. It seemed like a waste of space in the repository 
for relatively little benefit.

>
>> +}
>> +
>>  mkdir -p "$outdir"
>>
>>  # Disable globbing: command arguments may contain globbing characters and
>> diff --git a/tests/fate/concatdec.mak b/tests/fate/concatdec.mak
>> new file mode 100644
>> index 0000000..89d5409
>> --- /dev/null
>> +++ b/tests/fate/concatdec.mak
>> @@ -0,0 +1,12 @@
>> +FATE_CONCAT_TEMPLATE=tests/test_template.ffconcat
>> +
>> +FATE_CONCAT_DEMUXER_LAVF-$(call ENCDEC2, MPEG2VIDEO, MP2, MPEGTS)    += ts
>> +FATE_CONCAT_DEMUXER_LAVF-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += mxf
>> +FATE_CONCAT_DEMUXER_LAVF-$(call ENCDEC2, MPEG2VIDEO, PCM_S16LE, MXF) += mxf_d10
>> +
>> +$(foreach D,$(FATE_CONCAT_DEMUXER_LAVF-yes),$(eval fate-concat-demuxer-lavf-$(D): ffprobe$(PROGSSUF)$(EXESUF) fate-lavf-$(D)))
>> +$(foreach D,$(FATE_CONCAT_DEMUXER_LAVF-yes),$(eval fate-concat-demuxer-lavf-$(D): CMD = concat $(FATE_CONCAT_TEMPLATE) tests/data/lavf/lavf.$(D)))
>> +
>> +FATE_CONCAT_DEMUXER-$(CONFIG_CONCAT_DEMUXER) += $(FATE_CONCAT_DEMUXER_LAVF-yes:%=fate-concat-demuxer-lavf-%)
>> +
>> +FATE-$(CONFIG_FFPROBE) += $(FATE_CONCAT_DEMUXER-yes)
>
> I am not fluent enough in make, I will trust you on this.

I am not very fluent either, I only hope I've made it right...

[...]

>> diff --git a/tests/test_template.ffconcat b/tests/test_template.ffconcat
>> new file mode 100644
>> index 0000000..e9b685d
>> --- /dev/null
>> +++ b/tests/test_template.ffconcat
>> @@ -0,0 +1,112 @@
>
>> +#ffconcat version 1.0
>> +# ^ header is commented out to avoid probing therefore enable unsafe paths
>
> Probably better to pass "-safe 0" explicitly.

OK.

>> +
>> +file      %SRCFILE%
>> +
>> +file      %SRCFILE%
>> +file_packet_metadata dummy=1
>> +duration  1
>> +
>
>> +file      %SRCFILE%
>> +inpoint   00:00.00
>> +outpoint  00:00.04
>> +
>> +file      %SRCFILE%
>> +inpoint   00:00.04
>> +outpoint  00:00.08
> <snip>
>
> Does it need that many in/outpoints?

I used this many (25 1-frame segment) because this is also a seek test for 
open gop mxf. I can reduce it but in that case I will have to maintain my 
own out of tree tests for it which is counter-productive IMHO.

Let me know what you think, I can change any point you still find 
unjustified.

Thanks,
Marton