Quoting Diego Biurrun <[email protected]>:
From 9fde43d493e6ce3216dd4ff779a8440fb647d8db Mon Sep 17 00:00:00 2001
From: Peter B. <[email protected]>
Date: Wed, 6 Nov 2013 14:50:53 +0100
Subject: [PATCH] - Improved FFV1 fate test speed. - Added dependencies:
decoding requires encoding before - Added dependency: pass2
requires pass1 - Added dependency: vsynth1.yuv must be
generated - Added conditionals to work with multiple
targets (ffmpeg,avconv)
This list of changes belongs in an annotation, not in the log message.
I still prefer the original log message I suggested :)
I'm not really familiar with auto-generated patches.
The Subject is auto-generated by "git format-patch" and is basically
my commit message.
I did change the mail-subject to your suggested log-message, but
wasn't aware that you also meant the message inside the patch.
Thanks for clearing that up.
--- /dev/null
+++ b/tests/fate/ffv1.mak
@@ -0,0 +1,313 @@
+# This Makefile checks for $(CONFIG_...) variables being set, so we can
+# include/exclude tests accordingly:
+ifdef CONFIG_AVCONV
+FLAGS_FFV1_V3 = -strict experimental
+else
+FLAGS_FFV1_V3 =
+endif
trailing whitespace
Oops.
Overlooked. Sorry.
This ifdef is redundant, avconv is required to run these tests in the
first place.
Well, yes and no:
In order to make it easier for me to maintain this FFV1 fate-testset,
I've written the Makefile in a way that the same file will build the
tests properly for both: FFmpeg and Libav.
So, I've used the "CONFIG_AVCONV" and "CONFIG_FFMPEG" variables as
indicator for which target system the tests are being ran.
That also explains other references to FFmpeg in that Makefile.
Just limit the number of frames directly. Ideally by cutting the samples
to 4 frames.
Hm...
I actually wanted to use vsynth1.yuv "as-is", but actually it's a good idea:
It would also enable to generate other pix_fmt and size versions out
of the vsynth source.
Where would be a good location to store such intermediate files?
In tests/data/fate somewhere?
+FATE_FFV1_LEVEL1 = v1-defaults \
+ v1-gray \
+ v1-rgb32 \
+ v1-yuv410p \
+ v1-yuv411p \
+ v1-yuv420p \
+ v1-yuv422p \
+ v1-yuv444p \
+ v1-bgra \
+ v1-tff \
+ v1-bff
nit: You could align the '\'.
Like this?
+FATE_FFV1_LEVEL1 = v1-defaults \
+ v1-gray \
+ v1-rgb32 \
+###################################################
+# Decoding:
+###################################################
+# YUV (8bit)
+fate-ffv1-dec-v1-defaults: ${CMD = framecrc -i
$(DEC_SRC)/ffv1-enc-v1-defaults.avi} fate-ffv1-enc-v1-defaults
This works as expected?
Actually yes. Why?
Also, use $() syntax instead of ${}.
Roger that.
Thought it might be nice to have some difference between Makefile
variable-access use and actual Makefile-command sets.
It's purely cosmetic though, so I'll change it to whatever your custom is.
+###################################################
+# Encoding:
+###################################################
Some of these options could be factored out into variables.
You mean making variables to for-loop over?
This is my first Makefile and my first contact with FATE tests, so I
tried to keep it as straightforward as possible.
+# Requires generating vsynth1.yuv as input source:
+$(FATE_FFV1-yes): tests/data/vsynth1.yuv
pointless comment
I agree and disagree.
When I started to understand "tests/vcodec.mak", I didn't consider
this line necessary for ffv1.mak.
Only to realize days later, that "vsynth1.yuv" generating is triggered
if executing vcodec.mak - and so the ffv1 tests here "Requires
generating vsynth1.yuv as input source".
If there were some more of these kind of "pointless comments", I'd
wouldn't have had to spend several days reverse-engineering the "what
is where and why?" of FATE testing.
I am technically skilled, but not too familiar with Makefiles or C-code.
Comments might make the life of non-C-guru-contributors way easier.
No ranting, just my personal experience.
diff --git a/tests/ref/fate/ffv1-dec-v1-bff b/tests/ref/fate/ffv1-dec-v1-bff
new file mode 100644
index 0000000..e69de29
diff --git a/tests/ref/fate/ffv1-dec-v1-bgra
b/tests/ref/fate/ffv1-dec-v1-bgra
new file mode 100644
index 0000000..e69de29
diff --git a/tests/ref/fate/ffv1-dec-v1-defaults
b/tests/ref/fate/ffv1-dec-v1-defaults
new file mode 100644
index 0000000..e69de29
Empty files?
Hm...
Will check.
Thanks for your feedback!
Will incorporate the changes as soon as possible.
Regards,
Pb
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
