[FFmpeg-devel] [PATCH 1/2] MxPEG decoder

Måns Rullgård mans
Mon Feb 14 12:51:17 CET 2011


Anatoly Nenashev <anatoly.nenashev at ovsoft.ru> writes:

> On 29.11.2010 16:12, Anatoly Nenashev wrote:
>> On 25.11.2010 18:26, Michael Niedermayer wrote:
>>> On Mon, Nov 08, 2010 at 01:40:39PM +0300, Anatoly Nenashev wrote:
>>>> [...]
>>>> I think I've found a solution for this issue. If input packet doesn't
>>>> contain SOF data then the new picture is allocated from
>>>> reference_picture which is initiated at decode_frame end. Thus
>>>> reference_picture is always good. For more details see attachment.
>>>
>>> the issue i described has not been fixed
>>> a invalid SOF still can lead to inconsistant values and your code
>>> still naively
>>> sets got_picture=1 indicating a valid SOF even if that is not so.
>>>
>>> Fundamentally i think the problem is that you write the code while
>>> ignoring
>>> security aspects entirely and expect review to find security issues.
>>> You should make sure your code is secure and no crafted input no
>>> matter how
>>> evil and malformed can lead to any crash or exploit before you
>>> submit your
>>> code.
>>>
>>>
>>> [...]
>>>
>>
>> I've reimplemented decoder to be more secure. There is additional
>> flag named "got_sof_data" which shows that SOF data is succesfully
>> parsed.
>> Also ugly picture reallocation removed.
>>
> Add dimensions check for current and reference picture.
> Patch tested under valgrind and on trashed stream.
>
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 3972dd9..5b9c27d 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -261,6 +261,7 @@ OBJS-$(CONFIG_MSMPEG4V3_ENCODER)       += msmpeg4.o msmpeg4data.o h263dec.o \
>  OBJS-$(CONFIG_MSRLE_DECODER)           += msrle.o msrledec.o
>  OBJS-$(CONFIG_MSVIDEO1_DECODER)        += msvideo1.o
>  OBJS-$(CONFIG_MSZH_DECODER)            += lcldec.o
> +OBJS-$(CONFIG_MXPEG_DECODER)           += mjpegdec.o mjpeg.o
>  OBJS-$(CONFIG_NELLYMOSER_DECODER)      += nellymoserdec.o nellymoser.o
>  OBJS-$(CONFIG_NELLYMOSER_ENCODER)      += nellymoserenc.o nellymoser.o
>  OBJS-$(CONFIG_NUV_DECODER)             += nuv.o rtjpeg.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index fbae0f6..1c0bfa9 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -150,6 +150,7 @@ void avcodec_register_all(void)
>      REGISTER_DECODER (MSRLE, msrle);
>      REGISTER_DECODER (MSVIDEO1, msvideo1);
>      REGISTER_DECODER (MSZH, mszh);
> +    REGISTER_DECODER (MXPEG, mxpeg);
>      REGISTER_DECODER (NUV, nuv);
>      REGISTER_ENCDEC  (PAM, pam);
>      REGISTER_ENCDEC  (PBM, pbm);
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index e57f74f..0fa800a 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -205,9 +205,73 @@ int ff_mjpeg_decode_dht(MJpegDecodeContext *s)
>      return 0;
>  }
>
> +static int mjpeg_allocate_picture(MJpegDecodeContext *s, AVFrame* picture_ptr,
> +                                  int key_frame, int reference_frame)

Style nit: please keep stars attached to variable names, like this:
AVFrame *picture_ptr

> +{
> +    int i;
> +
> +    if (picture_ptr->data[0])
> +        s->avctx->release_buffer(s->avctx, picture_ptr);
> +
> +    picture_ptr->reference = reference_frame;
> +    if (s->avctx->get_buffer(s->avctx, picture_ptr) < 0) {
> +        av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> +        return -1;
> +    }
> +    picture_ptr->pict_type = key_frame ? FF_I_TYPE : FF_P_TYPE;
> +    picture_ptr->key_frame = key_frame;
> +    s->got_picture = 1;
> +
> +    for (i = 0; i < MAX_COMPONENTS; i++) {
> +        s->linesize[i] = picture_ptr->linesize[i] << s->interlaced;
> +    }
> +}

