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

[Måns Rullgård]

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
>
> Modified:
>    trunk/libavformat/oggenc.c
>
> Modified: trunk/libavformat/oggenc.c
> ==============================================================================
> --- trunk/libavformat/oggenc.c	Mon May 24 23:59:32 2010	(r23301)
> +++ trunk/libavformat/oggenc.c	Tue May 25 01:37:33 2010	(r23302)
> @@ -20,6 +20,7 @@
>   */
>
>  #include "libavutil/crc.h"
> +#include "libavutil/random_seed.h"
>  #include "libavcodec/xiph.h"
>  #include "libavcodec/bytestream.h"
>  #include "libavcodec/flac.h"
> @@ -50,6 +51,7 @@ typedef struct {
>      int eos;
>      unsigned page_count; ///< number of page buffered
>      OGGPage page; ///< current page
> +    unsigned serial_num; ///< serial number
>  } OGGStreamContext;
>
>  typedef struct OGGPageList {
> @@ -80,7 +82,7 @@ static void ogg_write_page(AVFormatConte
>      put_byte(s->pb, 0);
>      put_byte(s->pb, page->flags | extra_flags);
>      put_le64(s->pb, page->granule);
> -    put_le32(s->pb, page->stream_index);
> +    put_le32(s->pb, oggstream->serial_num);
>      put_le32(s->pb, oggstream->page_counter++);
>      crc_offset = url_ftell(s->pb);
>      put_le32(s->pb, 0); // crc
> @@ -280,8 +282,11 @@ static int ogg_write_header(AVFormatCont
>  {
>      OGGStreamContext *oggstream;
>      int i, j;
> +
>      for (i = 0; i < s->nb_streams; i++) {
>          AVStream *st = s->streams[i];
> +        unsigned serial_num = i;
> +
>          if (st->codec->codec_type == AVMEDIA_TYPE_AUDIO)
>              av_set_pts_info(st, 64, 1, st->codec->sample_rate);
>          else if (st->codec->codec_type == AVMEDIA_TYPE_VIDEO)
> @@ -300,6 +305,18 @@ static int ogg_write_header(AVFormatCont
>          }
>          oggstream = av_mallocz(sizeof(*oggstream));
>          oggstream->page.stream_index = i;
> +
> +        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.

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