On Mon, Mar 31, 2008 at 12:48:00AM +0800, zhentan feng wrote:
> [...]
>
> >
> > > + /*just consider dvd and mpeg2 for ff_pes_cal_header()*/
> > > + if(s->is_mpeg2){
> > > + stream->format = FMT_MPEG2;
> > > + if(s->is_dvd)
> > > + stream->format = FMT_DVD | FMT_MPEG2;
> > > + }
> > > id = stream->id;
> > >
> >
> > You must get rid of is_mpeg and is_dvd if you have FMT_*
> > Btw, after rethinking, PES_FMT_* is more appropriate.
> >
> well, I remove the assignments to "stream->format" to the function
> mpeg_mux_init().
> you can find it in the new patch.
> Since is_dvd and is_mpeg2 appear many times in mpeg_mux_init(), at the
> same time,PESStream haven't been initialized, so I think we cannot get
> rid of is_dvd and is_mpeg2.Moreover, they are variables in
> MpegMuxContext and may be used in somewhere else.
Having a format field in MpexMuxContext and in StreamInfo, is not that bad IMHO,
still it merges 5 fields into one, but yes, this could be done separately.
>
> > Furthermore, the new mechanism can be added separately, it is non
> > functionnal.
> > So this means the patch must be separate.
> >
> > > [...]
> > >
> > > Index: mpegpes.h
> > > ===================================================================
> > > --- mpegpes.h (revision 2048)
> > > +++ mpegpes.h (working copy)
> > > @@ -30,6 +30,12 @@
> > > #include "avformat.h"
> > > #include "fifo.h"
> > >
> > > +#define FMT_MPEG2 0x01
> > > +#define FMT_VCD 0x02
> > > +#define FMT_SVCD 0x04
> > > +#define FMT_DVD 0x08
> > > +#define FMT_TS 0x10
> > > +
> >
> > SVCD/DVD/TS are MPEG2 so you can add FMT_MPEG2 to constants.
> >
> > This will avoid some checks.
>
> fixed.
Well, why don't you always define TS as MPEG2 ?
> [...]
>
> +void ff_pes_cal_header(int id,PESStream * stream,
> + int *packet_size,int *header_len,int64_t *pts,int64_t *dts,
> + int *payload_size,int *startcode,int *stuffing_size,
> + int *trailer_size,int *pad_packet_bytes)
> +{
> + /* packet header size */
> + *packet_size -= 6;
>
> + /* packet header */
> + if (stream->format & PES_FMT_TS) {
> + *header_len = 3;
> + *header_len += 1; /* obligatory stuffing byte */
> + }else if (stream->format & PES_FMT_MPEG2){
> + *header_len = 3;
> + if (stream->packet_number==0)
> + *header_len += 3; /* PES extension */
> + *header_len += 1; /* obligatory stuffing byte */
> + } else {
> + *header_len = 0;
> + }
This can be merged in & PES_FMT_MPEG2 case.
> [...]
>
> + *pts=*dts=AV_NOPTS_VALUE;
> + *header_len -= timestamp_len;
> + if ((stream->format & PES_FMT_DVD) && stream->align_iframe){
> + *pad_packet_bytes += timestamp_len;
> + *packet_size -= timestamp_len;
> + }
> + else {
> + *payload_size += timestamp_len;
> + }
> + *stuffing_size += timestamp_len;
> + if(*payload_size > *trailer_size)
> + *stuffing_size += *payload_size - *trailer_size;
> + }
Indentation looks weird.
> [...]
>
> int pes_size;
> uint8_t* q = stream->payload;
> + pes_stream->format = PES_FMT_TS|PES_FMT_MPEG2;
>
See top comment.
[...]
--
Baptiste COUDURIER GnuPG Key Id: 0x5C1ABAAA
SMARTJOG SAS http://www.smartjog.com
Key fingerprint 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
Phone: +33 1 49966312
_______________________________________________
FFmpeg-soc mailing list
FFmpeg-soc@mplayerhq.hu
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
