[FFmpeg-devel] [PATCH]pes packetizer

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

>>  }
>>  
>> -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

That said, the reviewing process might be easier if the changes are
staged, even if some intermediate stages are not fit for SVN.

>>  typedef struct {
>> -    AVFifoBuffer fifo;
>> +    PESStream pes_stream;
>>      uint8_t id;
>> -    int max_buffer_size; /* in bytes */
>> -    int buffer_index;
>> -    PacketDesc *predecode_packet;
>> -    PacketDesc *premux_packet;
>> -    PacketDesc **next_packet;
>>      int packet_number;
>>      uint8_t lpcm_header[3];
>>      int lpcm_align;
>
> is the lpcm stuff mpeg-PS specific? if not then it does belong in
> the PES stuff

The PCM stuff is DVD-specific.  IMHO extensions to the MPEG formats
like DVD and DVB should be explicitly marked as extensions and only
enabled when specifically requested.  We'd probably need a new way of
signalling this information though.

>> +/**
>> + * muxer type for PES
>> + */
>> +typedef enum {
>> +    PESMUXER_PS,
>> +    PESMUXER_TS,
>> +    PESMUXER_PES
>> +} PESMuxerType;
>> +
>
> the PES muxer should not need to know in what container the PES stream
> will be embeded

There are a few subtle differences.  For instance, video PES packets
with unspecified (coded zero) length are allowed in TS.

>> +/**
>> + * 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

It will compile in some compilers other than gcc.  The problem is that
what the compiler does with it is somewhat erratic, even between gcc
versions.

> also i think it would be cleaner to split the PES muxer into a proper
> AVOutputFormat

Isn't the PES packetiser just an intermediate stage common to MPEG PS
and TS?  How will the PS and TS muxers make use of it if it is a full
AVOutputFormat?

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