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

[Adam Iglewski]

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?
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,
Adam