On Wed, Mar 09, 2016 at 10:27:29AM -0800, NagaChaitanya Vellanki wrote: > On Tue, Mar 8, 2016 at 5:33 PM, James Almer <jamr...@gmail.com> wrote: > > > On 3/8/2016 2:21 AM, NagaChaitanya Vellanki wrote: > > > --- > > > libavutil/Makefile | 1 + > > > libavutil/hash.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > > tests/fate/libavutil.mak | 4 ++++ > > > tests/ref/fate/hash | 45 > > +++++++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 92 insertions(+) > > > create mode 100644 tests/ref/fate/hash > > > > LGTM. I'll let the maintainer give the final ok and push it (CCing him as > > well).
It looks fine to me, though there are a few things that could be improved if wanted. For example all-0s for src is a bit boring, just a couple of random numbers would be a better test. At 64 bytes it is also a bit short, for some hashes that would mean the same size as the hash, and thus some parts of the algorithm would never ever run. A power-of-two size also means the padding special-cases are not tested. Something like e.g. 133 bytes might be a better test (assuming all our hashes support that as input? I actually have no idea who is responsible for padding). As we are testing things, changing the memset to not 0 but e.g. 'a' would ensure that we would notice if someone broke then 0-termination of the string, and it should then also be run before the av_hash_final_b64. But regardless of that it is great to have a test, these are just some improvement ideas that also can be added later. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
