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

[Michael Niedermayer]

On Wed, Aug 11, 2010 at 12:18:11AM +0400, Anatoly Nenashev wrote:
> 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.

seconded, this is a "must fix" for approval


[...]


> diff -r 02ffe42e2ae2 libavcodec/avcodec.h
> --- a/libavcodec/avcodec.h	Sun Aug 01 14:52:28 2010 +0400
> +++ b/libavcodec/avcodec.h	Tue Aug 10 23:42:47 2010 +0400
> @@ -73,6 +73,7 @@
>      CODEC_ID_MJPEG,
>      CODEC_ID_MJPEGB,
>      CODEC_ID_LJPEG,
> +    CODEC_ID_MXPEG,
>      CODEC_ID_SP5X,
>      CODEC_ID_JPEGLS,
>      CODEC_ID_MPEG4,


breaks abi


> diff -r 02ffe42e2ae2 libavcodec/mjpeg_parser.c
> --- a/libavcodec/mjpeg_parser.c	Sun Aug 01 14:52:28 2010 +0400
> +++ b/libavcodec/mjpeg_parser.c	Tue Aug 10 23:42:47 2010 +0400
> @@ -26,6 +26,7 @@
>   * MJPEG parser.
>   */
>  
> +#include "libavutil/intreadwrite.h"
>  #include "parser.h"
>  
>  

> @@ -70,6 +71,22 @@
>      return END_NOT_FOUND;
>  }
>  
> +static int64_t mxpeg_get_timestamp(const uint8_t *buf, int buf_size)
> +{
> +    int i;
> +    uint16_t state = 0;
> +    int64_t dts = AV_NOPTS_VALUE;
> +    for (i = 0; i < buf_size - 18; ++i) {
> +        state = (state<<8) | buf[i];
> +        if ((state == 0xFFFE) && (AV_RL32(buf+i+3) == MKTAG('M','X','F',0))) {
> +            dts = av_rescale(AV_RL64(buf+i+11), 1, 10000);
> +            break;
> +        }
> +    }
> +
> +    return dts;
> +}

this looks wrong and inefficient
i dont belive scaning the buffer like this is the correct way


> +
>  static int jpeg_parse(AVCodecParserContext *s,
>                        AVCodecContext *avctx,
>                        const uint8_t **poutbuf, int *poutbuf_size,
> @@ -92,12 +109,16 @@
>  
>      *poutbuf = buf;
>      *poutbuf_size = buf_size;
> +    if (avctx->codec_id == CODEC_ID_MXPEG) {
> +        s->dts = mxpeg_get_timestamp(buf, buf_size);
> +    }
> +
>      return next;
>  }
>  
>  
>  AVCodecParser mjpeg_parser = {
> -    { CODEC_ID_MJPEG },
> +    { CODEC_ID_MJPEG, CODEC_ID_MXPEG },
>      sizeof(ParseContext),
>      NULL,
>      jpeg_parse,

> diff -r 02ffe42e2ae2 libavcodec/mjpegdec.c
> --- a/libavcodec/mjpegdec.c	Sun Aug 01 14:52:28 2010 +0400
> +++ b/libavcodec/mjpegdec.c	Tue Aug 10 23:42:47 2010 +0400
> @@ -331,18 +331,28 @@
>              s->avctx->pix_fmt = PIX_FMT_GRAY16;
>      }
>  
> -    if(s->picture.data[0])
> -        s->avctx->release_buffer(s->avctx, &s->picture);
> +    if (CODEC_ID_MXPEG == s->avctx->codec_id) {
> +        if (s->avctx->reget_buffer(s->avctx, &s->picture) < 0) {

this makes double buffering with direct rendering impossible



> +            av_log(s->avctx, AV_LOG_ERROR, "reget_buffer() failed\n");
> +            return -1;
> +        }
> +    } else {
> +        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.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;
> +        }

cosmetics


>      }
> -    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;
> +    }
> +
>      for(i=0; i<3; i++){
>          s->linesize[i]= s->picture.linesize[i] << s->interlaced;
>      }
> @@ -789,6 +799,14 @@
>  
>      for(mb_y = 0; mb_y < s->mb_height; mb_y++) {
>          for(mb_x = 0; mb_x < s->mb_width; mb_x++) {
> +            if ((CODEC_ID_MXPEG == s->avctx->codec_id) &&
> +                 !s->picture.key_frame) {
> +                int mb_index = mb_y * s->mb_width + mb_x;
> +                int mb_selected = (s->mxctx.mxm_bitmask[mb_index >> 3] << (mb_index & 7)) & 0x80;
> +
> +                if (!mb_selected) continue;
> +            }
> +
>              if (s->restart_interval && !s->restart_count)
>                  s->restart_count = s->restart_interval;
>  

> @@ -1026,6 +1044,12 @@
>      if(8*len + get_bits_count(&s->gb) > s->gb.size_in_bits)
>          return -1;
>  
> +    /* skip MXPEG audio data*/
> +    if ((s->avctx->codec_id == CODEC_ID_MXPEG) && (s->start_code == APP13)) {
> +        skip_bits(&s->gb, (len-2) << 3);
> +        return 0;
> +    }

how does the audio data reach the video decoder?
this just looks like the demuxer is wrongly implemented

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100811/819f0a8f/attachment.pgp>