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

Michael Niedermayer michaelni
Mon May 11 18:45:25 CEST 2009


On Sat, May 09, 2009 at 02:31:19PM +0200, Adam Iglewski wrote:
> Michael Niedermayer pisze:
> [...]
>>> 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 ...
> I thought so. Is "joining" packets to ensure that it contains
> video in demuxer and then sending them to decoder is acceptable solution? 

yes


> If so I'm attaching patch
>
> 0004-Do-not-send-packets-with-no-video-data-to-decoder
>
> If not what would be the correct way to fix this?
>
>>> +    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
>
> Fixed locally.
>
> [...]
>>> +    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++;
>>
>
> Fixed locally.
>> [...]
>>> @@ -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
>
> Sure but is it possible that linesize[0] is negative? There
> are some codecs in FFmpeg that do similar calculations.

if you call AVCodecContext.get_buffer() to get the image ...


>
> [...]
>>> +
>>> +            if(!type) {
>>> +                    blocks_done = code;
>>> +                    block_x += blocks_done;
>>> +                    pixels += blocks_done * block_inc;
>>> +                    continue;
>> indent should be 4 spaces
>
> Fixed locally.
>
>>>  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
>> [...]
>> so is this
>> [...]
>
> Sorry about that. I'm sending them again. I hope this
> time they are in more readable form. I split patch
> that simplifies vqa_decode_chunk in two parts.
> First one adds new functions and second
> is rewrite of vqa_decode_chunk using those new
> function.
>
> And is there any other way to create readable
> patches than adding new content with indent different
> than in original file? And then fixing it in separate patch?
>
> Regards,
> Adam
>
>
>
>
>
>

>  westwood.c |   32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 8b3329deaaa854617dadf627646615e0a3914490  0004-Do-not-send-packets-with-no-video-data-to-decoder.patch
> >From cb9c70c2ce39754db0e61a9893d417e0ee21d4ca Mon Sep 17 00:00:00 2001
> From: Adam Iglewski <adam.iglewski at gmail.com>
> Date: Sat, 9 May 2009 01:53:12 +0200
> Subject: [PATCH] Do not send packets with no video data to decoder
> 
> ---
>  libavformat/westwood.c |   32 +++++++++++++++++++++++++++++++-
>  1 files changed, 31 insertions(+), 1 deletions(-)
> 
> diff --git a/libavformat/westwood.c b/libavformat/westwood.c
> index 8726385..9f988aa 100644
> --- a/libavformat/westwood.c
> +++ b/libavformat/westwood.c
> @@ -84,6 +84,8 @@ typedef struct WsVqaDemuxContext {
>      int video_stream_index;
>  
>      int64_t audio_frame_counter;
> +    unsigned int vqfl_chunk_size;
> +    unsigned char* vqfl_chunk_data;
>  } WsVqaDemuxContext;
>  
>  static int wsaud_probe(AVProbeData *p)

> @@ -314,6 +316,8 @@ static int wsvqa_read_header(AVFormatContext *s,
>          url_fseek(pb, chunk_size, SEEK_CUR);
>      } while (chunk_tag != FINF_TAG);
>  
> +    wsvqa->vqfl_chunk_size=0;
> +    wsvqa->vqfl_chunk_data=NULL;
>      return 0;
>  }

arent they 0 already?


>  
> @@ -333,8 +337,33 @@ static int wsvqa_read_packet(AVFormatContext *s,
>          chunk_size = AV_RB32(&preamble[4]);
>          skip_byte = chunk_size & 0x01;
>  
> -        if ((chunk_type == SND1_TAG) || (chunk_type == SND2_TAG) || (chunk_type == VQFR_TAG) || (chunk_type == VQFL_TAG)) {
> +        if (chunk_type == VQFL_TAG) {
>  
> +            wsvqa->vqfl_chunk_size = chunk_size;
> +            wsvqa->vqfl_chunk_data = av_mallocz(chunk_size);
> +            if (!wsvqa->vqfl_chunk_data)
> +                return AVERROR(ENOMEM);
> +            ret = get_buffer(pb, wsvqa->vqfl_chunk_data, chunk_size);
> +            if (ret != chunk_size)
> +                return AVERROR(EIO);
> +            if (skip_byte)
> +                url_fseek(pb, 1, SEEK_CUR);
> +            continue;
> +

is there something preventing this from being executed several times and
then leaking?


[...]
>  vqavideo.c |  121 ++++++++++++++++++++-----------------------------------------
>  1 file changed, 40 insertions(+), 81 deletions(-)
> a61405d330ff867b8eb35e6ce28addf012772baf  0005-Simplify-decode_format80-function.patch
> >From ea01aecb069cbf7798efe7f7ac4500d3f7e933c4 Mon Sep 17 00:00:00 2001
> From: Adam Iglewski <adam.iglewski at gmail.com>
> Date: Sat, 9 May 2009 01:57:21 +0200
> Subject: [PATCH] Simplify decode_format80 function
> 
> ---
>  libavcodec/vqavideo.c |  121 ++++++++++++++++---------------------------------
>  1 files changed, 40 insertions(+), 81 deletions(-)
> 
> diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c
> index 67dd7de..9646d67 100644
> --- a/libavcodec/vqavideo.c
> +++ b/libavcodec/vqavideo.c
> @@ -199,101 +199,61 @@ static av_cold int vqa_decode_init(AVCodecContext *avctx)
>      return 0;
>  }
>  
> -#define CHECK_COUNT() \
> -    if (dest_index + count > dest_size) { \
> -        av_log(NULL, AV_LOG_ERROR, "  VQA video: decode_format80 problem: next op would overflow dest_index\n"); \
> -        av_log(NULL, AV_LOG_ERROR, "  VQA video: current dest_index = %d, count = %d, dest_size = %d\n", \
> -            dest_index, count, dest_size); \
> +#define CHECK_PTR() \
> +    if (dest + count > dest_end) { \
> +        av_log(NULL, AV_LOG_ERROR, "  VQA video: decode_format80 problem: next op would overflow dest buffer\n"); \
>          return; \
>      }
>  
>  static void decode_format80(const unsigned char *src, int src_size,
>      unsigned char *dest, int dest_size, int check_size) {
>  
> -    int src_index = 0;
> -    int dest_index = 0;
> +    const unsigned char *from,*src_end = src+src_size;
> +    unsigned char *dest_end = dest+dest_size;

mixes functional and cosmetic changes

[...]

>  vqavideo.c |  119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 195a26f5e7badd5b9c5d399083ee5ce2c11d218f  0006-Simplify-vqa_decode_chunk-part-I.patch
> >From cbe79ab1c88aeb7c9ccdb869c63449c4c9c47871 Mon Sep 17 00:00:00 2001
> From: Adam Iglewski <adam.iglewski at gmail.com>
> Date: Sat, 9 May 2009 11:42:25 +0200
> Subject: [PATCH] Simplify vqa_decode_chunk part I
> 
> ---
>  libavcodec/vqavideo.c |  119 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 119 insertions(+), 0 deletions(-)

119 insertions and 0 deletions does not qualify as simplification, rather
adition of code

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- 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/20090511/81bb56c4/attachment.pgp>



More information about the ffmpeg-devel mailing list