[FFmpeg-devel] Patch: add AVOption for extra video descriptors in MPEGTS PMT

[Michael Niedermayer]

On Mon, May 21, 2012 at 10:06:17PM -0400, Jason Livingston wrote:
> Hi everyone,
> This is my first patch, I hope I followed the rules correctly. :)
> Looking forward to your comments.
> 
> Goal: Be able to add extra video descriptors to the MPEGTS PMT as a
> runtime option
> 
> Summary of patch:  Add a new AVOption "extra_video_descriptors"
> (string type, default NULL) to mpegtsenc.c, which can contain a
> BASE64-encoded string of extra descriptor data (since this data is
> variable length and can contain zeros at any point, a BASE64 encoded
> string seemed to be the best option).  If this AVOption is not set,
> then there will be no change to the output.  If the AVOption is set to
> a valid BASE64 encoded string, it will be BASE64 decoded and the bytes
> inserted into the PMTs generated by mpegts_write_pmt().
> 

> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
> index 36e958f..fae0ac7 100644
> --- a/libavformat/mpegtsenc.c
> +++ b/libavformat/mpegtsenc.c
> @@ -25,6 +25,7 @@
>  #include "libavutil/mathematics.h"
>  #include "libavutil/opt.h"
>  #include "libavutil/avassert.h"
> +#include "libavutil/base64.h"
>  #include "libavcodec/mpegvideo.h"
>  #include "avformat.h"
>  #include "internal.h"
> @@ -80,6 +81,8 @@ typedef struct MpegTSWrite {
>      int m2ts_mode;
>  
>      int reemit_pat_pmt;
> +    
> +    char *extra_video_descriptors;
>  } MpegTSWrite;
>  
>  /* a PES packet header is generated every DEFAULT_PES_HEADER_FREQ packets */
> @@ -105,6 +108,8 @@ static const AVOption options[] = {
>        offsetof(MpegTSWrite, pes_payload_size), AV_OPT_TYPE_INT, {DEFAULT_PES_PAYLOAD_SIZE}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
>      { "resend_headers", "Reemit PAT/PMT before writing the next packet",
>        offsetof(MpegTSWrite, reemit_pat_pmt), AV_OPT_TYPE_INT, {0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
> +    { "extra_video_descriptors", "BASE64 encoded string of extra video descriptor bytes",
> +      offsetof(MpegTSWrite, extra_video_descriptors), AV_OPT_TYPE_STRING, {.str = NULL}, 0, 0, AV_OPT_FLAG_ENCODING_PARAM},
>      { NULL },
>  };
>  
> @@ -242,7 +247,7 @@ static void mpegts_write_pat(AVFormatContext *s)
>  
>  static void mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
>  {
> -    //    MpegTSWrite *ts = s->priv_data;
> +    MpegTSWrite *ts = s->priv_data;
>      uint8_t data[1012], *q, *desc_length_ptr, *program_info_length_ptr;
>      int val, stream_type, i;
>  
> @@ -377,6 +382,18 @@ static void mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
>                  *q++ = 'a';
>                  *q++ = 'c';
>              }
> +            if (ts->extra_video_descriptors) {
> +                int evd_data_len = strlen(ts->extra_video_descriptors) * 3 / 4;
> +                if (evd_data_len > 0) {
> +                    uint8_t *evd_data = av_malloc(evd_data_len);
> +                    val = av_base64_decode(evd_data, ts->extra_video_descriptors, evd_data_len);
> +                    if (val > 0) {
> +                        memcpy(q, evd_data, val);

this looks like a buffer overflow to me
theres nothing checking that q is large enough for the user specified
data that is put into it

also do you have a testcase for this feature ?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120523/e3b6e26d/attachment.asc>