[FFmpeg-devel] [PATCH] Unfinished GSoC qual task (16bit VQA Video Decoder)
Adam Iglewski
adam.iglewski
Wed Apr 29 13:08:11 CEST 2009
Michael Niedermayer pisze:
>>> That will break the existing cases that use CODEC_ID_ADPCM_IMA_WS i assume
>>> [...]
>> Yes if there are VQA files with stereo sound where nibbles are ordered
>
> CODEC_ID_ADPCM_IMA_WS is used by apc.c and westwood.c
>
>
I knew that it was used in westwood.s but I wrongly assumed
that it's used only there (since codec name has _WS postfix).
I will be more careful next time.
>> like this:
>>
>> LL RR LL RR ....
>>
>> But I couldn't find any in VQA samples directory. Assuming that such files
>> do exists what is the correct way to fix this? Adding new codec id? If not
>
> if your case is identical to CODEC_ID_ADPCM_XA then use CODEC_ID_ADPCM_XA
> ad dont change another to become identical to it.
>
No it's not. Maybe I should clarify some more. There are 3
possibilities regarding sound files using CODEC_ID_ADPCM_IMA_WS:
1. mono files - plays correctly in ffplay.
2. stereo files with nibbles for each channel stored in bytes ordered
like this:
RR LL RR LL RR LL - now (thanks to Vitor) I finally found such
file in http://samples.mplayerhq.hu/game-formats/cryo/ - plays
correctly in ffplay.
3. stereo files with nibbles stored in different order that option 2.
Where first half of SND2 chunk contains nibbles for left channel
and second half for right channel - plays incorrectly in ffplay
This format is used in VQA files with 16 bit video. And decoding
this is almost identical to CODEC_ID_ADPCM_4XM but without
initialization of predictor and step_index.
That's why I proposed my first patch which now is of course
wrong because will break playing files from option 2.
> [...]
>
> divide by 2^x could use >> maybe
> /home/michael/add_vqa_hc.diff:103:+ int stride = s->frame.linesize[0] / 2;
> /home/michael/add_vqa_hc.diff:110:+ blocks_wide = s->width / 4;
>
> missing } prior to else
> /home/michael/add_vqa_hc.diff:34:+ else
> /home/michael/add_vqa_hc.diff:117:+ else
>
> { should be on the same line as the related previous statement
> /home/michael/add_vqa_hc.diff:121:+ {
> /home/michael/add_vqa_hc.diff:124:+ {
>
>
> Missing changelog entry (ignore if minor change)
>
OK.
>
> [...]
>> +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 blocks */
>> + int block_inc;
>> + int index_shift;
>> + int i;
>> +
>
>> + /* decoding parameters */
>> + int blocks_done;
>
> can be moved into the loop
>
OK.
>
>> + uint16_t *pixels,*frame_end;
>> + uint16_t *codebook = (uint16_t *)s->codebook;
>> + int stride = s->frame.linesize[0] / 2;
>> +
>> + int vptr_action_code;
>> + int vector_index = 0;
>
>> + int get_vectors_from_stream;
>
> redundant, the type can be checked directly
>
OK.
>
>> +
>> + blocks_done = 0;
>
> redundant
>
OK.
>
>> + blocks_wide = s->width / 4;
>> + blocks_high = s->height / s->vector_height;
>> + block_inc = 4;
>> + frame_end = (uint16_t *)s->frame.data[0] + s->height * stride + s->width;
>> +
>> + 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; )
>> + {
>> + get_vectors_from_stream=0;
>> + vptr_action_code = bytestream_get_le16(&src);
>> +
>> + switch(vptr_action_code & 0xe000) {
>> +
>> + /* Skip Count blocks. Count is (Val & 0x1fff). */
>> + case 0x0000:
>> + blocks_done = vptr_action_code & 0x1fff;
>> + block_x += blocks_done;
>> + pixels += blocks_done * block_inc;
>> + continue;
>> +
>> + /* Write block number (Val & 0xff) Count times.
>> + * Count is (((Val/256) & 0x1f)+1)*2
>> + */
>> + case 0x2000:
>> + vector_index = (vptr_action_code & 0xff) << index_shift;
>> + blocks_done = (((vptr_action_code>>8) & 0x1f)+1) << 1;
>> + break;
>> +
>> + /* Write block number (Val & 0xff) and then write Count blocks
>> + * getting their indexes by reading next Count bytes from
>> + * the VPTR chunk. Count is (((Val/256) & 0x1f)+1)*2
>> + */
>> + case 0x4000:
>> + vector_index = (vptr_action_code & 0xff) << index_shift;
>> + blocks_done = ((((vptr_action_code>>8) & 0x1f)+1) << 1)+1;
>> + get_vectors_from_stream=1;
>> + break;
>> +
>> + /* Write block (Val & 0x1fff).*/
>> + case 0x6000:
>> + vector_index = (vptr_action_code & 0x1fff) << index_shift;
>> + blocks_done=1;
>> + break;
>> +
>> + /* Write block (Val & 0x1fff) Count times.
>> + * Count is the next byte from the VPTR chunk
>> + */
>> + case 0xa000:
>> + vector_index = (vptr_action_code & 0x1fff) << index_shift;
>> + blocks_done = *src++;
>> + break;
>> + }
>> +
>> + if(pixels + s->vector_height * stride + blocks_done * block_inc > frame_end) {
>> + av_log(s->avctx, AV_LOG_ERROR, " VQA video: too many blocks in frame.\n");
>> + return;
>> + }
>> +
>> + for(i=0;i<blocks_done;i++) {
>> + vqa_copy_hc_block(pixels,stride,&codebook[vector_index],s->vector_height);
>> + pixels += block_inc;
>> + if(i && get_vectors_from_stream) {
>> + vector_index = *src++;
>> + vector_index <<= index_shift;
>> + }
>> + }
>> + block_x += blocks_done;
>
> block_x += blocks_done;
> while( blocks_done-- ){
> }
>
OK. But using while I loose ability to check if this
is second iteration. When 'action code'==0x4000
I need to render blocks_done+1 blocks using
vector_index derived from 'action code' as first block
and reading blocks_done indices from stream.
Or am I missing something ?
And actually current code is buggy. It writes first block
twice before starting reading vector index from stream
and doesn't write last block from stream at all.
Loop should look like this:
for(i=0;i<blocks_done;i++) {
if(i && (vptr_action_code & 0xe000)==0x400) {
vector_index = *src++;
vector_index <<= index_shift;
}
vqa_copy_hc_block(pixels,stride,&codebook[vector_index],
s->vector_height);
pixels += block_inc;
}
block_x += blocks_done;
>
> [...]
>> @@ -308,6 +433,8 @@ static void vqa_decode_chunk(VqaContext
>> int cpl0_chunk = -1;
>> int cplz_chunk = -1;
>> int vptz_chunk = -1;
>> + int vptr_chunk = -1;
>> + int vprz_chunk = -1;
>>
>> int x, y;
>> int lines = 0;
>> @@ -354,6 +481,14 @@ static void vqa_decode_chunk(VqaContext
>> vptz_chunk = index;
>> break;
>>
>> + case VPTR_TAG:
>> + vptr_chunk = index;
>> + break;
>> +
>> + case VPRZ_TAG:
>> + vprz_chunk = index;
>> + break;
>> +
>> default:
>> av_log(s->avctx, AV_LOG_ERROR, " VQA video: Found unknown chunk type: %c%c%c%c (%08X)\n",
>> (chunk_type >> 24) & 0xFF,
>
> IIRC mike said they are in correct order in files ...
>
So do You want me to rewrite vqa_decode_chunk as
suggested in previous mail? Using separate function to
get chunks as needed? If so should this be in separate
patch and after my current code will be accepted?
I will send updated patch addressing issues pointed by
You and Vitor in the evening.
Adam
More information about the ffmpeg-devel
mailing list