[FFmpeg-cvslog] r23302 - trunk/libavformat/oggenc.c

[Måns Rullgård]

Baptiste Coudurier <baptiste.coudurier at gmail.com> writes:

> On 05/24/2010 04:45 PM, M?ns Rullg?rd wrote:
>> bcoudurier<subversion at mplayerhq.hu>  writes:
>>
>>> Author: bcoudurier
>>> Date: Tue May 25 01:37:33 2010
>>> New Revision: 23302
>>>
>>> Log:
>>> In ogg muxer, use random serial number of each ogg streams
>>>
>>> +        if (!(st->codec->flags&  CODEC_FLAG_BITEXACT))
>>> +            do {
>>> +                serial_num = av_get_random_seed();
>>
>> This isn't how av_get_random_seed() is meant to be used.  On many
>> systems this simply returns the current time, so you're likely to end
>> up with a consecutive sequence here regardless.  You're supposed to
>> use a seed with one of the PRNGs.
>>
>>> +                for (j = 0; j<  i; j++) {
>>> +                    OGGStreamContext *sc = s->streams[j]->priv_data;
>>> +                    if (serial_num == sc->serial_num)
>>> +                        break;
>>> +                }
>>> +            } while (j<  i);
>>> +        oggstream->serial_num = serial_num;
>>
>> This could potentially never terminate.  That is of course exceedingly
>> unlikely, but I think such code should be avoided nonetheless.
>>
>> Please redo this properly, or not at all.  The ogg spec clearly allows
>> serial_num = index.  Until that changes, there is no reason whatsoever
>> to follow their stupid, made-up rules.
>
> IMHO this is good enough. Feel free to submit a better solution.

IMHO you are being lazy.  At the very least, could you provide some
rationale for this idiotic change?

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