[FFmpeg-devel] [PATCH] Add tests for functions in hash.c
nagachaitanya.vellanki at gmail.com
Thu Mar 10 16:08:39 CET 2016
Will send in a new patch with the improvements for review :-).
On Wed, Mar 9, 2016 at 11:05 AM, Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> On Wed, Mar 09, 2016 at 10:27:29AM -0800, NagaChaitanya Vellanki wrote:
> > On Tue, Mar 8, 2016 at 5:33 PM, James Almer <jamrial at 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
> > > 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 at ffmpeg.org
More information about the ffmpeg-devel