[FFmpeg-devel] [PATCH] Unfinished GSoC qual task (16bit VQA Video Decoder)

Michael Niedermayer michaelni
Thu May 7 02:49:00 CEST 2009


On Thu, May 07, 2009 at 01:10:14AM +0200, Adam Iglewski wrote:
> Michael Niedermayer pisze:
> [...]
>>>
>>> --- ffmpeg/libavcodec/adpcm.c	2009-04-29 23:54:38.000000000 +0200
>>> +++ ffmpeg_work/libavcodec/adpcm.c	2009-05-02 00:30:17.000000000 +0200
>>> @@ -1005,6 +1005,7 @@ static int adpcm_decode_frame(AVCodecCon
>>>          if (cs->step_index < 0) cs->step_index = 0;
>>>          if (cs->step_index > 88) cs->step_index = 88;
>>>
>>> +    case CODEC_ID_ADPCM_IMA_WS_V3:
>>>          m= (buf_size - (src - buf))>>st;
>>>          for(i=0; i<m; i++) {
>>>              *samples++ = adpcm_ima_expand_nibble(&c->status[0], src[i] & 
>>> 0x0F, 4);
>>>
>>> and
>>>
>>> --- ffmpeg/libavformat/westwood.c	2009-04-20 18:35:42.000000000 +0200
>>> +++ ffmpeg_work/libavformat/westwood.c	2009-05-02 00:28:54.000000000 
>>> +0200
>>>
>>> @@ -254,8 +258,10 @@ static int wsvqa_read_header(AVFormatCon
>>>          st->codec->codec_type = CODEC_TYPE_AUDIO;
>>>          if (AV_RL16(&header[0]) == 1)
>>>              st->codec->codec_id = CODEC_ID_WESTWOOD_SND1;
>>> -        else
>>> +        else if(AV_RL16(&header[14]))
>>>              st->codec->codec_id = CODEC_ID_ADPCM_IMA_WS;
>>> +        else
>>> +            st->codec->codec_id = CODEC_ID_ADPCM_IMA_WS_V3;
>>>
>>> plus of course adding codec_id to avcodec.h etc.
>>> Is this option acceptable?
>> yes
> Patches attached.
>> [...]
>>> As for refactoring this is part of the function that you
>>> wanted to be rewritten to process chunks as needed.
>>> So maybe for now this could be as is?
>> only if you promisse to do that rewrite afterwards.
>
> Actually I already started this. I'm attaching 2 patches for review:
>
> 0005 - it simplifies decode_format80 function. In one of earlier emails
> you also mentioned that this function requires rewrite to avoid code
> duplication.
>
> 0006 - rewrite of vqa_decode_chunk to decode chunks as needed and
> remove some code duplication.
>
> This patches should be applied after first four if they will
> be accepted.
>

> And here I'v got a question about demuxing. Is it valid that
> demuxer sends packet that contains no video to decoder?

its not good ...


> That's the case with 16 bit vqa files. There are some packets
> with codebook only,


>
>> [...]
>>> @@ -291,6 +308,95 @@ static void decode_format80(const unsign
>>>                  dest_index, dest_size);
>>>  }
>>>  +static inline void vqa_copy_hc_block(uint16_t *pixels,int stride,const 
>>> uint16_t *codebook,
>>> +                                      int block_h)
>>> +{
>>> +    int pixel_y;
>>> +    for (pixel_y = 0; pixel_y < block_h; pixel_y++) {
>> while(block_h--){
>
> Fixed.
>
>>> +
>>> +static void vqa_decode_hc_video_chunk(VqaContext *s,const unsigned char 
>>> *src,unsigned int src_size)
>>> +{
>>> +    int block_x, block_y;          /* block width and height iterators 
>>> */
>>> +    int blocks_wide, blocks_high;  /* width and height in 4x4|2 blocks 
>>> */
>>> +    int block_inc;
>>> +    int index_shift;
>>> +    int i;
>>> +
>>> +    /* decoding parameters */
>>> +    uint16_t *pixels,*frame_end;
>>> +    uint16_t *codebook = (uint16_t *)s->codebook;
>>> +    int stride = s->frame.linesize[0] >> 1;
>>> +
>>> +    int vptr_action_code;
>> int type is shorter and does not seem worse
>
> OK
>
>> [...]
>>> +                case 0x0000:
>>> +                    blocks_done = vptr_action_code & 0x1fff;
>>> +                    block_x += blocks_done;
>>> +                    pixels += blocks_done * block_inc;
>>> +                    continue;
>>> +
>>> +                case 0x2000:
>>> +                case 0x4000:
>>> +                    vector_index = (vptr_action_code & 0xff) << 
>>> index_shift;
>>> +                    blocks_done = ((vptr_action_code & 0x1f00)+0x0100) 
>>> >> 7;
>>> +                    if((vptr_action_code & 0xe000)==0x4000)
>>> +                        blocks_done++;
>>> +                    break;
>>> +
>>> +                case 0x6000:
>>> +                    blocks_done=1;
>>> +                case 0xa000:
>>> +                    vector_index = (vptr_action_code & 0x1fff) << 
>>> index_shift;
>>> +                    if((vptr_action_code & 0xe000)==0xa000)
>>> +                        blocks_done = *src++;
>>> +                    break;
>> if(type==0){
>>    ...
>> }else if(type < 3){
>>     vector_index = (code & 0xff) << index_shift;
>>     blocks_done  = (code>>7) + 1 + type;
>
> I think this should be ((code&0x1f00)>>7) + 1 + type.
>
>> }else if(type==3 || type==5){
>>     vector_index =  code         << index_shift;
>>     if(type==3) blocks_done= 1;
>>     else        blocks_done= *src++;
>> }
>
> First four patches implements: demuxing and decoding
> of 16 bit VQA file,new codec_id to
> properly decode stereo sound in this files.
>
>
>
> Regards,
[...]
> @@ -156,6 +158,11 @@ static av_cold int vqa_decode_init(AVCodecContext *avctx)
>      s->vector_height = vqa_header[11];
>      s->partial_count = s->partial_countdown = vqa_header[13];
>  

> +    if(!AV_RL16(&vqa_header[14]))
> +        avctx->pix_fmt = PIX_FMT_RGB555;
> +    else
> +        avctx->pix_fmt = PIX_FMT_PAL8;
> +

if(!x) a
else   b

can be simplified to
if(x) b
else  a


>      /* the vector dimensions have to meet very stringent requirements */
>      if ((s->vector_width != 4) ||
>          ((s->vector_height != 2) && (s->vector_height != 4))) {
> @@ -205,11 +212,17 @@ static void decode_format80(const unsigned char *src, int src_size,
>  
>      int src_index = 0;
>      int dest_index = 0;
> +    int new_format = 0;
>      int count;
>      int src_pos;
>      unsigned char color;
>      int i;
>  
> +    if (!src[src_index] || src_size > 0xffff) {
> +        new_format = 1;
> +        src_index++;
> +    }

int new_format = !src[0] || src_size > 0xffff;

if(new_format)
    src_index++;


[...]
> @@ -291,6 +308,86 @@ static void decode_format80(const unsigned char *src, int src_size,
>                  dest_index, dest_size);
>  }
>  
> +static inline void vqa_copy_hc_block(uint16_t *pixels,int stride,const uint16_t *codebook,
> +                                      int block_h)
> +{
> +    while(block_h--) {
> +        memcpy(pixels,codebook,8);
> +        pixels += stride;
> +        codebook += 4;
> +    }
> +}
> +
> +static void vqa_decode_hc_video_chunk(VqaContext *s,const unsigned char *src,unsigned int src_size)
> +{
> +    int block_x, block_y;          /* block width and height iterators */
> +    int blocks_wide, blocks_high;  /* width and height in 4x4|2 blocks */
> +    int block_inc;
> +    int index_shift;
> +    int i;
> +
> +    /* decoding parameters */
> +    uint16_t *pixels,*frame_end;
> +    uint16_t *codebook = (uint16_t *)s->codebook;
> +    int stride = s->frame.linesize[0] >> 1;
> +
> +    int type,code;
> +    int vector_index = 0;
> +
> +    blocks_wide = s->width >> 2;
> +    blocks_high = s->height / s->vector_height;
> +    block_inc = 4;

> +    frame_end = (uint16_t *)s->frame.data[0] + s->height * stride + s->width;

this likely doesnt work if stride is <0


> +
> +    if (s->vector_height == 4)
> +        index_shift = 4;
> +    else
> +        index_shift = 3;
> +
> +    for(block_y=0; block_y < blocks_high; block_y ++) {
> +        pixels = (uint16_t *)s->frame.data[0] + (block_y * s->vector_height * stride);
> +
> +        for(block_x=0; block_x < blocks_wide; ) {
> +            int blocks_done;
> +            code = bytestream_get_le16(&src);
> +            type = code >> 13;
> +            code &= 0x1fff;
> +
> +            if(!type) {
> +                    blocks_done = code;
> +                    block_x += blocks_done;
> +                    pixels += blocks_done * block_inc;
> +                    continue;

indent should be 4 spaces

[...]

>  allcodecs.c |    1 +
>  avcodec.h   |    1 +
>  2 files changed, 2 insertions(+)
> 526e4add2cc526b43dac9997b48648166522a3dd  0002-Add-IMA_WS_V3-codec-id.patch
> >From 3b1100ba49964ccc280643b7600d68bd82ab046a Mon Sep 17 00:00:00 2001
> From: Adam Iglewski <szwagros at szwagros-desktop.(none)>
> Date: Wed, 6 May 2009 22:26:38 +0200
> Subject: [PATCH] Add IMA_WS_V3 codec id

ok
[...]

>  adpcm.c |    2 ++
>  1 file changed, 2 insertions(+)
> 2b1ac0f94c2427ca735cd4200d48aed079728f30  0004-Add-IMS_WS_V3-codec.patch
> >From d9db160b2d364c3f55b2d10868026aea5d460023 Mon Sep 17 00:00:00 2001
> From: Adam Iglewski <szwagros at szwagros-desktop.(none)>
> Date: Wed, 6 May 2009 22:30:32 +0200
> Subject: [PATCH] Add IMS_WS_V3 codec

ok

[...]


>  vqavideo.c |  118 +++++++++++++++++++------------------------------------------
>  1 file changed, 38 insertions(+), 80 deletions(-)
> 2c0d249f1dd11c66086648c2433c2d3bf7a63da2  0005-Simplf-decode_format80.patch
> >From 8d1c315a8d3b3a68322eb309c4befe7f42462ff5 Mon Sep 17 00:00:00 2001
> From: Adam Iglewski <adam.iglewski at gmail.com>
> Date: Thu, 7 May 2009 00:09:49 +0200
> Subject: [PATCH] Simplf decode_format80

This patch is pretty much unreviewable

[...]


>  vqavideo.c |  383 ++++++++++++++++++++++---------------------------------------
>  1 file changed, 139 insertions(+), 244 deletions(-)
> a16fa5877d53326566a718b18554f25c936e3be2  0006-Simplify-vqa_decode_chunk.patch
> >From c36d1f378076e929fa679033ad5a4a1a7487ff62 Mon Sep 17 00:00:00 2001
> From: Adam Iglewski <adam.iglewski at gmail.com>
> Date: Thu, 7 May 2009 00:38:11 +0200
> Subject: [PATCH] Simplify vqa_decode_chunk

so is this

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090507/f95e1dc5/attachment.pgp>



More information about the ffmpeg-devel mailing list