[FFmpeg-devel] [PATCH]pes packetizer

[Vitor Sessak]

Hi

I hope you don't mind some nitpicking...

realsun wrote:

[...]

> -static int mpeg_mux_init(AVFormatContext *ctx)
> -{
> -    MpegMuxContext *s = ctx->priv_data;
> -    int bitrate, i, mpa_id, mpv_id, mps_id, ac3_id, dts_id, lpcm_id, j;
>      AVStream *st;
> -    StreamInfo *stream;
> -    int audio_bitrate;
> -    int video_bitrate;
> +    PESStream *stream;
> +    int i;
>  

[...]

> -
> +        stream = (PESStream*)st->priv_data;
>          av_set_pts_info(st, 64, 1, 90000);

Unneeded cast

> -        if (s->is_mpeg2) {
>              /* special stuffing byte that is always written
> -               to prevent accidental generation of start codes. */
> -            put_byte(&ctx->pb, 0xff);
> +               to prevent accidental generation of startcodes. */
> +             put_byte(&ctx->pb, 0xff);
>  
> -            for(i=0;i<stuffing_size;i++)
> -                put_byte(&ctx->pb, 0xff);
> -        }
> +             for(i=0;i<stuffing_size;i++)
> +                 put_byte(&ctx->pb, 0xff);

Cosmetical

>  
> -static int output_packet(AVFormatContext *ctx, int flush){
> -    MpegMuxContext *s = ctx->priv_data;
> -    AVStream *st;
> -    StreamInfo *stream;
> -    int i, avail_space, es_size, trailer_size;
> -    int best_i= -1;
> -    int best_score= INT_MIN;
> -    int ignore_constraints=0;
> -    int64_t scr= s->last_scr;
> -    PacketDesc *timestamp_packet;
> +
> +int ff_pes_find_beststream(AVFormatContext *ctx, int packet_size, int flush, int64_t scr, int* best_i)
> +{
> +    int best_score = INT_MIN;
> +    int i, avail_space = 0;
> +    int ignore_constraints = 0;
>      const int64_t max_delay= av_rescale(ctx->max_delay, 90000, AV_TIME_BASE);

Contain cosmeticals

>  
> -static int mpeg_mux_write_packet(AVFormatContext *ctx, AVPacket *pkt)
> +
> +void ff_pes_write_packet(AVFormatContext *ctx, AVPacket *pkt)
>  {
> -    MpegMuxContext *s = ctx->priv_data;
>      int stream_index= pkt->stream_index;
> +    AVStream *st = ctx->streams[stream_index];
> +    PESStream *stream = st->priv_data;
> +    PacketDesc *pkt_desc;
>      int size= pkt->size;
>      uint8_t *buf= pkt->data;
> -    AVStream *st = ctx->streams[stream_index];
> -    StreamInfo *stream = st->priv_data;
>      int64_t pts, dts;
> -    PacketDesc *pkt_desc;
>      const int preload= av_rescale(ctx->preload, 90000, AV_TIME_BASE);

Cosmetical: reordering

> -    const int is_iframe = st->codec->codec_type == CODEC_TYPE_VIDEO && (pkt->flags & PKT_FLAG_KEY);
>  
>      pts= pkt->pts;
>      dts= pkt->dts;
> @@ -1151,13 +267,12 @@
>      if(pts != AV_NOPTS_VALUE) pts += preload;
>      if(dts != AV_NOPTS_VALUE) dts += preload;
>  
> -//av_log(ctx, AV_LOG_DEBUG, "dts:%f pts:%f flags:%d stream:%d nopts:%d\n", dts/90000.0, pts/90000.0, pkt->flags, pkt->stream_index, pts != AV_NOPTS_VALUE);

I think dead code removal is considered cosmetical

>      bitrate = 0;
>      audio_bitrate = 0;
>      video_bitrate = 0;
>      for(i=0;i<ctx->nb_streams;i++) {
>          int codec_rate;
>          st = ctx->streams[i];
> -        stream = (StreamInfo*) st->priv_data;
> +        stream = st->priv_data;

While it is an unneeded cast (which was complained about), I think that
if it was there before it should be removed in a next patch.

> +/**
> + * Remove decoded packets of each stream.
> + * @param[in] ctx  the AVFormatContext
> + * @param[in] scr  System Clock Reference of PES stream
> + * @return  On error a negative or zero value is returned, on success 1 is returned.
> + */
> +int ff_pes_remove_decoded_packets(AVFormatContext *ctx, int64_t scr);
> +
> +/*
> + * Insert a timestamp into the ByteIOContext.
> + * @param[in] pb        the ByteIOContext to be written to
> + * @param[in] id        stream ID
> + * @param[in] timestamp the timestamp
> + * @return  NULL
> + */
> +void ff_put_timestamp(ByteIOContext *pb, int id, int64_t timestamp);
> +
> +#endif/* AVFORMAT_MPEG_PES_H */

You forgot the extra star for doxygen in the last one.

-Vitor