| Message ID | 20191011061444.4988-6-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
lance.lmwang@gmail.com: > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavfilter/vf_random.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_random.c b/libavfilter/vf_random.c > index 1937eae771..9f2153be61 100644 > --- a/libavfilter/vf_random.c > +++ b/libavfilter/vf_random.c > @@ -33,7 +33,7 @@ typedef struct RandomContext { > > AVLFG lfg; > int nb_frames; > - int64_t random_seed; > + int32_t random_seed; > int nb_frames_filled; > AVFrame *frames[MAX_FRAMES]; > int64_t pts[MAX_FRAMES]; > { "seed", "set the seed", OFFSET(random_seed), AV_OPT_TYPE_INT64, {.i64=-1}, -1, UINT32_MAX, FLAGS }, So in addition to the complete range of an uint32_t one also needs another value that instructs init to get a random seed of its own. - Andreas
On Fri, Oct 11, 2019 at 06:20:00AM +0000, Andreas Rheinhardt wrote: > lance.lmwang@gmail.com: > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavfilter/vf_random.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavfilter/vf_random.c b/libavfilter/vf_random.c > > index 1937eae771..9f2153be61 100644 > > --- a/libavfilter/vf_random.c > > +++ b/libavfilter/vf_random.c > > @@ -33,7 +33,7 @@ typedef struct RandomContext { > > > > AVLFG lfg; > > int nb_frames; > > - int64_t random_seed; > > + int32_t random_seed; > > int nb_frames_filled; > > AVFrame *frames[MAX_FRAMES]; > > int64_t pts[MAX_FRAMES]; > > > { "seed", "set the seed", OFFSET(random_seed), > AV_OPT_TYPE_INT64, {.i64=-1}, -1, UINT32_MAX, FLAGS }, > > So in addition to the complete range of an uint32_t one also needs > another value that instructs init to get a random seed of its own. yes, the max range is UINT32_MAX, so no need to use int64_t > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Limin Wang: > On Fri, Oct 11, 2019 at 06:20:00AM +0000, Andreas Rheinhardt wrote: >> lance.lmwang@gmail.com: >>> From: Limin Wang <lance.lmwang@gmail.com> >>> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >>> --- >>> libavfilter/vf_random.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavfilter/vf_random.c b/libavfilter/vf_random.c >>> index 1937eae771..9f2153be61 100644 >>> --- a/libavfilter/vf_random.c >>> +++ b/libavfilter/vf_random.c >>> @@ -33,7 +33,7 @@ typedef struct RandomContext { >>> >>> AVLFG lfg; >>> int nb_frames; >>> - int64_t random_seed; >>> + int32_t random_seed; >>> int nb_frames_filled; >>> AVFrame *frames[MAX_FRAMES]; >>> int64_t pts[MAX_FRAMES]; >>> >> { "seed", "set the seed", OFFSET(random_seed), >> AV_OPT_TYPE_INT64, {.i64=-1}, -1, UINT32_MAX, FLAGS }, >> >> So in addition to the complete range of an uint32_t one also needs >> another value that instructs init to get a random seed of its own. > > yes, the max range is UINT32_MAX, so no need to use int64_t > First of all, simply changing the value to 32 bit is not right -- you would also have to change the type in the corresponding AVOption (it might seem to work on little-endian systems, but it certainly doesn't on big-endian systems; actually using -1 as random_seed would probably already not work on little-endian systems either). Second, there are more legal values for random_seed than values representable in 32 bit, hence one has to use a type with more than 32 bit. - Andreas
On Fri, Oct 11, 2019 at 06:59:00AM +0000, Andreas Rheinhardt wrote: > Limin Wang: > > On Fri, Oct 11, 2019 at 06:20:00AM +0000, Andreas Rheinhardt wrote: > >> lance.lmwang@gmail.com: > >>> From: Limin Wang <lance.lmwang@gmail.com> > >>> > >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >>> --- > >>> libavfilter/vf_random.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/libavfilter/vf_random.c b/libavfilter/vf_random.c > >>> index 1937eae771..9f2153be61 100644 > >>> --- a/libavfilter/vf_random.c > >>> +++ b/libavfilter/vf_random.c > >>> @@ -33,7 +33,7 @@ typedef struct RandomContext { > >>> > >>> AVLFG lfg; > >>> int nb_frames; > >>> - int64_t random_seed; > >>> + int32_t random_seed; > >>> int nb_frames_filled; > >>> AVFrame *frames[MAX_FRAMES]; > >>> int64_t pts[MAX_FRAMES]; > >>> > >> { "seed", "set the seed", OFFSET(random_seed), > >> AV_OPT_TYPE_INT64, {.i64=-1}, -1, UINT32_MAX, FLAGS }, > >> > >> So in addition to the complete range of an uint32_t one also needs > >> another value that instructs init to get a random seed of its own. > > > > yes, the max range is UINT32_MAX, so no need to use int64_t > > > First of all, simply changing the value to 32 bit is not right -- you > would also have to change the type in the corresponding AVOption (it I haven't see .i32 type for AVOption > might seem to work on little-endian systems, but it certainly doesn't > on big-endian systems; actually using -1 as random_seed would probably > already not work on little-endian systems either). Second, there are > more legal values for random_seed than values representable in 32 bit, > hence one has to use a type with more than 32 bit. random_seed = -1, it'll get the seed by av_get_random_seed(), it'll return 32bit. else uint32_t seed seed = s->random_seed; av_lfg_init(&s->lfg, seed); the seed is uint32_t still, so user can't use with more than 32bit for the current code. > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Limin Wang: > On Fri, Oct 11, 2019 at 06:59:00AM +0000, Andreas Rheinhardt wrote: >> Limin Wang: >>> On Fri, Oct 11, 2019 at 06:20:00AM +0000, Andreas Rheinhardt wrote: >>>> lance.lmwang@gmail.com: >>>>> From: Limin Wang <lance.lmwang@gmail.com> >>>>> >>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> >>>>> --- >>>>> libavfilter/vf_random.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/libavfilter/vf_random.c b/libavfilter/vf_random.c >>>>> index 1937eae771..9f2153be61 100644 >>>>> --- a/libavfilter/vf_random.c >>>>> +++ b/libavfilter/vf_random.c >>>>> @@ -33,7 +33,7 @@ typedef struct RandomContext { >>>>> >>>>> AVLFG lfg; >>>>> int nb_frames; >>>>> - int64_t random_seed; >>>>> + int32_t random_seed; >>>>> int nb_frames_filled; >>>>> AVFrame *frames[MAX_FRAMES]; >>>>> int64_t pts[MAX_FRAMES]; >>>>> >>>> { "seed", "set the seed", OFFSET(random_seed), >>>> AV_OPT_TYPE_INT64, {.i64=-1}, -1, UINT32_MAX, FLAGS }, >>>> >>>> So in addition to the complete range of an uint32_t one also needs >>>> another value that instructs init to get a random seed of its own. >>> >>> yes, the max range is UINT32_MAX, so no need to use int64_t >>> >> First of all, simply changing the value to 32 bit is not right -- you >> would also have to change the type in the corresponding AVOption (it > > I haven't see .i32 type for AVOption > >> might seem to work on little-endian systems, but it certainly doesn't >> on big-endian systems; actually using -1 as random_seed would probably >> already not work on little-endian systems either). Second, there are >> more legal values for random_seed than values representable in 32 bit, >> hence one has to use a type with more than 32 bit. > > random_seed = -1, it'll get the seed by av_get_random_seed(), it'll > return 32bit. > > else > > uint32_t seed > seed = s->random_seed; > av_lfg_init(&s->lfg, seed); > > the seed is uint32_t still, so user can't use with more than 32bit > for the current code. > There are more than 2^32 legal values the user can input for random_seed: The 2^32 values which should be directly used as random seed and the special value -1. You need a type with more than 32 bits to distinguish -1 from UINT32_MAX. - Andreas
On Fri, Oct 11, 2019 at 07:34:00AM +0000, Andreas Rheinhardt wrote: > Limin Wang: > > On Fri, Oct 11, 2019 at 06:59:00AM +0000, Andreas Rheinhardt wrote: > >> Limin Wang: > >>> On Fri, Oct 11, 2019 at 06:20:00AM +0000, Andreas Rheinhardt wrote: > >>>> lance.lmwang@gmail.com: > >>>>> From: Limin Wang <lance.lmwang@gmail.com> > >>>>> > >>>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > >>>>> --- > >>>>> libavfilter/vf_random.c | 2 +- > >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/libavfilter/vf_random.c b/libavfilter/vf_random.c > >>>>> index 1937eae771..9f2153be61 100644 > >>>>> --- a/libavfilter/vf_random.c > >>>>> +++ b/libavfilter/vf_random.c > >>>>> @@ -33,7 +33,7 @@ typedef struct RandomContext { > >>>>> > >>>>> AVLFG lfg; > >>>>> int nb_frames; > >>>>> - int64_t random_seed; > >>>>> + int32_t random_seed; > >>>>> int nb_frames_filled; > >>>>> AVFrame *frames[MAX_FRAMES]; > >>>>> int64_t pts[MAX_FRAMES]; > >>>>> > >>>> { "seed", "set the seed", OFFSET(random_seed), > >>>> AV_OPT_TYPE_INT64, {.i64=-1}, -1, UINT32_MAX, FLAGS }, > >>>> > >>>> So in addition to the complete range of an uint32_t one also needs > >>>> another value that instructs init to get a random seed of its own. > >>> > >>> yes, the max range is UINT32_MAX, so no need to use int64_t > >>> > >> First of all, simply changing the value to 32 bit is not right -- you > >> would also have to change the type in the corresponding AVOption (it > > > > I haven't see .i32 type for AVOption > > > >> might seem to work on little-endian systems, but it certainly doesn't > >> on big-endian systems; actually using -1 as random_seed would probably > >> already not work on little-endian systems either). Second, there are > >> more legal values for random_seed than values representable in 32 bit, > >> hence one has to use a type with more than 32 bit. > > > > random_seed = -1, it'll get the seed by av_get_random_seed(), it'll > > return 32bit. > > > > else > > > > uint32_t seed > > seed = s->random_seed; > > av_lfg_init(&s->lfg, seed); > > > > the seed is uint32_t still, so user can't use with more than 32bit > > for the current code. > > > There are more than 2^32 legal values the user can input for > random_seed: The 2^32 values which should be directly used as random > seed and the special value -1. You need a type with more than 32 bits > to distinguish -1 from UINT32_MAX. OK, thanks for reviewing and feedback, I haven't think it too much at first, let keep it 64bit anyway. > > - Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavfilter/vf_random.c b/libavfilter/vf_random.c index 1937eae771..9f2153be61 100644 --- a/libavfilter/vf_random.c +++ b/libavfilter/vf_random.c @@ -33,7 +33,7 @@ typedef struct RandomContext { AVLFG lfg; int nb_frames; - int64_t random_seed; + int32_t random_seed; int nb_frames_filled; AVFrame *frames[MAX_FRAMES]; int64_t pts[MAX_FRAMES];