[FFmpeg-devel] [PATCH] change the order of params for av_init_random()

Måns Rullgård mans
Mon Aug 18 00:34:53 CEST 2008


Michael Niedermayer <michaelni at gmx.at> writes:

> On Sun, Aug 17, 2008 at 10:44:48PM +0100, M?ns Rullg?rd wrote:
>> Michael Niedermayer <michaelni at gmx.at> writes:
>> 
>> > On Sun, Aug 17, 2008 at 10:06:01PM +0100, M?ns Rullg?rd wrote:
>> >> Justin Ruggles <justinruggles at bellsouth.net> writes:
>> > [...]
>> >> > Index: libavutil/random.c
>> >> > ===================================================================
>> >> > --- libavutil/random.c	(revision 14819)
>> >> > +++ libavutil/random.c	(working copy)
>> >> > @@ -36,7 +36,7 @@
>> >> >  #define LOWER_MASK 0x7fffffff /* least significant r bits */
>> >> >  
>> >> >  /** initializes mt[AV_RANDOM_N] with a seed */
>> >> > -void av_init_random(unsigned int seed, AVRandomState *state)
>> >> > +void av_init_random(AVRandomState *state, unsigned int seed)
>> >> 
>> >> This requires a major version bump, since it breaks ABI.
>> >
>> > i dont think anyone is using it, besides its not hard to add a
>> 
>> The header is installed, so we have to assume that somebody might be
>> using it.  Changing this would cause those apps to start crashing for
>> no apparent reason.
>
> non existing apps crashing ...

Can you prove that no such apps exist?  If not, breaking ABI is not
acceptable.  People complain enough about FFmpeg API/ABI stability as
it is.  No need to prove them right.  (They're mostly mistaken, of
course.)

>> > av_init_random2() and make av_init_random() call it if someone
>> > wants it.
>> 
>> This is another option, but is it really worth it just to change the
>> order of arguments?
>
> I think it is because it improves consistency and readability, taken on
> its own its rather minor but still it should be fixed, it just costs us
>
> #if version <
> av_init_random(b,a)
> {
>     av_init_random2(a,b)
> }
> #endif
>
> until the next major bump ...

Fine by me.

-- 
M?ns Rullg?rd
mans at mansr.com




More information about the ffmpeg-devel mailing list