[FFmpeg-devel] [PATCH]pes packetizer

Michael Niedermayer michaelni
Sat Jun 23 23:47:22 CEST 2007


Hi

On Sat, Jun 23, 2007 at 06:11:29PM +0800, realsun wrote:
> Hi,
> This is the PES packetizer code and I have made MPEG PS encoder to use
> this packetizer and I hope this could be reused by TS muxer.

please diff files against their proper parent file

following review of pes.c is based on a diff of it against mpegenc.c
ive also edited the diff to place related functions adjacent to each
other


> --- /home/michael/orgmpegmux/mpegenc.c	2007-06-23 23:24:43.000000000 +0200
> +++ pes.c	2007-06-23 23:24:51.000000000 +0200
> @@ -1,6 +1,6 @@
>  /*
> - * MPEG1/2 muxer
> - * Copyright (c) 2000, 2001, 2002 Fabrice Bellard.
> + * PES muxer.
> + * Copyright (c) 2007 Xiaohui Sun <sunxiaohui at dsp.ac.cn>
>   *

this is not correct, you are not the only author of this code


[...]

> +/*
> + * Put timestamp into packet
> + * @param[in] p         the stream to write
> + * @param[in] id        stream id
> + * @param[in] timestamp the timestamp to put in
> + * @return  NULL
> + */
> +static inline void put_timestamp(uint8_t* p, int id, int64_t timestamp)
> +{
> +    bytestream_put_byte(&p,
> +        (id << 4) |
> +        (((timestamp >> 30) & 0x07) << 1) |
> +        1);
> +    bytestream_put_be16(&p, (uint16_t)((((timestamp >> 15) & 0x7fff) << 1) | 1));
> +    bytestream_put_be16(&p, (uint16_t)((((timestamp) & 0x7fff) << 1) | 1));
>  }

