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

Michael Niedermayer michaelni
Thu Nov 4 01:16:33 CET 2010


On Tue, Nov 02, 2010 at 01:58:21PM +0300, Anatoly Nenashev wrote:
> On 01.11.2010 15:08, Michael Niedermayer wrote:
>> On Mon, Nov 01, 2010 at 02:55:26PM +0300, Anatoly Nenashev wrote:
>>    
>>> On 01.11.2010 14:35, Michael Niedermayer wrote:
>>>      
>>>> On Mon, Nov 01, 2010 at 02:06:25AM +0300, Anatoly Nenashev wrote:
>>>>
>>>>        
>>>>> On 01.11.2010 00:44, Michael Niedermayer wrote:
>>>>>
>>>>>          
>>>>>> On Sun, Oct 31, 2010 at 09:20:34PM +0300, Anatoly Nenashev wrote:
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>            
>>>>>>> @@ -195,6 +198,37 @@ int ff_mjpeg_decode_dht(MJpegDecodeContext *s)
>>>>>>>         return 0;
>>>>>>>     }
>>>>>>>
>>>>>>> +static int mjpeg_allocate_picture(MJpegDecodeContext *s)
>>>>>>> +{
>>>>>>> +    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, "mjpeg get_buffer() failed\n");
>>>>>>> +        return -1;
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int mxpeg_allocate_picture(MJpegDecodeContext *s)
>>>>>>> +{
>>>>>>> +    if (s->mxctx.reference_picture.data[0])
>>>>>>> +        s->avctx->release_buffer(s->avctx,&s->mxctx.reference_picture);
>>>>>>> +    s->mxctx.reference_picture = s->picture;
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>>
>>>>>>            
>>>>>>> +    s->picture.data[0] = s->picture.data[1] =
>>>>>>> +        s->picture.data[2] = s->picture.data[3] = 0;
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> the indention is odd
>>>>>>
>>>>>>
>>>>>>
>>>>>>            
>>>>> Just a feature of Emacs. Indention removed.
>>>>>
>>>>>
>>>>>          
>>>>>>
>>>>>>            
>>>>>>> +
>>>>>>> +    s->picture.reference= 1;
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>>
>>>>>>            
>>>>>>> +    if(s->avctx->get_buffer(s->avctx,&s->picture)<    0){
>>>>>>> +        av_log(s->avctx, AV_LOG_ERROR, "mxpeg get_buffer() failed\n");
>>>>>>> +        return -1;
>>>>>>> +    }
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> duplicate code
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>            
>>>>> May be not because "mxpeg_allocate_picture" used in two places:
>>>>> ff_mjpeg_decode_sof and mxpeg_decode_mxm.
>>>>>
>>>>>
>>>>>          
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>            
>>>>>>> @@ -788,14 +823,29 @@ static int mjpeg_decode_scan(MJpegDecodeContext *s, int nb_components, int Ah, i
>>>>>>>             }
>>>>>>>         }
>>>>>>>
>>>>>>> +    if (use_mxm_bitmask) {
>>>>>>> +        if (s->mb_width*s->mb_height>    (s->mxctx.mxm_bitmask_size<<    3)) {
>>>>>>> +            av_log(s->avctx, AV_LOG_ERROR, "MXM bitmask is not complete\n");
>>>>>>> +            return -1;
>>>>>>> +        }
>>>>>>> +
>>>>>>> +        init_get_bits(&mxm_bitmask_gb, s->mxctx.mxm_bitmask,
>>>>>>> +                      s->mxctx.mxm_bitmask_size<<    3);
>>>>>>> +
>>>>>>> +        s->picture.pict_type= FF_P_TYPE;
>>>>>>> +        s->picture.key_frame= 0;
>>>>>>> +    }
>>>>>>>         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);
>>>>>>> +            if (!reference_picture_avail&&    copy_mb) continue;
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> cant use_mxm_bitmask be 0 when !reference_picture_avail
>>>>>> (would avoid 1 check per mb)
>>>>>>
>>>>>>
>>>>>>
>>>>>>            
>>>>> Fixed. Conditions are switched in "if()".
>>>>>
>>>>>
>>>>>          
>>>>>> [...]
>>>>>>
>>>>>>
>>>>>>            
>>>>>>> @@ -1505,6 +1631,15 @@ not_the_end:
>>>>>>>     the_end:
>>>>>>>         av_log(avctx, AV_LOG_DEBUG, "mjpeg decode frame unused %td bytes\n", buf_end - buf_ptr);
>>>>>>>     //    return buf_end - buf_ptr;
>>>>>>> +
>>>>>>> +    if (s->avctx->codec_id == CODEC_ID_MXPEG&&
>>>>>>> +        !s->mxctx.has_complete_frame) {
>>>>>>> +        if (s->picture.key_frame)
>>>>>>> +            s->mxctx.has_complete_frame = 1;
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>>
>>>>>>            
>>>>>>> +        else if (!(s->avctx->flags&    CODEC_FLAG_LOW_DELAY))
>>>>>>> +            *data_size = 0;
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> mxpeg is always low delay, unless i misunderstand something
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>            
>>>>> Not absolutely so. Generally there's no I-Frame concept for MxPEG known
>>>>> from other codecs. Every encoded frame contains some parts of all image.
>>>>> All that we know is after fixed time delay we can collect a full puzzle.
>>>>> Actually some cameras send full JPEG  frame in connection and this
>>>>> feature is supported in attached patch but this is not that case on
>>>>> which it is necessary to rely. That's why if CODEC_FLAG_LOW_DELAY is
>>>>> disabled the decoder must collect a full image before return a first
>>>>> decoded frame. Otherwise, if CODEC_FLAG_LOW_DELAY is enabled we may see
>>>>> a frame full of holes at the start of played video.
>>>>>
>>>>>          
>>>> i see what you are trying to do but thats misusing that flag
>>>> i think you will have to add a new flag for this, and h264 could use that too
>>>>
>>>>
>>>>        
>>> Ok. I will add a new flag. It might be called CODEC_FLAG_COLLECT_FRAME
>>>      
>> CODEC_FLAG_RETURN_INCOMPLETE_FRAMES
>> unless someone has a better idea
>>
>>
>>    
>
> CODEC_FLAG2_RETURN_INCOMPLETE_FRAMES is defined in new patch

>  Makefile    |    1 
>  allcodecs.c |    1 
>  avcodec.h   |    2 
>  mjpegdec.c  |  187 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  mjpegdec.h  |   11 +++
>  options.c   |    1 
>  6 files changed, 189 insertions(+), 14 deletions(-)
> 4f468ef9bdc94b7c61d6446cde6cf3c9307b4a13  mxpeg_v4.patch
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 385ae02..7fb41a6 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -259,6 +259,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 89614ab..6104b53 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/avcodec.h b/libavcodec/avcodec.h
> index 705259e..e553c0a 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -249,6 +249,7 @@ enum CodecID {
>      CODEC_ID_A64_MULTI,
>      CODEC_ID_A64_MULTI5,
>      CODEC_ID_R10K,
> +    CODEC_ID_MXPEG,
>  
>      /* various PCM "codecs" */
>      CODEC_ID_PCM_S16LE= 0x10000,
> @@ -652,6 +653,7 @@ typedef struct RcOverride{
>  #define CODEC_FLAG2_PSY           0x00080000 ///< Use psycho visual optimizations.
>  #define CODEC_FLAG2_SSIM          0x00100000 ///< Compute SSIM during encoding, error[] values are undefined.
>  #define CODEC_FLAG2_INTRA_REFRESH 0x00200000 ///< Use periodic insertion of intra blocks instead of keyframes.
> +#define CODEC_FLAG2_RETURN_INCOMPLETE_FRAMES 0x00400000 ///< Allow decoder to return incomplete frames.
>  
>  /* Unsupported options :
>   *              Syntax Arithmetic coding (SAC)
> diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> index e57f74f..37c97ab 100644
> --- a/libavcodec/mjpegdec.c
> +++ b/libavcodec/mjpegdec.c
> @@ -112,6 +112,9 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx)
>      if (avctx->codec->id == CODEC_ID_AMV)
>          s->flipped = 1;
>  
> +    if (avctx->codec_id == CODEC_ID_MXPEG)
> +        memset(&s->mxctx, 0, sizeof(s->mxctx));
> +
>      return 0;
>  }
>  
> @@ -205,6 +208,37 @@ int ff_mjpeg_decode_dht(MJpegDecodeContext *s)
>      return 0;
>  }
>  
> +static int mjpeg_allocate_picture(MJpegDecodeContext *s)
> +{
> +    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, "mjpeg get_buffer() failed\n");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int mxpeg_allocate_picture(MJpegDecodeContext *s)
> +{
> +    if (s->mxctx.reference_picture.data[0])
> +        s->avctx->release_buffer(s->avctx, &s->mxctx.reference_picture);
> +    s->mxctx.reference_picture = s->picture;
> +    s->picture.data[0] = s->picture.data[1] =
> +    s->picture.data[2] = s->picture.data[3] = 0;
> +
> +    s->picture.reference= 1;
> +    if(s->avctx->get_buffer(s->avctx, &s->picture) < 0){
> +        av_log(s->avctx, AV_LOG_ERROR, "mxpeg get_buffer() failed\n");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>  int ff_mjpeg_decode_sof(MJpegDecodeContext *s)
>  {
>      int len, nb_components, i, width, height, pix_fmt_id;
> @@ -342,14 +376,12 @@ 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;
> +    if (s->avctx->codec_id == CODEC_ID_MXPEG && !s->got_picture) {
> +        if (mxpeg_allocate_picture(s) < 0) return -1;
> +    } else {
> +        if (mjpeg_allocate_picture(s) < 0) return -1;
>      }

looks like s->picture.reference will b wrong for the first pic


[...]
> +static int mxpeg_decode_mxm(MJpegDecodeContext *s, char *buf, int len)
> +{
> +    int32_t  mb_width, mb_height, bitmask_size, i;
> +    uint32_t mb_count;
> +
> +    mb_width  = AV_RL16(&buf[4]);
> +    mb_height = AV_RL16(&buf[6]);
> +    mb_count = (uint32_t)mb_width*mb_height;
> +
> +    if (!mb_count || mb_count > 0x7FFFFFF8) {
> +        av_log(s->avctx, AV_LOG_ERROR, "MXM wrong macroblocks count");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    bitmask_size = (int32_t)(mb_count + 7) >> 3;
> +
> +    if (bitmask_size > (len - 12)) {
> +        av_log(s->avctx, AV_LOG_ERROR, "MXM bitmask is not complete");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    av_freep(&s->mxctx.comment_buffer);
> +    s->mxctx.comment_buffer = buf;
> +    s->mxctx.mxm_bitmask = buf + 12;
> +
> +    if (!s->got_picture && s->picture.data[0]) {
> +        if (mxpeg_allocate_picture(s) < 0) return -1;
> +        s->got_picture = 1;
> +    }

allocating pictures in the comment parsing code is not a good idea IMHO

also maybe it would be easier to use reget_buffer() ?

[...]

> diff --git a/libavcodec/options.c b/libavcodec/options.c
> index ef7573c..6a8c24d 100644
> --- a/libavcodec/options.c
> +++ b/libavcodec/options.c
> @@ -88,6 +88,7 @@ static const AVOption options[]={
>  {"sgop", "strictly enforce gop size", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_STRICT_GOP, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"noout", "skip bitstream encoding", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_NO_OUTPUT, INT_MIN, INT_MAX, V|E, "flags2"},
>  {"local_header", "place global headers at every keyframe instead of in extradata", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_LOCAL_HEADER, INT_MIN, INT_MAX, V|E, "flags2"},
> +{"return_incomplete_frames", "allow decoder to return incomplete frames", 0, FF_OPT_TYPE_CONST, CODEC_FLAG2_RETURN_INCOMPLETE_FRAMES, INT_MIN, INT_MAX, V|D, "flags2"},
>  {"sub_id", NULL, OFFSET(sub_id), FF_OPT_TYPE_INT, DEFAULT, INT_MIN, INT_MAX},
>  {"me_method", "set motion estimation method", OFFSET(me_method), FF_OPT_TYPE_INT, ME_EPZS, INT_MIN, INT_MAX, V|E, "me_method"},
>  {"zero", "zero motion estimation (fastest)", 0, FF_OPT_TYPE_CONST, ME_ZERO, INT_MIN, INT_MAX, V|E, "me_method" },

that should be a seperate patch

[...]
--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101104/da5c5e77/attachment.pgp>



More information about the ffmpeg-devel mailing list