[FFmpeg-devel] [PATCH]pes packetizer

[realsun]

it seems my last mail not sent, so sent again.
Baptiste Coudurier wrote:
> Hi
>
> realsun wrote:
>   
>> [...]
>>
>> --- pes.h	(revision 0)
>> +++ pes.h	(revision 0)
>>
>> [...]
>>
>> +#ifndef PES_H
>> +#define PES_H
>>     
>
> AVFORMAT_PES_H is better.
>   
changed

>   
>> [...]
>>
>> +
>> +typedef struct {
>> +    int is_ps;  /*< whether it is a Program Stream */
>> +    int packet_number;
>> +} PESContext;
>>     
>
> We might have 3 possibilites (PS, TS, PES).
> An enum is better IMHO.
>   

changed

>   
>> [...]
>>
>> + */
>> +int ff_pes_mux_write(AVFormatContext *ctx, int stream_index,
>> +          int64_t pts,int64_t dts, int  id, int startcode,
>> +          uint8_t* pes_content, int pes_content_len,
>> +          int header_len, int packet_size, int payload_size, int stuffing_size, int tailer_size);
>>     
>
> Typo: tailer_size
>
>   

removed this redundent param

>> [...]
>>
>> Property changes on: pes.h
>> ___________________________________________________________________
>> Name: svn:executable
>>    + *
>>     
>
> Executable ?
>
>   
>> [...]
>>
>> --- mpegenc.c	(revision 9444)
>> +++ mpegenc.c	(working copy)
>>
>> [...]
>>
>> +        if(s->is_mpeg2) {
>> +            pes_context->packet_number = s->packet_number;
>> +            pes_context->is_ps = 1;
>> +            if (startcode == PRIVATE_STREAM_1) {
>> +                bytestream_put_byte(&p, id);
>> +                if (id >= 0xa0) {
>> +                    /* LPCM (XXX: check nb_frames) */
>> +                    bytestream_put_byte(&p, 7);
>> +                    bytestream_put_be16(&p, 4); /* skip 3 header bytes */
>>  
>> -        nb_frames= get_nb_frames(ctx, stream, payload_size - stuffing_size);
>> +                    bytestream_put_byte(&p, stream->lpcm_header[0]);
>> +                    bytestream_put_byte(&p, stream->lpcm_header[1]);
>> +                    bytestream_put_byte(&p, stream->lpcm_header[2]);
>> +                } else if (id >= 0x40) {
>> +                    /* AC3 */
>> +                    nb_frames= ff_get_nb_frames(ctx, stream, payload_size - stuffing_size);
>> +                    bytestream_put_byte(&p, nb_frames);
>> +                    bytestream_put_be16(&p, trailer_size+1);
>> +                }
>> +            }
>>     
>
> Current code handles PRIVATE_STREAM_1 even if !is_mpeg2, are you
> changing muxer behaviour here ?
>   

i moved this out of the judgement

>   
>> +        } else {
>>  
>> -        put_be32(&ctx->pb, startcode);
>> +            put_be32(&ctx->pb, startcode);
>>  
>> -        put_be16(&ctx->pb, packet_size);
>> +            put_be16(&ctx->pb, packet_size);
>>     
>
> Cosmetics (don't reindent code yet, reindent in next commit).
>   

fixed

>   
>> [...]
>>
>> +            /* output data */
>> +            if(av_fifo_generic_read(&pes_stream->fifo, payload_size - stuffing_size, &put_buffer, &ctx->pb) < 0) {
>> +                return -1;
>>              }
>>          }
>> -
>> -        /* output data */
>> -        if(av_fifo_generic_read(&stream->fifo, payload_size - stuffing_size, &put_buffer, &ctx->pb) < 0)
>> -            return -1;
>>     
>
> Again, don't reindent.
>   

fixed

>   
>> [...]
>>
>> --- pesenc.c	(revision 9444)
>> +++ pesenc.c	(working copy)
>>
>> [...]
>>
>>     
>  > -    if (pad_packet_bytes > 0)
>   
>> -        put_padding_packet(ctx,&ctx->pb, pad_packet_bytes);
>> +    /* special stuffing byte that is always written
>> +       to prevent accidental generation of start codes. */
>> +    put_byte(&ctx->pb, 0xff);
>>  
>> -    for(i=0;i<zero_trail_bytes;i++)
>> -        put_byte(&ctx->pb, 0x00);
>> +    for (i=0;i<stuffing_size;i++)
>> +        put_byte(&ctx->pb, 0xff);
>>  
>> -    put_flush_packet(&ctx->pb);
>> +     put_buffer(&ctx->pb, pes_content, pes_content_len);
>>  
>>     
>
> Can you try to not reindent the code and keep code in place, so we
> clearly see what is removed and what is keep ?
>   

fixed

>   
>> [...]
>>
>> +   /* output data */
>> +    if(av_fifo_generic_read(&stream->fifo, data_size, &put_buffer, &ctx->pb) < 0)
>> +        return -1;
>> +    return data_size;
>>     
>
> Isn't that duplicate of mpegenc.c ?
>   

we need to handle both mpeg1 and mpeg2 stream , here we only handle mpeg2


>   
>> [...]
>>                  break;
>>              }
>>              stream->buffer_index -= pkt_desc->size;
>> -
>>              stream->predecode_packet= pkt_desc->next;
>>              av_freep(&pkt_desc);
>>          }
>>      }
>> -
>>     
>
> Cosmectics.
>   

fixed

>   
>> [...]
>>
>>      for(i=0; i<ctx->nb_streams; i++){
>>          AVStream *st = ctx->streams[i];
>> -        StreamInfo *stream = st->priv_data;
>> +        PESStream*stream = st->priv_data;
>>     
>
> Space between Stream and stream please.
>
>   
>> [...]
>>  
>> -    if(best_i < 0){
>> +    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;
>> +            PESStream *stream = st->priv_data;
>>              PacketDesc *pkt_desc= stream->predecode_packet;
>>              if(pkt_desc && pkt_desc->dts < best_dts)
>>                  best_dts= pkt_desc->dts;
>> -        }
>> +    }
>>     
>
> Indentation problem ?
>   

fixed

>   
>> [...]
>>  
>>  #if 0
>>          av_log(ctx, AV_LOG_DEBUG, "bumping scr, scr:%f, dts:%f\n",
>>                 scr/90000.0, best_dts/90000.0);
>>  #endif
>> +
>>     
>
> Cosmetic.
>   

fixed

> [...]
>
>   

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: pes.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070629/30fb1dfa/attachment.asc>