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

[Anatoly Nenashev]

On 04.11.2010 03:16, Michael Niedermayer wrote:
> On Tue, Nov 02, 2010 at 01:58:21PM +0300, Anatoly Nenashev wrote:
>    
>>   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
>
>
>    

No, s->picture.reference will be always 1 for MxPEG frames. But I've 
reimplemented this code to be more clean. See attachment.
> [...]
>    
>> +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
>    
I don't agree.
There are 3 types of frames in MxPEG stream:
1) clear JPEG which contains SOF, DHT, DQT, SOS  - may be the first 
frame in stream or (in new cameras firmware) may be periodically available.
2) Pseudo-I frame which contains SOF, DHT, DQT, MXM bitmask (in COM), 
SOS - must be periodically available but doesn't need to contain full 
parts of image.
3) P-frame which contains MXM bitmask, SOS and optional DHT, DQT - just 
usual frame.

So where I must allocate picture for the last frame type? I've decided 
to do it in MXM parser.


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

No, I think reget_buffer is a bad idea. We don't need to copy all 
reference frame but only non-changed part of it.
And we had talking about it in August. It was your idea to remove 
reget_buffer from this code to reject full image copying.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: mxpeg_v5.patch
Type: text/x-patch
Size: 13343 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101104/3d310841/attachment.bin>