> -static inline void put_timestamp(ByteIOContext *pb, int id, int64_t timestamp)
> -{
> -    put_byte(pb,
> -             (id << 4) |
> -             (((timestamp >> 30) & 0x07) << 1) |
> -             1);
> -    put_be16(pb, (uint16_t)((((timestamp >> 15) & 0x7fff) << 1) | 1));
> -    put_be16(pb, (uint16_t)((((timestamp) & 0x7fff) << 1) | 1));

a change from ByteIOContext to uint8_t* belongs into a seperate patch
the addition of doxygen comments also belongs into a seperate patch
and so does the splitig out of functions from mpegenc.c


[...]

> -static int get_nb_frames(AVFormatContext *ctx, StreamInfo *stream, int len){
> -    int nb_frames=0;
> -    PacketDesc *pkt_desc= stream->premux_packet;
> -
> -    while(len>0){
> -        if(pkt_desc->size == pkt_desc->unwritten_size)
> -            nb_frames++;
> -        len -= pkt_desc->unwritten_size;
> -        pkt_desc= pkt_desc->next;
> -    }
> -
> -    return nb_frames;
> +static int get_nb_frames(AVFormatContext *ctx, PESStream *stream, int len){
> +    int nb_frames=0;
> +    PacketDesc *pkt_desc= stream->premux_packet;
>  
> +    while(len>0){
> +        if(pkt_desc->size == pkt_desc->unwritten_size)
> +            nb_frames++;
> +        len -= pkt_desc->unwritten_size;
> +        pkt_desc= pkt_desc->next;
> +    }

please dont reorder functions


[...]
>  
> -static int mpeg_mux_init(AVFormatContext *ctx)
> +/*
> + * Initialization of PES mux.
> + * @param[in] ctx the AVFormatContext which contains streams
> + * @return  On error a negative value is returned, on success zero.
> + */

not doxygen compatible


> +int pes_mux_init(AVFormatContext *ctx)

non static function without proper prefix


[...]
>      mpa_id = AUDIO_ID;
>      ac3_id = AC3_ID;
>      dts_id = DTS_ID;
>      mpv_id = VIDEO_ID;
>      mps_id = SUB_ID;
>      lpcm_id = LPCM_ID;
> -    for(i=0;i<ctx->nb_streams;i++) {
> -        st = ctx->streams[i];
> -        stream = av_mallocz(sizeof(StreamInfo));
> -        if (!stream)
> -            goto fail;
> -        st->priv_data = stream;
>  
> +    for (i = 0; i < ctx->nb_streams; i++) {
> +        st = ctx->streams[i];
> +        stream = (PESStream*)st->priv_data;

cosmetic


>          av_set_pts_info(st, 64, 1, 90000);
>  
>          switch(st->codec->codec_type) {
> @@ -336,7 +91,7 @@ static int mpeg_mux_init(AVFormatContext
>                          break;
>                  }
>                  if (j == 4)
> -                    goto fail;
> +                    return AVERROR(ENOMEM);
>                  if (st->codec->channels > 8)
>                      return -1;
>                  stream->lpcm_header[0] = 0x0c;
> @@ -346,18 +101,13 @@ static int mpeg_mux_init(AVFormatContext
>              } else {
>                  stream->id = mpa_id++;
>              }
> -
>              /* This value HAS to be used for VCD (see VCD standard, p. IV-7).
> -               Right now it is also used for everything else.*/
> +             Right now it is also used for everything else.*/
>              stream->max_buffer_size = 4 * 1024;

cosmetic


> -            s->audio_bound++;
> +

>              break;
>          case CODEC_TYPE_VIDEO:
>              stream->id = mpv_id++;
> -            if (st->codec->rc_buffer_size)
> -                stream->max_buffer_size = 6*1024 + st->codec->rc_buffer_size/8;
> -            else
> -                stream->max_buffer_size = 230*1024; //FIXME this is probably too small as default
>  #if 0
>                  /* see VCD standard, p. IV-7*/
>                  stream->max_buffer_size = 46 * 1024;
> @@ -366,7 +116,10 @@ static int mpeg_mux_init(AVFormatContext
>                     Right now it is also used for everything else.*/
>                  stream->max_buffer_size = 230 * 1024;
>  #endif
> -            s->video_bound++;
> +            if (st->codec->rc_buffer_size)
> +                stream->max_buffer_size = 6*1024 + st->codec->rc_buffer_size/8;
> +            else
> +                stream->max_buffer_size = 230*1024; //FIXME this is probably too small as default
>              break;

cosmetic


[...]

>      const int64_t max_delay= av_rescale(ctx->max_delay, 90000, AV_TIME_BASE);
>  
>  retry:
> -    for(i=0; i<ctx->nb_streams; i++){
> +    for (i = 0; i < ctx->nb_streams; i++){
>          AVStream *st = ctx->streams[i];
> -        StreamInfo *stream = st->priv_data;
> -        const int avail_data=  av_fifo_size(&stream->fifo);
> -        const int space= stream->max_buffer_size - stream->buffer_index;
> -        int rel_space= 1024*space / stream->max_buffer_size;
> -        PacketDesc *next_pkt= stream->premux_packet;
> +        PESStream*stream = st->priv_data;
> +        const int avail_data =  av_fifo_size(&stream->fifo);
> +        const int space = stream->max_buffer_size - stream->buffer_index;
> +        int rel_space = 1024*space / stream->max_buffer_size;
> +        PacketDesc *next_pkt = stream->premux_packet;
>  
>          /* for subtitle, a single PES packet must be generated,
> -           so we flush after every single subtitle packet */
> -        if(s->packet_size > avail_data && !flush
> -           && st->codec->codec_type != CODEC_TYPE_SUBTITLE)
> +          so we flush after every single subtitle packet */
> +        if (packet_size > avail_data && !flush
> +            && st->codec->codec_type != CODEC_TYPE_SUBTITLE)
>              return 0;
> -        if(avail_data==0)
> +        if (avail_data == 0)
>              continue;
> -        assert(avail_data>0);
> +        assert(avail_data > 0);
>  
> -        if(space < s->packet_size && !ignore_constraints)
> +        if (space < packet_size && !ignore_constraints)
>              continue;
>  
> -        if(next_pkt && next_pkt->dts - scr > max_delay)
> +        if (next_pkt && next_pkt->dts - scr > max_delay)
>              continue;
>  
> -        if(rel_space > best_score){
> +        if (rel_space > best_score){
>              best_score= rel_space;
> -            best_i = i;
> -            avail_space= space;
> +            *best_i = i;
> +            avail_space = space;
>          }
>      }
>  
> -    if(best_i < 0){
> -        int64_t best_dts= INT64_MAX;
> +    if (*best_i < 0){
> +      int64_t best_dts = INT64_MAX;
>  
> -        for(i=0; i<ctx->nb_streams; i++){
> -            AVStream *st = ctx->streams[i];
> -            StreamInfo *stream = st->priv_data;
> -            PacketDesc *pkt_desc= stream->predecode_packet;
> -            if(pkt_desc && pkt_desc->dts < best_dts)
> -                best_dts= pkt_desc->dts;
> -        }
> +    for (i = 0; i < ctx->nb_streams; i++){
> +        AVStream *st = ctx->streams[i];
> +        PESStream *stream = st->priv_data;
> +        PacketDesc *pkt_desc = stream->predecode_packet;
> +        if (pkt_desc && pkt_desc->dts < best_dts)
> +            best_dts = pkt_desc->dts;
> +    }
>  
>  #if 0
>          av_log(ctx, AV_LOG_DEBUG, "bumping scr, scr:%f, dts:%f\n",
> -               scr/90000.0, best_dts/90000.0);
> +            scr/90000.0, best_dts/90000.0);
>  #endif
> -        if(best_dts == INT64_MAX)
> +
> +        if (best_dts == INT64_MAX)
>              return 0;
>  
> -        if(scr >= best_dts+1 && !ignore_constraints){
> -            av_log(ctx, AV_LOG_ERROR, "packet too large, ignoring buffer limits to mux it\n");
> -            ignore_constraints= 1;
> +        if (scr >= best_dts+1 && !ignore_constraints){
> +          av_log(ctx, AV_LOG_ERROR, "packet too large, ignoring buffer limits to mux it\n");
> +          ignore_constraints = 1;
>          }

lots of cosmetics

remainder of the patch not reviewed

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070623/7a678827/attachment.pgp>



More information about the ffmpeg-devel mailing list