Hi Reimar, Will send in a new patch with the improvements for review :-). Thank you, Naga
On Wed, Mar 9, 2016 at 11:05 AM, Reimar Döffinger <reimar.doeffin...@gmx.de> wrote: > 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 > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
