[FFmpeg-devel] [PATCH] Mobotix .mxg demixer and MxPEG decoder basic implementation

[Anatoly Nenashev]

On 08.08.2010 16:47, Reimar D?ffinger wrote:
> On Sun, Aug 08, 2010 at 01:07:07PM +0400, Anatoly Nenashev wrote:
>    
>> diff -r 02ffe42e2ae2 libavcodec/Makefile
>> --- a/libavcodec/Makefile	Sun Aug 01 14:52:28 2010 +0400
>> +++ b/libavcodec/Makefile	Sun Aug 08 12:56:11 2010 +0400
>> [...]
>>      
> This belongs into one patch together with the actual decoder changes.
>
>    

Now all changes are all together in one attached patch.


>
>
>> +    dts = get_le64(s->pb); /* get dts in useconds */
>> +    dts /= 10000; /*convert dts to 0.01 seconds timebase*/
>>      
> That's wrong, set the time_base correctly instead.
>
>    

This is done such way due to function tb_unreliable(...) in 
libavformat/utils.c:2146

static int tb_unreliable(AVCodecContext *c){
     if(   c->time_base.den >= 101L*c->time_base.num
        || c->time_base.den <    5L*c->time_base.num
        || c->codec_id == CODEC_ID_MPEG2VIDEO
        || c->codec_id == CODEC_ID_H264
        )
         return 1;
     return 0;
}

time_base must be between 1/5 sec and 1/100 sec. I don't know other way 
to support variable frame rate in video stream.


>
>
>> +    end_pos = url_ftell(s->pb);
>> +    size = end_pos - start_pos;
>> +
>> +    url_fseek(s->pb, -size, SEEK_CUR);
>>      
> This is a really bad idea and will break with streaming,
> e.g.
> cat file | ffmpeg -i -
>
>    

Changed in  new patch.

> [...]
>> +        case EOI:
>> +            if (VIDEO_STREAM_INDEX == stream_index) {
>> +                ret = mxg_create_video_packet(s, start_pos, is_key,
>> +                                              video_dts, pkt);
>> +                break;
>> +            }
>> +            continue;
>>      
> Splitting into individual frames really does not belong into the demuxer,
> it should be done in a parser.
> A demuxer should only do things like splitting audio and video,
> and if available process pts, dts, metadata, keyframe flags etc.
> This is doubly true for JPEG where I think there are still issues
> with interlaced JPEG and this could not benefit from
> any fixes to that.
>
>    

MxPEG parser added in mjpeg_parser.c

>
>
>> @@ -331,18 +336,28 @@
>> [...]
>> -    s->picture.pict_type= FF_I_TYPE;
>> -    s->picture.key_frame= 1;
>>       s->got_picture = 1;
>>
>> +    if (!s->mxctx.mxm_bitmask) {
>> +        s->picture.pict_type= FF_I_TYPE;
>> +        s->picture.key_frame= 1;
>> +    }
>>      
> Do not reindent code that is not otherwise changed, it just
> makes the patch hard to read.
>
>    

Ok. But in this place it makes program code hard to read.

>> @@ -789,6 +804,14 @@
>>   
> [...]
> Also
>    
>> +                int mb_selected = (s->mxctx.mxm_bitmask[mb_index>>  3]<<  (mb_index&  7)) 0x80;
>>      
> is likely to be faster.
>
>    

Changed in new patch.


Anatoly.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: mxpeg_v1.patch
Type: text/x-patch
Size: 14414 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100811/6c5832e4/attachment.bin>