[FFmpeg-devel] [PATCH] Encapsulate Dirac in MPEG TS.

[Anuradha Suraparaju]

Hi,

Resubmitting patch against svn revision 14260 with the recommended
changes.

On Wed, 2008-07-16 at 09:31 +0200, Benoit Fouet wrote:
> Hi,

[snip]

> > ------------------------------------------------------------------------
> >
> > Index: libavformat/mpegts.c
> > ===================================================================
> > --- libavformat/mpegts.c	(revision 14255)
> > +++ libavformat/mpegts.c	(working copy)
> > @@ -482,6 +482,7 @@
> >      int comp_page = 0, anc_page = 0; /* initialize to kill warnings */
> >      char language[4] = {0}; /* initialize to kill warnings */
> >      int has_hdmv_descr = 0;
> > +    int has_dirac_descr = 0;
> >  
> >  #ifdef DEBUG_SI
> >      av_log(ts->stream, AV_LOG_DEBUG, "PMT: len %i\n", section_len);
> > @@ -589,6 +590,18 @@
> >                  language[2] = get8(&p, desc_end);
> >                  language[3] = 0;
> >                  break;
> > +            case REGISTRATION_DESCRIPTOR: /*MPEG-2 Registration descriptor */
> > +                if (desc_len >= 4) {
> >   
> 
> this check seems unneeded.

Fixed. 

> 
> > +                    uint8_t bytes[4];
> > +                    bytes[0] = get8(&p, desc_end);
> > +                    bytes[1] = get8(&p, desc_end);
> > +                    bytes[2] = get8(&p, desc_end);
> > +                    bytes[3] = get8(&p, desc_end);
> > +                    if(bytes[0] == 'd' && bytes[1] == 'r' &&
> > +                       bytes[2] == 'a' && bytes[3] == 'c')
> > +                        has_dirac_descr = 1;
> > +                }
> > +                break;
> >              default:
> >                  break;
> >              }
> > @@ -610,12 +623,14 @@
> >          case STREAM_TYPE_VIDEO_MPEG4:
> >          case STREAM_TYPE_VIDEO_H264:
> >          case STREAM_TYPE_VIDEO_VC1:
> > +        case STREAM_TYPE_VIDEO_DIRAC:
> >          case STREAM_TYPE_AUDIO_AAC:
> >          case STREAM_TYPE_AUDIO_AC3:
> >          case STREAM_TYPE_AUDIO_DTS:
> >          case STREAM_TYPE_AUDIO_HDMV_DTS:
> >          case STREAM_TYPE_SUBTITLE_DVB:
> > -            if(stream_type == STREAM_TYPE_AUDIO_HDMV_DTS && !has_hdmv_descr)
> > +            if((stream_type == STREAM_TYPE_AUDIO_HDMV_DTS && !has_hdmv_descr)
> > +            || (stream_type == STREAM_TYPE_VIDEO_DIRAC && !has_dirac_descr))
> >   
> 
> nit: you can also align the && part
> 

Fixed.

> > Index: libavformat/mpegtsenc.c
> > ===================================================================
> > --- libavformat/mpegtsenc.c	(revision 14255)
> > +++ libavformat/mpegtsenc.c	(working copy)
> > @@ -220,6 +220,9 @@
> >          case CODEC_ID_H264:
> >              stream_type = STREAM_TYPE_VIDEO_H264;
> >              break;
> > +        case CODEC_ID_DIRAC:
> > +            stream_type = STREAM_TYPE_VIDEO_DIRAC;
> > +            break;
> >          case CODEC_ID_MP2:
> >          case CODEC_ID_MP3:
> >              stream_type = STREAM_TYPE_AUDIO_MPEG1;
> > @@ -267,6 +270,16 @@
> >                  put16(&q, 1); /* ancillary page id */
> >              }
> >              break;
> > +        case CODEC_TYPE_VIDEO:
> > +            if (stream_type == STREAM_TYPE_VIDEO_DIRAC) {
> > +                *q++ = 0x05; /*MPEG-2 registration descriptor*/
> > +                *q++ = 4;
> > +                *q++ = 0x64; /* drac */
> > +                *q++ = 0x72;
> > +                *q++ = 0x61;
> > +                *q++ = 0x63;
> >   
> 
> you wouldn't need this comment if you were using 'd', 'r', ...
> 

Fixed.

> > +            }
> > +            break;
> >          }
> >  
> >          val = 0xf000 | (q - desc_length_ptr - 2);
> > @@ -486,6 +499,7 @@
> >      uint8_t buf[TS_PACKET_SIZE];
> >      uint8_t *q;
> >      int val, is_start, len, header_len, write_pcr, private_code, flags;
> > +    int pes_extension = 0;
> >   
> 
> this declaration could be moved to where it is used.

Done.

> 
> >      int afc_len, stuffing_len;
> >      int64_t pcr = -1; /* avoid warning */
> >  
> > @@ -533,7 +547,10 @@
> >              *q++ = 0x01;
> >              private_code = 0;
> >              if (st->codec->codec_type == CODEC_TYPE_VIDEO) {
> > -                *q++ = 0xe0;
> > +                if (st->codec->codec_id == CODEC_ID_DIRAC) {
> > +                    *q++ = 0xfd;
> > +                } else
> > +                    *q++ = 0xe0;
> >              } else if (st->codec->codec_type == CODEC_TYPE_AUDIO &&
> >                         (st->codec->codec_id == CODEC_ID_MP2 ||
> >                          st->codec->codec_id == CODEC_ID_MP3)) {
> > @@ -554,6 +571,19 @@
> >                  header_len += 5;
> >                  flags |= 0x40;
> >              }
> > +            if (st->codec->codec_type == CODEC_TYPE_VIDEO &&
> > +                st->codec->codec_id == CODEC_ID_DIRAC) {
> > +                /* set PES_extension_flag */
> > +                pes_extension = 1;
> > +                flags |= 0x01;
> > +
> > +                /*
> > +                * One byte for PES2 extension flag +
> > +                * one byte for extension length +
> > +                * one byte for extension id
> > +                */
> > +                header_len += 3;
> > +            }
> >              len = payload_size + header_len + 3;
> >              if (private_code != 0)
> >                  len++;
> > @@ -574,6 +604,16 @@
> >                  write_pts(q, 1, dts);
> >                  q += 5;
> >              }
> > +            if (pes_extension && st->codec->codec_id == CODEC_ID_DIRAC) {
> > +                flags = 0x01;  /* set PES_extension_flag_2 */
> > +                *q++ = flags;
> > +                *q++ = 0x80 | 0x01;  /* marker bit + extension length */
> > +                /*
> > +                * Set the stream id extension flag bit to 0 and
> > +                * write the extended stream id
> > +                */
> > +                *q++ = 0x00 | 0x60;
> > +            }
> >              if (private_code != 0)
> >                  *q++ = private_code;
> >              is_start = 0;
> >   
> 
> -- 
> Benoit Fouet
> Purple Labs S.A.
> www.purplelabs.com

Regards,
Anuradha