[FFmpeg-soc] [FFmpeg-devel] [Patch]GSoC 2008 qualification task TS Muxer
zhentan feng
spyfeng at gmail.com
Tue Mar 25 16:18:44 CET 2008
2008/3/25, Michael Niedermayer <michaelni at gmx.at>:
> On Tue, Mar 25, 2008 at 11:57:16AM +0800, zhentan feng wrote:
> > 2008/3/25, Michael Niedermayer <michaelni at gmx.at>:
>
> [...]
>
> > I modified the codes and attached 2 new patches.
> >
> > The patch names "TSMuxerPatch_svn_dev" is the 4 files against
> > svn://svn.mplayerhq.hu/ffmpeg/trunk/libavformat
> > I think if the patch works correctly, it would be our expect.
>
>
> [...]
>
> Review done based on diffs relative to the correct ancestor files.
>
> > --- ffmpeg-svn/trunk/libavformat/mpegenc.c 2008-02-02 05:27:56.000000000 +0100
> > +++ mpegpes.h 2008-03-25 12:00:15.000000000 +0100
> [...]
> > +/**
> > + * PES packet description
> > + */
> > typedef struct PacketDesc {
> > int64_t pts;
> > int64_t dts;
> > @@ -39,10 +42,13 @@
> > struct PacketDesc *next;
> > } PacketDesc;
> >
> > +/**
> > + * PES stream structure
> > + */
> > typedef struct {
> > AVFifoBuffer fifo;
> > uint8_t id;
> > - int max_buffer_size; /* in bytes */
> > + int max_buffer_size; /**< in bytes */
> > int buffer_index;
> > PacketDesc *predecode_packet;
> > PacketDesc *premux_packet;
>
> These (and many other) changes are unrelated to factorizing the PES code out.
Thanks.
I have fixed it.
>
>
> > --- ffmpeg-svn/trunk/libavformat/mpegenc.c 2008-02-02 05:27:56.000000000 +0100
> > +++ mpegpesenc.c 2008-03-25 12:00:15.000000000 +0100
> [...]
> > switch(st->codec->codec_type) {
> > case CODEC_TYPE_AUDIO:
> > - if (st->codec->codec_id == CODEC_ID_AC3) {
> > - stream->id = ac3_id++;
> > - } else if (st->codec->codec_id == CODEC_ID_DTS) {
> > - stream->id = dts_id++;
> > - } else if (st->codec->codec_id == CODEC_ID_PCM_S16BE) {
> > - stream->id = lpcm_id++;
> > - for(j = 0; j < 4; j++) {
> > - if (lpcm_freq_tab[j] == st->codec->sample_rate)
> > - break;
> > + if(ps_audio_bound != 0||ps_video_bound != 0){
> > + if (st->codec->codec_id == CODEC_ID_AC3) {
> > + stream->id = ac3_id++;
> > + } else if (st->codec->codec_id == CODEC_ID_DTS) {
> > + stream->id = dts_id++;
> > + } else if (st->codec->codec_id == CODEC_ID_PCM_S16BE) {
> > + stream->id = lpcm_id++;
> > + for(j = 0; j < 4; j++) {
> > + if (lpcm_freq_tab[j] == st->codec->sample_rate)
> > + break;
>
> All reindentions as well as other cosmetic changes must be in one or more
> seperate patch(s)
I see :)
>
>
> [...]
>
> > @@ -1171,7 +480,6 @@
> > stream->next_packet= &pkt_desc->next;
> >
> > av_fifo_realloc(&stream->fifo, av_fifo_size(&stream->fifo) + size + 1);
> > -
> > if (s->is_dvd){
> > if (is_iframe && (s->packet_number == 0 || (pts - stream->vobu_start_pts >= 36000))) { // min VOBU length 0.4 seconds (mpucoder)
> > stream->bytes_to_iframe = av_fifo_size(&stream->fifo);
>
> please remove all useless cosmetic changes
fixed
>
>
>
>
> > Index: mpegenc.c
> > ===================================================================
> > --- mpegenc.c (revision 12559)
> > +++ mpegenc.c (working copy)
> [...]
> > @@ -181,7 +157,7 @@
> > int P_STD_max_mpeg_PS1 = 0;
> >
> > for(i=0;i<ctx->nb_streams;i++) {
> > - StreamInfo *stream = ctx->streams[i]->priv_data;
> > + PESStream *stream = ctx->streams[i]->priv_data;
>
> Why is the struct renamed?
> If there is some sense in this, then it must be in a seperate patch.
> If there is no sense in this then it should not be renamed at all.
>
> [...]
> > @@ -314,82 +292,17 @@
> >
> > s->audio_bound = 0;
> > s->video_bound = 0;
> > - mpa_id = AUDIO_ID;
> > - ac3_id = AC3_ID;
> > - dts_id = DTS_ID;
> > - mpv_id = VIDEO_ID;
> > - mps_id = SUB_ID;
> > - lpcm_id = LPCM_ID;
> > - for(i=0;i<ctx->nb_streams;i++) {
> > - st = ctx->streams[i];
> > - stream = av_mallocz(sizeof(StreamInfo));
> > - if (!stream)
> > - goto fail;
> > - st->priv_data = stream;
> > +
>
> trailing whitespace is forbidden in ffmpeg svn
fixed
>
>
> > + if(ff_pes_muxer_init(ctx,ps_audio_bound,ps_video_bound) != 0)
> > + goto fail;
> >
> [...]
> > @@ -621,26 +534,12 @@
> > put_byte(pb, 0xff);
> > }
> >
> > -static int get_nb_frames(AVFormatContext *ctx, StreamInfo *stream, int len){
> > - int nb_frames=0;
> > - PacketDesc *pkt_desc= stream->premux_packet;
> > -
> > - while(len>0){
> > - if(pkt_desc->size == pkt_desc->unwritten_size)
> > - nb_frames++;
> > - len -= pkt_desc->unwritten_size;
> > - pkt_desc= pkt_desc->next;
> > - }
> > -
> > - return nb_frames;
> > -}
> > -
> > /* flush the packet on stream stream_index */
> > static int flush_packet(AVFormatContext *ctx, int stream_index,
>
> [...]
>
> > if (packet_size > 0) {
> > + int ps_flag = 1;
> > + ff_pes_cal_header(ps_flag,s,id,stream,
> > + &packet_size,&header_len,&pts,&dts,
> > + &payload_size,&startcode,&stuffing_size,
> > + &trailer_size,&pad_packet_bytes);
> >
> > - /* packet header size */
> > - packet_size -= 6;
> > + nb_frames= ff_pes_get_nb_frames(ctx, stream, payload_size - stuffing_size);
> >
> > - /* packet header */
> > - if (s->is_mpeg2) {
> > - header_len = 3;
> > - if (stream->packet_number==0)
> > - header_len += 3; /* PES extension */
> [...]
> >
> > put_be16(ctx->pb, packet_size);
> > @@ -987,105 +814,19 @@
> > }
> > #endif
> >
> > -static int remove_decoded_packets(AVFormatContext *ctx, int64_t scr){
> > -// MpegMuxContext *s = ctx->priv_data;
> > - int i;
> > -
> > - for(i=0; i<ctx->nb_streams; i++){
> > - AVStream *st = ctx->streams[i];
> > - StreamInfo *stream = st->priv_data;
> > - PacketDesc *pkt_desc;
> > -
> > - while((pkt_desc= stream->predecode_packet)
> > - && scr > pkt_desc->dts){ //FIXME > vs >=
> > - if(stream->buffer_index < pkt_desc->size ||
> > - stream->predecode_packet == stream->premux_packet){
> > - av_log(ctx, AV_LOG_ERROR,
> > - "buffer underflow i=%d bufi=%d size=%d\n",
> > - i, stream->buffer_index, pkt_desc->size);
> > - break;
> > - }
> > - stream->buffer_index -= pkt_desc->size;
> > -
> > - stream->predecode_packet= pkt_desc->next;
> > - av_freep(&pkt_desc);
> > - }
> > - }
> > -
> > - return 0;
> > -}
> > -
> > static int output_packet(AVFormatContext *ctx, int flush){
> > MpegMuxContext *s = ctx->priv_data;
>
> It would be very nice if you could split the different factorizations of
> code into seperate patches.
I see :)
>
>
> [...]
>
> > +#define AUDIO_ID 0xc0
> > +#define VIDEO_ID 0xe0
> > +#define AC3_ID 0x80
> > +#define DTS_ID 0x8a
> > +#define LPCM_ID 0xa0
> > +#define SUB_ID 0x20
>
> code duplication
>
I will fix it.
>
> [...]
>
> >
> > /* prepare packet header */
> > q = buf;
> > *q++ = 0x47;
> > val = (ts_st->pid >> 8);
> > - if (is_start)
> > + if (is_start) {
> > val |= 0x40;
> > + is_start = 0;
> > + }
> > *q++ = val;
> > *q++ = ts_st->pid;
> > *q++ = 0x10 | ts_st->cc | (write_pcr ? 0x20 : 0);
> > ts_st->cc = (ts_st->cc + 1) & 0xf;
> > if (write_pcr) {
> > + /* add header and pcr bytes to pcr according to specs */
> > + pcr = ts->cur_pcr + (32+56) * 90000 / ts->mux_rate;
> > *q++ = 7; /* AFC length */
> > *q++ = 0x10; /* flags: PCR present */
> > *q++ = pcr >> 25;
> > @@ -524,59 +567,8 @@
> > *q++ = pcr >> 1;
> > *q++ = (pcr & 1) << 7;
> > *q++ = 0;
> > + ts->last_pcr = pcr;
> > }
>
> I assume these are bugfixes for the TS muxer? We certainly do want them but
> they must be in a seperate patch. Not in the patch spliting the common PES
> code out.
yes, these are bugfixes.But it was modified by the student last year,not me.
The situation is that I just download the codes from
svn://svn.mplayerhq.hu/soc/dvbmuxer
and I got 4 files mpegtsenc.c, mpegenc.c,mpegpesenc.c,mpegpes.h,which
are already bug
fixed for mpegtsenc.c against the svn-devl codes, and already
extracted some common codes.
According to baptiste.coudurie wrote in the early mail,I have just
extracted more common codes and do not do any functional changes.
As qualification task, I think I will do the next 2 things:
1.Test my codes , confirm it works correctly.
2.Create different patches for different situation:
i) create patches against svn://svn.mplayerhq.hu/soc/dvbmuxer to
show my changes from the last year's students.
ii) create patches against svn-devel codes to show the bug fixed
and common codes extracted from mpegtsenc.c and mpegenc.c to
mpegpesenc.c.
am I right?
thank you very much.
>
>
> [...]
>
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Frequently ignored awnser#1 FFmpeg bugs should be sent to our bugtracker. User
> questions about the command line tools should be sent to the ffmpeg-user ML.
> And questions about how to use libav* should be sent to the libav-user ML.
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.6 (GNU/Linux)
>
> iD8DBQFH6OSSYR7HhwQLD6sRAk+9AKCTQiebtxysgiwlCJI/MGRj8yeW8QCdHzmN
> iPdnWr7Y+hhjjF3vz8K8sGU=
> =1wM7
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
--
Best wishes~
More information about the FFmpeg-soc
mailing list