diff mbox

[FFmpeg-devel] avutil/tests: Improved code coverage for random_seed

Message ID 1482451956-3947-1-git-send-email-thomastdt@googlemail.com
State Accepted
Commit 8dcb28cf6dd1c68810e7aa857bb6f2a778bef4de
Headers show

Commit Message

Thomas Turner Dec. 23, 2016, 12:12 a.m. UTC
Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
---
 libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
 tests/ref/fate/random_seed    |  1 +
 2 files changed, 22 insertions(+), 13 deletions(-)

Comments

Michael Niedermayer Dec. 23, 2016, 1:55 a.m. UTC | #1
On Thu, Dec 22, 2016 at 04:12:36PM -0800, Thomas Turner wrote:
> Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
> ---
>  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
>  tests/ref/fate/random_seed    |  1 +
>  2 files changed, 22 insertions(+), 13 deletions(-)

patch applied

thx

[...]
James Almer Dec. 24, 2016, 3:30 a.m. UTC | #2
On 12/22/2016 9:12 PM, Thomas Turner wrote:
> Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
> ---
>  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
>  tests/ref/fate/random_seed    |  1 +
>  2 files changed, 22 insertions(+), 13 deletions(-)
> 
> diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
> index ebe9b3e..fcd68bc 100644
> --- a/libavutil/tests/random_seed.c
> +++ b/libavutil/tests/random_seed.c
> @@ -23,24 +23,32 @@
>  
>  #undef printf
>  #define N 256
> +#define F 2
>  #include <stdio.h>
>  
> +typedef uint32_t (*random_seed_ptr_t)(void);
> +
>  int main(void)
>  {
> -    int i, j, retry;
> +    int i, j, rsf, retry;
>      uint32_t seeds[N];
> +    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
>  
> -    for (retry=0; retry<3; retry++){
> -        for (i=0; i<N; i++){
> -            seeds[i] = av_get_random_seed();
> -            for (j=0; j<i; j++)
> -                if (seeds[j] == seeds[i])
> -                    goto retry;
> +    for (rsf=0; rsf<F; ++rsf){
> +        for (retry=0; retry<3; retry++){
> +            for (i=0; i<N; i++){
> +                seeds[i] = random_seed[rsf]();
> +                for (j=0; j<i; j++)
> +                    if (seeds[j] == seeds[i])
> +                        goto retry;
> +            }
> +            printf("seeds OK\n");
> +            goto next;
> +            retry:;
>          }
> -        printf("seeds OK\n");
> -        return 0;
> -        retry:;
> +        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
> +        return 1;
> +        next:;
>      }
> -    printf("FAIL at %d with %X\n", j, seeds[j]);
> -    return 1;
> -}
> +    return 0;
> + }
> \ No newline at end of file
> diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
> index 2b5b3af..ef0eef2 100644
> --- a/tests/ref/fate/random_seed
> +++ b/tests/ref/fate/random_seed
> @@ -1 +1,2 @@
>  seeds OK
> +seeds OK

This is making the test run for an absurd amount of time, to the point
FATE clients are killing the process because it was just not stopping.

Before this it would take a few milliseconds.
Michael Niedermayer Dec. 24, 2016, 1:48 p.m. UTC | #3
On Sat, Dec 24, 2016 at 12:30:06AM -0300, James Almer wrote:
> On 12/22/2016 9:12 PM, Thomas Turner wrote:
> > Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
> > ---
> >  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
> >  tests/ref/fate/random_seed    |  1 +
> >  2 files changed, 22 insertions(+), 13 deletions(-)
> > 
> > diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
> > index ebe9b3e..fcd68bc 100644
> > --- a/libavutil/tests/random_seed.c
> > +++ b/libavutil/tests/random_seed.c
> > @@ -23,24 +23,32 @@
> >  
> >  #undef printf
> >  #define N 256
> > +#define F 2
> >  #include <stdio.h>
> >  
> > +typedef uint32_t (*random_seed_ptr_t)(void);
> > +
> >  int main(void)
> >  {
> > -    int i, j, retry;
> > +    int i, j, rsf, retry;
> >      uint32_t seeds[N];
> > +    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
> >  
> > -    for (retry=0; retry<3; retry++){
> > -        for (i=0; i<N; i++){
> > -            seeds[i] = av_get_random_seed();
> > -            for (j=0; j<i; j++)
> > -                if (seeds[j] == seeds[i])
> > -                    goto retry;
> > +    for (rsf=0; rsf<F; ++rsf){
> > +        for (retry=0; retry<3; retry++){
> > +            for (i=0; i<N; i++){
> > +                seeds[i] = random_seed[rsf]();
> > +                for (j=0; j<i; j++)
> > +                    if (seeds[j] == seeds[i])
> > +                        goto retry;
> > +            }
> > +            printf("seeds OK\n");
> > +            goto next;
> > +            retry:;
> >          }
> > -        printf("seeds OK\n");
> > -        return 0;
> > -        retry:;
> > +        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
> > +        return 1;
> > +        next:;
> >      }
> > -    printf("FAIL at %d with %X\n", j, seeds[j]);
> > -    return 1;
> > -}
> > +    return 0;
> > + }
> > \ No newline at end of file
> > diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
> > index 2b5b3af..ef0eef2 100644
> > --- a/tests/ref/fate/random_seed
> > +++ b/tests/ref/fate/random_seed
> > @@ -1 +1,2 @@
> >  seeds OK
> > +seeds OK
> 
> This is making the test run for an absurd amount of time, to the point
> FATE clients are killing the process because it was just not stopping.

this is more a problem of get_generic_seed() and less the test.

should be fixed


> 
> Before this it would take a few milliseconds.
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Hendrik Leppkes Dec. 28, 2016, 6:08 p.m. UTC | #4
On Fri, Dec 23, 2016 at 1:12 AM, Thomas Turner <thomastdt@googlemail.com> wrote:
> Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
> ---
>  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
>  tests/ref/fate/random_seed    |  1 +
>  2 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
> index ebe9b3e..fcd68bc 100644
> --- a/libavutil/tests/random_seed.c
> +++ b/libavutil/tests/random_seed.c
> @@ -23,24 +23,32 @@
>
>  #undef printf
>  #define N 256
> +#define F 2
>  #include <stdio.h>
>
> +typedef uint32_t (*random_seed_ptr_t)(void);
> +
>  int main(void)
>  {
> -    int i, j, retry;
> +    int i, j, rsf, retry;
>      uint32_t seeds[N];
> +    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
>
> -    for (retry=0; retry<3; retry++){
> -        for (i=0; i<N; i++){
> -            seeds[i] = av_get_random_seed();
> -            for (j=0; j<i; j++)
> -                if (seeds[j] == seeds[i])
> -                    goto retry;
> +    for (rsf=0; rsf<F; ++rsf){
> +        for (retry=0; retry<3; retry++){
> +            for (i=0; i<N; i++){
> +                seeds[i] = random_seed[rsf]();
> +                for (j=0; j<i; j++)
> +                    if (seeds[j] == seeds[i])
> +                        goto retry;
> +            }
> +            printf("seeds OK\n");
> +            goto next;
> +            retry:;
>          }
> -        printf("seeds OK\n");
> -        return 0;
> -        retry:;
> +        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
> +        return 1;
> +        next:;
>      }
> -    printf("FAIL at %d with %X\n", j, seeds[j]);
> -    return 1;
> -}
> +    return 0;
> + }
> \ No newline at end of file
> diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
> index 2b5b3af..ef0eef2 100644
> --- a/tests/ref/fate/random_seed
> +++ b/tests/ref/fate/random_seed
> @@ -1 +1,2 @@
>  seeds OK
> +seeds OK
> --
> 1.9.1

The new test sporadically fails on msvc x86_64 for some reason. What
does it actually mean when it fails, ie. what does this thing test?

- Hendrik
Hendrik Leppkes Dec. 28, 2016, 6:12 p.m. UTC | #5
On Wed, Dec 28, 2016 at 7:08 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> On Fri, Dec 23, 2016 at 1:12 AM, Thomas Turner <thomastdt@googlemail.com> wrote:
>> Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
>> ---
>>  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
>>  tests/ref/fate/random_seed    |  1 +
>>  2 files changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
>> index ebe9b3e..fcd68bc 100644
>> --- a/libavutil/tests/random_seed.c
>> +++ b/libavutil/tests/random_seed.c
>> @@ -23,24 +23,32 @@
>>
>>  #undef printf
>>  #define N 256
>> +#define F 2
>>  #include <stdio.h>
>>
>> +typedef uint32_t (*random_seed_ptr_t)(void);
>> +
>>  int main(void)
>>  {
>> -    int i, j, retry;
>> +    int i, j, rsf, retry;
>>      uint32_t seeds[N];
>> +    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
>>
>> -    for (retry=0; retry<3; retry++){
>> -        for (i=0; i<N; i++){
>> -            seeds[i] = av_get_random_seed();
>> -            for (j=0; j<i; j++)
>> -                if (seeds[j] == seeds[i])
>> -                    goto retry;
>> +    for (rsf=0; rsf<F; ++rsf){
>> +        for (retry=0; retry<3; retry++){
>> +            for (i=0; i<N; i++){
>> +                seeds[i] = random_seed[rsf]();
>> +                for (j=0; j<i; j++)
>> +                    if (seeds[j] == seeds[i])
>> +                        goto retry;
>> +            }
>> +            printf("seeds OK\n");
>> +            goto next;
>> +            retry:;
>>          }
>> -        printf("seeds OK\n");
>> -        return 0;
>> -        retry:;
>> +        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
>> +        return 1;
>> +        next:;
>>      }
>> -    printf("FAIL at %d with %X\n", j, seeds[j]);
>> -    return 1;
>> -}
>> +    return 0;
>> + }
>> \ No newline at end of file
>> diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
>> index 2b5b3af..ef0eef2 100644
>> --- a/tests/ref/fate/random_seed
>> +++ b/tests/ref/fate/random_seed
>> @@ -1 +1,2 @@
>>  seeds OK
>> +seeds OK
>> --
>> 1.9.1
>
> The new test sporadically fails on msvc x86_64 for some reason. What
> does it actually mean when it fails, ie. what does this thing test?
>

Specifically, it always fails with rsf 1, which seems to be
get_generic_seed  - but windows has a special crypto seed provider,
get_generic_seed is never used. Making fate fail for some inaccuracy
in the clock in code thats never used is a bit annoying.

- Hendrik
Michael Niedermayer Dec. 28, 2016, 6:57 p.m. UTC | #6
On Wed, Dec 28, 2016 at 07:12:25PM +0100, Hendrik Leppkes wrote:
> On Wed, Dec 28, 2016 at 7:08 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> > On Fri, Dec 23, 2016 at 1:12 AM, Thomas Turner <thomastdt@googlemail.com> wrote:
> >> Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
> >> ---
> >>  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
> >>  tests/ref/fate/random_seed    |  1 +
> >>  2 files changed, 22 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
> >> index ebe9b3e..fcd68bc 100644
> >> --- a/libavutil/tests/random_seed.c
> >> +++ b/libavutil/tests/random_seed.c
> >> @@ -23,24 +23,32 @@
> >>
> >>  #undef printf
> >>  #define N 256
> >> +#define F 2
> >>  #include <stdio.h>
> >>
> >> +typedef uint32_t (*random_seed_ptr_t)(void);
> >> +
> >>  int main(void)
> >>  {
> >> -    int i, j, retry;
> >> +    int i, j, rsf, retry;
> >>      uint32_t seeds[N];
> >> +    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
> >>
> >> -    for (retry=0; retry<3; retry++){
> >> -        for (i=0; i<N; i++){
> >> -            seeds[i] = av_get_random_seed();
> >> -            for (j=0; j<i; j++)
> >> -                if (seeds[j] == seeds[i])
> >> -                    goto retry;
> >> +    for (rsf=0; rsf<F; ++rsf){
> >> +        for (retry=0; retry<3; retry++){
> >> +            for (i=0; i<N; i++){
> >> +                seeds[i] = random_seed[rsf]();
> >> +                for (j=0; j<i; j++)
> >> +                    if (seeds[j] == seeds[i])
> >> +                        goto retry;
> >> +            }
> >> +            printf("seeds OK\n");
> >> +            goto next;
> >> +            retry:;
> >>          }
> >> -        printf("seeds OK\n");
> >> -        return 0;
> >> -        retry:;
> >> +        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
> >> +        return 1;
> >> +        next:;
> >>      }
> >> -    printf("FAIL at %d with %X\n", j, seeds[j]);
> >> -    return 1;
> >> -}
> >> +    return 0;
> >> + }
> >> \ No newline at end of file
> >> diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
> >> index 2b5b3af..ef0eef2 100644
> >> --- a/tests/ref/fate/random_seed
> >> +++ b/tests/ref/fate/random_seed
> >> @@ -1 +1,2 @@
> >>  seeds OK
> >> +seeds OK
> >> --
> >> 1.9.1
> >
> > The new test sporadically fails on msvc x86_64 for some reason. What
> > does it actually mean when it fails, ie. what does this thing test?
> >
> 
> Specifically, it always fails with rsf 1, which seems to be
> get_generic_seed  - but windows has a special crypto seed provider,
> get_generic_seed is never used. Making fate fail for some inaccuracy
> in the clock in code thats never used is a bit annoying.

Thats like saying that as long as asm works it doesnt matter if the C
code doesnt work.

get_generic_seed() should work on any platform, ideally.
why does it fail on windows ?
can you take a look, its probably not very hard to improve it, the
function is also quite short.

[...]
Hendrik Leppkes Dec. 28, 2016, 7:05 p.m. UTC | #7
On Wed, Dec 28, 2016 at 7:57 PM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Wed, Dec 28, 2016 at 07:12:25PM +0100, Hendrik Leppkes wrote:
>> On Wed, Dec 28, 2016 at 7:08 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>> > On Fri, Dec 23, 2016 at 1:12 AM, Thomas Turner <thomastdt@googlemail.com> wrote:
>> >> Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
>> >> ---
>> >>  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
>> >>  tests/ref/fate/random_seed    |  1 +
>> >>  2 files changed, 22 insertions(+), 13 deletions(-)
>> >>
>> >> diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
>> >> index ebe9b3e..fcd68bc 100644
>> >> --- a/libavutil/tests/random_seed.c
>> >> +++ b/libavutil/tests/random_seed.c
>> >> @@ -23,24 +23,32 @@
>> >>
>> >>  #undef printf
>> >>  #define N 256
>> >> +#define F 2
>> >>  #include <stdio.h>
>> >>
>> >> +typedef uint32_t (*random_seed_ptr_t)(void);
>> >> +
>> >>  int main(void)
>> >>  {
>> >> -    int i, j, retry;
>> >> +    int i, j, rsf, retry;
>> >>      uint32_t seeds[N];
>> >> +    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
>> >>
>> >> -    for (retry=0; retry<3; retry++){
>> >> -        for (i=0; i<N; i++){
>> >> -            seeds[i] = av_get_random_seed();
>> >> -            for (j=0; j<i; j++)
>> >> -                if (seeds[j] == seeds[i])
>> >> -                    goto retry;
>> >> +    for (rsf=0; rsf<F; ++rsf){
>> >> +        for (retry=0; retry<3; retry++){
>> >> +            for (i=0; i<N; i++){
>> >> +                seeds[i] = random_seed[rsf]();
>> >> +                for (j=0; j<i; j++)
>> >> +                    if (seeds[j] == seeds[i])
>> >> +                        goto retry;
>> >> +            }
>> >> +            printf("seeds OK\n");
>> >> +            goto next;
>> >> +            retry:;
>> >>          }
>> >> -        printf("seeds OK\n");
>> >> -        return 0;
>> >> -        retry:;
>> >> +        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
>> >> +        return 1;
>> >> +        next:;
>> >>      }
>> >> -    printf("FAIL at %d with %X\n", j, seeds[j]);
>> >> -    return 1;
>> >> -}
>> >> +    return 0;
>> >> + }
>> >> \ No newline at end of file
>> >> diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
>> >> index 2b5b3af..ef0eef2 100644
>> >> --- a/tests/ref/fate/random_seed
>> >> +++ b/tests/ref/fate/random_seed
>> >> @@ -1 +1,2 @@
>> >>  seeds OK
>> >> +seeds OK
>> >> --
>> >> 1.9.1
>> >
>> > The new test sporadically fails on msvc x86_64 for some reason. What
>> > does it actually mean when it fails, ie. what does this thing test?
>> >
>>
>> Specifically, it always fails with rsf 1, which seems to be
>> get_generic_seed  - but windows has a special crypto seed provider,
>> get_generic_seed is never used. Making fate fail for some inaccuracy
>> in the clock in code thats never used is a bit annoying.
>
> Thats like saying that as long as asm works it doesnt matter if the C
> code doesnt work.

get_generic_seed is literally dead code in such a build, so its not
the same thing. Which platforms do even realistically use it at all?
On top of that, its behavior depends on platform specifics, ie. the
behavior of the clock function.

>
> get_generic_seed() should work on any platform, ideally.
> why does it fail on windows ?
> can you take a look, its probably not very hard to improve it, the
> function is also quite short.
>

I do not have the time right now to debug random sporadic failures,
since I'm going out of country in a few days for a couple weeks.
The MSDN documents the clock function as follows:

The clock function tells how much wall-clock time the calling process
has used. Note that this is not strictly conformant with ISO C99,
which specifies net CPU time as the return value. To obtain CPU time,
use the Win32 GetProcessTimes function.

So if the function is super fast, its certainly possible the values
just don't increment, since its wall-clock based. But its generally
not a problem, since its just never used.

- Hendrik
Hendrik Leppkes Dec. 28, 2016, 7:36 p.m. UTC | #8
On Wed, Dec 28, 2016 at 8:05 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
> On Wed, Dec 28, 2016 at 7:57 PM, Michael Niedermayer
> <michael@niedermayer.cc> wrote:
>> On Wed, Dec 28, 2016 at 07:12:25PM +0100, Hendrik Leppkes wrote:
>>> On Wed, Dec 28, 2016 at 7:08 PM, Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>>> > On Fri, Dec 23, 2016 at 1:12 AM, Thomas Turner <thomastdt@googlemail.com> wrote:
>>> >> Signed-off-by: Thomas Turner <thomastdt@googlemail.com>
>>> >> ---
>>> >>  libavutil/tests/random_seed.c | 34 +++++++++++++++++++++-------------
>>> >>  tests/ref/fate/random_seed    |  1 +
>>> >>  2 files changed, 22 insertions(+), 13 deletions(-)
>>> >>
>>> >> diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
>>> >> index ebe9b3e..fcd68bc 100644
>>> >> --- a/libavutil/tests/random_seed.c
>>> >> +++ b/libavutil/tests/random_seed.c
>>> >> @@ -23,24 +23,32 @@
>>> >>
>>> >>  #undef printf
>>> >>  #define N 256
>>> >> +#define F 2
>>> >>  #include <stdio.h>
>>> >>
>>> >> +typedef uint32_t (*random_seed_ptr_t)(void);
>>> >> +
>>> >>  int main(void)
>>> >>  {
>>> >> -    int i, j, retry;
>>> >> +    int i, j, rsf, retry;
>>> >>      uint32_t seeds[N];
>>> >> +    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
>>> >>
>>> >> -    for (retry=0; retry<3; retry++){
>>> >> -        for (i=0; i<N; i++){
>>> >> -            seeds[i] = av_get_random_seed();
>>> >> -            for (j=0; j<i; j++)
>>> >> -                if (seeds[j] == seeds[i])
>>> >> -                    goto retry;
>>> >> +    for (rsf=0; rsf<F; ++rsf){
>>> >> +        for (retry=0; retry<3; retry++){
>>> >> +            for (i=0; i<N; i++){
>>> >> +                seeds[i] = random_seed[rsf]();
>>> >> +                for (j=0; j<i; j++)
>>> >> +                    if (seeds[j] == seeds[i])
>>> >> +                        goto retry;
>>> >> +            }
>>> >> +            printf("seeds OK\n");
>>> >> +            goto next;
>>> >> +            retry:;
>>> >>          }
>>> >> -        printf("seeds OK\n");
>>> >> -        return 0;
>>> >> -        retry:;
>>> >> +        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
>>> >> +        return 1;
>>> >> +        next:;
>>> >>      }
>>> >> -    printf("FAIL at %d with %X\n", j, seeds[j]);
>>> >> -    return 1;
>>> >> -}
>>> >> +    return 0;
>>> >> + }
>>> >> \ No newline at end of file
>>> >> diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
>>> >> index 2b5b3af..ef0eef2 100644
>>> >> --- a/tests/ref/fate/random_seed
>>> >> +++ b/tests/ref/fate/random_seed
>>> >> @@ -1 +1,2 @@
>>> >>  seeds OK
>>> >> +seeds OK
>>> >> --
>>> >> 1.9.1
>>> >
>>> > The new test sporadically fails on msvc x86_64 for some reason. What
>>> > does it actually mean when it fails, ie. what does this thing test?
>>> >
>>>
>>> Specifically, it always fails with rsf 1, which seems to be
>>> get_generic_seed  - but windows has a special crypto seed provider,
>>> get_generic_seed is never used. Making fate fail for some inaccuracy
>>> in the clock in code thats never used is a bit annoying.
>>
>> Thats like saying that as long as asm works it doesnt matter if the C
>> code doesnt work.
>
> get_generic_seed is literally dead code in such a build, so its not
> the same thing. Which platforms do even realistically use it at all?
> On top of that, its behavior depends on platform specifics, ie. the
> behavior of the clock function.
>
>>
>> get_generic_seed() should work on any platform, ideally.
>> why does it fail on windows ?
>> can you take a look, its probably not very hard to improve it, the
>> function is also quite short.
>>
>
> I do not have the time right now to debug random sporadic failures,
> since I'm going out of country in a few days for a couple weeks.
> The MSDN documents the clock function as follows:
>
> The clock function tells how much wall-clock time the calling process
> has used. Note that this is not strictly conformant with ISO C99,
> which specifies net CPU time as the return value. To obtain CPU time,
> use the Win32 GetProcessTimes function.
>
> So if the function is super fast, its certainly possible the values
> just don't increment, since its wall-clock based. But its generally
> not a problem, since its just never used.
>

What I forgot to add, CLOCKS_PER_SEC is also only 1000, so only
millisecond precision, which probably plays into here.

- Hendrik
Michael Niedermayer Dec. 28, 2016, 7:57 p.m. UTC | #9
On Wed, Dec 28, 2016 at 08:05:17PM +0100, Hendrik Leppkes wrote:
> On Wed, Dec 28, 2016 at 7:57 PM, Michael Niedermayer
[...]
> >
> > get_generic_seed() should work on any platform, ideally.
> > why does it fail on windows ?
> > can you take a look, its probably not very hard to improve it, the
> > function is also quite short.
> >
> 
> I do not have the time right now to debug random sporadic failures,
> since I'm going out of country in a few days for a couple weeks.
> The MSDN documents the clock function as follows:
> 
> The clock function tells how much wall-clock time the calling process
> has used. Note that this is not strictly conformant with ISO C99,
> which specifies net CPU time as the return value. To obtain CPU time,
> use the Win32 GetProcessTimes function.
> 

> So if the function is super fast, its certainly possible the values
> just don't increment, since its wall-clock based. But its generally
> not a problem, since its just never used.

the code should not accept "non incrementing clocks" but wait for a
few increments to have happened, it possibly waits for too few to
gather enough entropy, possibly theres also something else wrong

[...]
diff mbox

Patch

diff --git a/libavutil/tests/random_seed.c b/libavutil/tests/random_seed.c
index ebe9b3e..fcd68bc 100644
--- a/libavutil/tests/random_seed.c
+++ b/libavutil/tests/random_seed.c
@@ -23,24 +23,32 @@ 
 
 #undef printf
 #define N 256
+#define F 2
 #include <stdio.h>
 
+typedef uint32_t (*random_seed_ptr_t)(void);
+
 int main(void)
 {
-    int i, j, retry;
+    int i, j, rsf, retry;
     uint32_t seeds[N];
+    random_seed_ptr_t random_seed[F] = {av_get_random_seed, get_generic_seed};
 
-    for (retry=0; retry<3; retry++){
-        for (i=0; i<N; i++){
-            seeds[i] = av_get_random_seed();
-            for (j=0; j<i; j++)
-                if (seeds[j] == seeds[i])
-                    goto retry;
+    for (rsf=0; rsf<F; ++rsf){
+        for (retry=0; retry<3; retry++){
+            for (i=0; i<N; i++){
+                seeds[i] = random_seed[rsf]();
+                for (j=0; j<i; j++)
+                    if (seeds[j] == seeds[i])
+                        goto retry;
+            }
+            printf("seeds OK\n");
+            goto next;
+            retry:;
         }
-        printf("seeds OK\n");
-        return 0;
-        retry:;
+        printf("rsf %d: FAIL at %d with %X\n", rsf, j, seeds[j]);
+        return 1;
+        next:;
     }
-    printf("FAIL at %d with %X\n", j, seeds[j]);
-    return 1;
-}
+    return 0;
+ }
\ No newline at end of file
diff --git a/tests/ref/fate/random_seed b/tests/ref/fate/random_seed
index 2b5b3af..ef0eef2 100644
--- a/tests/ref/fate/random_seed
+++ b/tests/ref/fate/random_seed
@@ -1 +1,2 @@ 
 seeds OK
+seeds OK