Is this purely factored out of the code below?  If so, separating such
changes into a preparatory patch would make the main patch easier to
read.  If it's not too much trouble...

> +static int mxpeg_allocate_picture(MXpegDecodeContext *mxctx, MJpegDecodeContext* s)
> +{
> +    AVFrame *picture_ptr, *reference_ptr;
> +    if (s->first_picture) {
> +        av_log(s->avctx, AV_LOG_WARNING, "First picture has no SOF, skipping\n");
> +        return -1;
> +    }
> +    if (!mxctx->got_sof_data) {
> +        av_log(s->avctx, AV_LOG_WARNING, "Can not process SOS after broken SOF, skipping\n");
> +        return -1;
> +    }
> +    if (!mxctx->mxm_bitmask) {
> +        av_log(s->avctx, AV_LOG_WARNING, "Non-key frame has no MXM, skipping\n");
> +        return -1;
> +    }

Doing these checks in a function called allocate_picture seems a bit
odd.  I would expect a function with such a name to allocate a picture
and do things directly related to that, nothing more.  The checks should
of course be somewhere, but this is not the place I'd go looking for them.

> +    reference_ptr = &mxctx->pictures[mxctx->picture_index ^ 1];
> +    if (!reference_ptr->data[0]) {
> +        av_log(s->avctx, AV_LOG_WARNING, "No reference picture data for non-key frame, skipping\n");
> +        return -1;
> +    }
> +
> +    picture_ptr = &mxctx->pictures[mxctx->picture_index];
> +    if (mjpeg_allocate_picture(s, picture_ptr, 0, 1) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
>  int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>  {
>      int len, nb_components, i, width, height, pix_fmt_id;
> +    AVFrame *picture_ptr;
> +    MXpegDecodeContext *mxctx = (s->avctx->codec_id == CODEC_ID_MXPEG) ? s->avctx->priv_data : 0;

I'd prefer if you did this a bit differently.  More on that below.

> +    if (mxctx) {
> +        mxctx->got_sof_data = 0;
> +        if (s->lossless) {
> +            av_log(s->avctx, AV_LOG_ERROR, "Lossless mode doesn't available in MxPEG\n");

Bad grammar.

> +            return -1;
> +        }
> +        picture_ptr = &mxctx->pictures[mxctx->picture_index];
> +    } else {
> +        picture_ptr = &s->picture;
> +    }
>
>      /* XXX: verify len field validity */
>      len = get_bits(&s->gb, 16);
> @@ -282,8 +346,8 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>              s->height < ((s->org_height * 3) / 4)) {
>              s->interlaced = 1;
>              s->bottom_field = s->interlace_polarity;
> -            s->picture.interlaced_frame = 1;
> -            s->picture.top_field_first = !s->interlace_polarity;
> +            picture_ptr->interlaced_frame = 1;
> +            picture_ptr->top_field_first = !s->interlace_polarity;
>              height *= 2;
>          }
>
> @@ -342,20 +406,13 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>              s->avctx->pix_fmt = PIX_FMT_GRAY16;
>      }
>
> -    if(s->picture.data[0])
> -        s->avctx->release_buffer(s->avctx, &s->picture);
> -
> -    s->picture.reference= 0;
> -    if(s->avctx->get_buffer(s->avctx, &s->picture) < 0){
> -        av_log(s->avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> -        return -1;
> -    }
> -    s->picture.pict_type= FF_I_TYPE;
> -    s->picture.key_frame= 1;
> -    s->got_picture = 1;
> -
> -    for(i=0; i<3; i++){
> -        s->linesize[i]= s->picture.linesize[i] << s->interlaced;
> +    if (mxctx) {
> +        mxctx->got_sof_data = 1;
> +        if (mjpeg_allocate_picture(s, picture_ptr, 1, 1) < 0)
> +            return -1;
> +    } else {
> +        if (mjpeg_allocate_picture(s, picture_ptr, 1, 0) < 0)
> +            return -1;
>      }
>
>  //    printf("%d %d %d %d %d %d\n", s->width, s->height, s->linesize[0], s->linesize[1], s->interlaced, s->avctx->height);
> @@ -775,6 +832,17 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah, i
>      int i, mb_x, mb_y;
>      uint8_t* data[MAX_COMPONENTS];
>      int linesize[MAX_COMPONENTS];
> +    MXpegDecodeContext *mxctx = (s->avctx->codec_id == CODEC_ID_MXPEG) ? s->avctx->priv_data : 0;
> +    AVFrame *picture_ptr   = mxctx ? &mxctx->pictures[mxctx->picture_index] : &s->picture,
> +            *reference_ptr = mxctx ? &mxctx->pictures[mxctx->picture_index ^ 1] : 0;
> +    const int use_mxm_bitmask = mxctx && mxctx->mxm_bitmask;
> +    const int use_reference = reference_ptr && reference_ptr->data[0];
> +    GetBitContext mxm_bitmask_gb;
> +
> +    if (use_mxm_bitmask) {
> +        init_get_bits(&mxm_bitmask_gb, mxctx->mxm_bitmask,
> +                          mxctx->bitmask_size << 3);
> +    }
>
>      if(s->flipped && s->avctx->flags & CODEC_FLAG_EMU_EDGE) {
>          av_log(s->avctx, AV_LOG_ERROR, "Can not flip image with CODEC_FLAG_EMU_EDGE set!\n");
> @@ -782,7 +850,7 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah, i
>      }
>      for(i=0; i < nb_components; i++) {
>          int c = s->comp_index[i];
> -        data[c] = s->picture.data[c];
> +        data[c] = picture_ptr->data[c];
>          linesize[c]=s->linesize[c];
>          s->coefs_finished[c] |= 1;
>          if(s->flipped) {
> @@ -794,12 +862,15 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah, i
>
>      for(mb_y = 0; mb_y < s->mb_height; mb_y++) {
>          for(mb_x = 0; mb_x < s->mb_width; mb_x++) {
> +            const int copy_mb = use_mxm_bitmask && !get_bits1(&mxm_bitmask_gb);

Indexing the bit directly is probably faster than using get_bits.
Something like bitmask[idx>>3] & (128 >> (idx & 7)) should do it.
Also, what happens if the bitmask is smaller than the full frame?
If this is allowed, the easiest way to handle it is probably to allocate
a larger bitmask buffer and pad it with zeros.

> +            if (copy_mb && !use_reference) continue;
> +
>              if (s->restart_interval && !s->restart_count)
>                  s->restart_count = s->restart_interval;
>
>              for(i=0;i<nb_components;i++) {
>                  uint8_t *ptr;
> -                int n, h, v, x, y, c, j;
> +                int n, h, v, x, y, c, j, blk_offset;
>                  n = s->nb_blocks[i];
>                  c = s->comp_index[i];
>                  h = s->h_scount[i];
> @@ -807,11 +878,25 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah, i
>                  x = 0;
>                  y = 0;
>                  for(j=0;j<n;j++) {
> -                    ptr = data[c] +
> -                        (((linesize[c] * (v * mb_y + y) * 8) +
> -                        (h * mb_x + x) * 8) >> s->avctx->lowres);
> +                    blk_offset = (((linesize[c] * (v * mb_y + y) * 8) +
> +                                   (h * mb_x + x) * 8) >> s->avctx->lowres);
>                      if(s->interlaced && s->bottom_field)
> -                        ptr += linesize[c] >> 1;
> +                        blk_offset += linesize[c] >> 1;
> +                    ptr = data[c] + blk_offset;
> +                    if (copy_mb) {
> +                        const uint8_t *src = reference_ptr->data[c] + blk_offset;
> +                        switch (s->avctx->lowres) {
> +                        case 0: copy_block8(ptr, src, linesize[c], linesize[c], 8);
> +                            break;
> +                        case 1: copy_block4(ptr, src, linesize[c], linesize[c], 4);
> +                            break;
> +                        case 2: copy_block2(ptr, src, linesize[c], linesize[c], 2);
> +                            break;
> +                        case 3: *ptr = *src;
> +                            break;
> +                        default:;

Please, no useless switch cases like this.

> +                        }
> +                    } else {
>                      if(!s->progressive) {
>                          s->dsp.clear_block(s->block);
>                          if(decode_block(s, s->block, i,
> @@ -831,6 +916,7 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah, i
>                              return -1;
>                          }
>                      }
> +                    }
>  //                    av_log(s->avctx, AV_LOG_DEBUG, "mb: %d %d processed\n", mb_y, mb_x);
>  //av_log(NULL, AV_LOG_DEBUG, "%d %d %d %d %d %d %d %d \n", mb_x, mb_y, x, y, c, s->bottom_field, (v * mb_y + y) * 8, (h * mb_x + x) * 8);
>                      if (++x == h) {
> @@ -859,6 +945,14 @@ static int mjpeg_decode_scan_progressive_ac(MJpegDecodeContext *s, int ss, int s
>      int linesize = s->linesize[c];
>      int last_scan = 0;
>      int16_t *quant_matrix = s->quant_matrixes[ s->quant_index[c] ];
> +    MXpegDecodeContext *mxctx = (s->avctx->codec_id == CODEC_ID_MXPEG) ? s->avctx->priv_data : 0;
> +
> +    if (mxctx) {
> +        //FIXME implement progressive mode for MxPEG if needed (no samples available now)
> +        av_log(s->avctx, AV_LOG_ERROR,
> +               "Progressive mode doesn't implemented in MxPEG decoder\n");
> +        return -1;
> +    }
>
>      if(!Al) {
>          s->coefs_finished[c] |= (1LL<<(se+1))-(1LL<<ss);
> @@ -900,6 +994,7 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s)
>      int index, id;
>      const int block_size= s->lossless ? 1 : 8;
>      int ilv, prev_shift;
> +    MXpegDecodeContext *mxctx = (s->avctx->codec_id == CODEC_ID_MXPEG) ? s->avctx->priv_data : 0;
>
>      /* XXX: verify len field validity */
>      len = get_bits(&s->gb, 16);
> @@ -968,6 +1063,13 @@ int ff_mjpeg_decode_sos(MJpegDecodeContext *s)
>          s->v_scount[0] = 1;
>      }
>
> +    /* compare picture size in macroblocks from SOF and MXM (MxPEG decoder) */
> +    if (mxctx && mxctx->mxm_bitmask &&
> +        (s->mb_width != mxctx->mb_width || s->mb_height != mxctx->mb_height)) {
> +        av_log(s->avctx, AV_LOG_ERROR, "Picture size in MB's from SOF and MXM doesn't equal\n");
> +        return AVERROR(EINVAL);
> +    }
> +
>      if(s->avctx->debug & FF_DEBUG_PICT_INFO)
>          av_log(s->avctx, AV_LOG_DEBUG, "%s %s p:%d >>:%d ilv:%d bits:%d %s\n", s->lossless ? "lossless" : "sequential DCT", s->rgb ? "RGB" : "",
>                 predictor, point_transform, ilv, s->bits,
> @@ -1168,6 +1270,49 @@ out:
>      return 0;
>  }
>
> +static int mxpeg_decode_mxm(AVCodecContext *avctx, MXpegDecodeContext *mxctx,
> +                            char *buf, int len)
> +{
> +    int32_t  bitmask_size, i;
> +    uint32_t mb_count;

Why do you use sized types here?  For local variables it is almost
always better to use plain int or unsigned int types.

> +    av_freep(&mxctx->comment_buffer);
> +    mxctx->comment_buffer = buf;
> +    mxctx->mxm_bitmask = buf + 12;
> +
> +    mxctx->mb_width  = AV_RL16(&buf[4]);
> +    mxctx->mb_height = AV_RL16(&buf[6]);
> +    mb_count = (uint32_t)mxctx->mb_width * mxctx->mb_height;

Why the cast?

> +    if (!mb_count || mb_count > 0x7FFFFFF8) {

This looks strange.  What condition are you actually trying to check for?

> +        av_log(avctx, AV_LOG_ERROR, "MXM wrong macroblocks count");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    bitmask_size = (mb_count + 7) >> 3;
> +    if (bitmask_size > (len - 12)) {
> +        av_log(avctx, AV_LOG_ERROR, "MXM bitmask is not complete");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (mxctx->bitmask_size != bitmask_size) {
> +        av_freep(&mxctx->completion_bitmask);
> +        mxctx->completion_bitmask = av_mallocz(bitmask_size);
> +        if (!mxctx->completion_bitmask)
> +            return AVERROR(ENOMEM);
> +        mxctx->bitmask_size = bitmask_size;
> +    }

This will crash if the allocation fails and the next call has the same
value for bitmask_size.

> +    if (!mxctx->has_complete_frame) {
> +        mxctx->has_complete_frame = 1;
> +        for (i = 0; i < bitmask_size; ++i) {
> +            mxctx->completion_bitmask[i] |= mxctx->mxm_bitmask[i];
> +            mxctx->has_complete_frame &= !(uint8_t)~mxctx->completion_bitmask[i];

It would be clearer (and faster) to simply & all the bits in the loop
and compare to 255 at the end.

> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  static int mjpeg_decode_com(MJpegDecodeContext *s)
>  {
>      int len = get_bits(&s->gb, 16);
> @@ -1198,8 +1343,14 @@ static int mjpeg_decode_com(MJpegDecodeContext *s)
>              else if((len > 20 && !strncmp(cbuf, "Intel(R) JPEG Library", 21)) ||
>                      (len > 19 && !strncmp(cbuf, "Metasoft MJPEG Codec", 20))){
>                  s->flipped = 1;
> +            } else if (s->avctx->codec_id == CODEC_ID_MXPEG &&
> +                       !strncmp(cbuf, "MXM", 3)){
> +                int ret = mxpeg_decode_mxm(s->avctx, s->avctx->priv_data,
> +                                           cbuf, len - 2);
> +                if (ret < 0)
> +                    return ret;
> +                return 0;

I would rather add "int ret = 0" at the top of the function, "return ret"
at the end, and "cbuf = NULL" here to prevent it being released.

The call to mjpeg_decode_com() also needs to have error checking added.

>              }
> -
>              av_free(cbuf);
>          }
>      }
> @@ -1260,6 +1411,23 @@ found:
>      return val;
>  }
>
> +static int mxpeg_check_dimensions(MXpegDecodeContext *mxctx)
> +{
> +    AVFrame *picture_ptr = &mxctx->pictures[mxctx->picture_index],
> +        *reference_ptr = &mxctx->pictures[mxctx->picture_index ^ 1];

Style nit: I think making each of those a separate declaration would be
more readable:

    AVFrame *picture_ptr   = &mxctx->pictures[mxctx->picture_index];
    AVFrame *reference_ptr = &mxctx->pictures[mxctx->picture_index ^ 1];

> +    int i;
> +
> +    if (mxctx->mxm_bitmask && reference_ptr->data[0]) {
> +        for (i = 0; i < MAX_COMPONENTS; ++i) {
> +            if ( (!reference_ptr->data[i] ^ !picture_ptr->data[i]) ||
> +                 reference_ptr->linesize[i] != picture_ptr->linesize[i])
> +                return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
>  int ff_mjpeg_decode_frame(AVCodecContext *avctx,
>                                void *data, int *data_size,
>                                AVPacket *avpkt)
> @@ -1267,11 +1435,14 @@ int ff_mjpeg_decode_frame(AVCodecContext *avctx,
>      const uint8_t *buf = avpkt->data;
>      int buf_size = avpkt->size;
>      MJpegDecodeContext *s = avctx->priv_data;
> +    MXpegDecodeContext *mxctx = (avctx->codec_id == CODEC_ID_MXPEG) ? avctx->priv_data : 0;
>      const uint8_t *buf_end, *buf_ptr;
>      int start_code;
>      AVFrame *picture = data;
>
>      s->got_picture = 0; // picture from previous image can not be reused
> +    if (mxctx) mxctx->mxm_bitmask = 0;

NULL

>      buf_ptr = buf;
>      buf_end = buf + buf_size;
>      while (buf_ptr < buf_end) {
> @@ -1445,8 +1616,20 @@ eoi_parser:
>                              if (s->bottom_field == !s->interlace_polarity)
>                                  goto not_the_end;
>                          }
> -                        *picture = s->picture;
>                          *data_size = sizeof(AVFrame);
> +                        if (mxctx) {
> +                            *picture = mxctx->pictures[mxctx->picture_index];
> +                            mxctx->picture_index ^= 1;
> +                            if (!mxctx->has_complete_frame) {
> +                                if (!mxctx->mxm_bitmask) {
> +                                    mxctx->has_complete_frame = 1;
> +                                } else {
> +                                    *data_size = 0;
> +                                }
> +                            }
> +                        } else {
> +                            *picture = s->picture;
> +                        }
>
>                          if(!s->lossless){
>                              picture->quality= FFMAX3(s->qscale[0], s->qscale[1], s->qscale[2]);
> @@ -1463,9 +1646,21 @@ eoi_parser:
>                      break;
>                  case SOS:
>                      if (!s->got_picture) {
> -                        av_log(avctx, AV_LOG_WARNING, "Can not process SOS before SOF, skipping\n");
> -                        break;
> +                        if (mxctx) {
> +                            if (mxpeg_allocate_picture(mxctx, s) < 0)
> +                                return -1;
> +                        } else {
> +                            av_log(avctx, AV_LOG_WARNING,"Can not process SOS before SOF, skipping\n");
> +                            break;
> +                        }
>                      }
> +
> +                    if (mxctx && mxpeg_check_dimensions(mxctx) < 0) {
> +                        av_log(avctx, AV_LOG_WARNING,
> +                               "Dimensions of reference picture don't correspond to given SOF, skipping\n");
> +                        return -1;
> +                    }
> +
>                      ff_mjpeg_decode_sos(s);
>                      /* buggy avid puts EOI every 10-20th frame */
>                      /* if restart period is over process EOI */
> @@ -1515,6 +1710,7 @@ the_end:
>  av_cold int ff_mjpeg_decode_end(AVCodecContext *avctx)
>  {
>      MJpegDecodeContext *s = avctx->priv_data;
> +    MXpegDecodeContext *mxctx = (avctx->codec_id == CODEC_ID_MXPEG) ? avctx->priv_data : 0;
>      int i, j;
>
>      if (s->picture.data[0])
> @@ -1533,6 +1729,16 @@ av_cold int ff_mjpeg_decode_end(AVCodecContext *avctx)
>          av_freep(&s->blocks[i]);
>          av_freep(&s->last_nnz[i]);
>      }
> +
> +    if (mxctx) {
> +        for (i = 0; i < 2; i++) {
> +            if (mxctx->pictures[i].data[0])
> +                avctx->release_buffer(avctx, &mxctx->pictures[i]);
> +        }
> +        av_freep(&mxctx->comment_buffer);
> +        av_freep(&mxctx->completion_bitmask);
> +    }
> +
>      return 0;
>  }
>
> @@ -1565,3 +1771,18 @@ AVCodec thp_decoder = {
>      .max_lowres = 3,
>      .long_name = NULL_IF_CONFIG_SMALL("Nintendo Gamecube THP video"),
>  };
> +
> +AVCodec mxpeg_decoder = {
> +    "mxpeg",
> +    AVMEDIA_TYPE_VIDEO,
> +    CODEC_ID_MXPEG,
> +    sizeof(MXpegDecodeContext),
> +    ff_mjpeg_decode_init,
> +    NULL,
> +    ff_mjpeg_decode_end,
> +    ff_mjpeg_decode_frame,
> +    CODEC_CAP_DR1,
> +    NULL,
> +    .max_lowres = 3,
> +    .long_name = NULL_IF_CONFIG_SMALL("Mobotix MxPEG video"),
> +};
> diff --git a/libavcodec/mjpegdec.h b/libavcodec/mjpegdec.h
> index 7baa5dc..24597d5 100644
> --- a/libavcodec/mjpegdec.h
> +++ b/libavcodec/mjpegdec.h
> @@ -107,6 +107,19 @@ typedef struct MJpegDecodeContext {
>      unsigned int ljpeg_buffer_size;
>  } MJpegDecodeContext;
>
> +typedef struct MXpegDecodeContext {
> +    MJpegDecodeContext jpgctx;
> +    int got_sof_data; /* true if SOF data successfully parsed */
> +    uint8_t *comment_buffer; /* temporary comment buffer */
> +    uint8_t *mxm_bitmask; /* bitmask buffer */
> +    int32_t bitmask_size; /* size of bitmask */
> +    uint8_t has_complete_frame; /* true if has complete frame */
> +    uint8_t *completion_bitmask; /* completion bitmask of macroblocks */
> +    int mb_width, mb_height; /* size of picture in MB's from MXM header */

Are these different from the fields with the same names in MJpegDecodeContext?

> +    AVFrame pictures[2]; /* pictures array */
> +    int picture_index; /* index of current picture, other picture is reference */
> +} MXpegDecodeContext;

I would add these, apart from pictures[], to the end of
MJpegDecodeContext and make the existing picture there an array of two
instead.

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



More information about the ffmpeg-devel mailing list