[FFmpeg-devel] [PATCH]pes packetizer

[Xiaohui Sun]

Michael Niedermayer wrote:
> On Fri, Aug 31, 2007 at 05:43:35PM +0800, Xiaohui Sun wrote:
> [...]
>   
>>> [...]
>>>
>>>   
>>>       
>>>> +    put_byte(&ctx->pb, 0x80); /* mpeg2 id */
>>>>  -        if (!s->is_mpeg2)
>>>> -            for(i=0;i<stuffing_size;i++)
>>>> -                put_byte(&ctx->pb, 0xff);
>>>> -
>>>> -        if (s->is_mpeg2) {
>>>> -            put_byte(&ctx->pb, 0x80); /* mpeg2 id */
>>>>     
>>>>         
>>> as already said in past reviews, this is a cosmetic change and does not
>>> belong in here, please do not reindent the code
>>>   
>>>       
>> fixed
>>     
>
> no, its not fixed:
>   

I misunderstood that, this time fixed

>   
>> +    put_byte(&ctx->pb, 0x80); /* mpeg2 id */
>>  
>> -        if (!s->is_mpeg2)
>> -            for(i=0;i<stuffing_size;i++)
>> -                put_byte(&ctx->pb, 0xff);
>> -
>> -        if (s->is_mpeg2) {
>> -            put_byte(&ctx->pb, 0x80); /* mpeg2 id */
>>     
>
>
> [...]
>
>   
>>>>  }
>>>>  -static void put_vcd_padding_sector(AVFormatContext *ctx)
>>>> +int ff_pes_remove_decoded_packets(AVFormatContext *ctx, int64_t scr)
>>>>     
>>>>         
>>> [...]
>>>   
>>>       
>>>> -static int remove_decoded_packets(AVFormatContext *ctx, int64_t scr){
>>>>     
>>>>         
>>> as ive said in past reviews
>>> please put this function rename in a seperate patch
>>>   
>>>       
>> these two function has no relation, and I don't know why they were diffed 
>> against each other...
>>     
>
> these fuctions are identical
>
> --- old	2007-08-31 20:13:30.000000000 +0200
> +++ new	2007-08-31 20:13:42.000000000 +0200
> @@ -1,10 +1,10 @@
> -static int remove_decoded_packets(AVFormatContext *ctx, int64_t scr){
> -//    MpegMuxContext *s = ctx->priv_data;
> +int ff_pes_remove_decoded_packets(AVFormatContext *ctx, int64_t scr)
> +{
>      int i;
>  
>      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;
>  
>          while((pkt_desc= stream->predecode_packet)
>
>   

yes I used the original function name except adding a ff_ prefix

> [...]
>   
>>  }
>>  
>> -static inline void put_timestamp(ByteIOContext *pb, int id, int64_t timestamp)
>> +inline void put_timestamp(ByteIOContext *pb, int id, int64_t timestamp)
>>  {
>>     
>
> this lacks a ff_ prefix now, which would make the code after this patch
> broken so we cannot apply it, svn must always be working
> commiting broken code with the intent to correct it at some unspecified
> point in the future is not good
> its better not to break it in the first place
>
>   

fixed

[...]
>
>>      id = stream->id;
>> -
>>  #if 0
>>      printf("packet ID=%2x PTS=%0.3f\n",
>>             id, pts / 90000.0);
>>     
>
> cosmetic
>   

fixed

>
> [...]
>   
>>  
>> -        nb_frames= get_nb_frames(ctx, stream, payload_size - stuffing_size);
>>     
> [...]
>   
>> +                nb_frames= get_nb_frames(ctx, stream, payload_size - stuffing_size);
>>     
>
> cosmetic
>
>
>   

fixed
[...]
>> +/**
>> + * 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
>> + */
>> +inline void put_timestamp(ByteIOContext *pb, int id, int64_t timestamp);
>>     
>
> this wont compile on a non gnu c compiler
>
>
>   

removed the inline

> also i think it would be cleaner to split the PES muxer into a proper
> AVOutputFormat
>
> [...]
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> http://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: pes.diff
Type: text/x-patch
Size: 68329 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070901/7efee280/attachment.bin>