[FFmpeg-devel] GSoC Qual VQA v3 : updated patch.

Benjamin Larsson banan
Mon Apr 13 18:37:52 CEST 2009


The Deep Explorer wrote:
> Hi,
>    Compiles, but wont do the complete thing since I am waiting on the
> vector indexing algorithm.
> Please if you could comment on the other parts of the algorithm , it
> would be extremely nice
> while I am waiting for the algorithm.
> 
> I guess I just have today , so any help is appreciated.
> 
> Things I need to do is :
> 
> i) Finish the vector index algorithm to populate the frame after
> someone tells me the vector indexing algo
> ii) Test it on vqa v3 files.
> 
> 
> Also, for vptr decoding , I am getting codes 4,6 from movies under
> blade , which are not mentioned in the document ,
> I am ignoring those right now since nothing is mentioned for them.
> 
> Look forward to some feedback.... Please help
> 
> Thanks,
> -tde
> 
> PS : patcheck reports some  NULL refs (exisitng ones not mine),and
> some other stuff which I have not introduced.
> 
> 



> Index: libavcodec/vqavideo.c
> ===================================================================
> --- libavcodec/vqavideo.c	(revision 18493)
> +++ libavcodec/vqavideo.c	(working copy)
> @@ -89,13 +89,16 @@
>  #define CPL0_TAG MKBETAG('C', 'P', 'L', '0')
>  #define CPLZ_TAG MKBETAG('C', 'P', 'L', 'Z')
>  #define VPTZ_TAG MKBETAG('V', 'P', 'T', 'Z')
> -

Cosmetics.

> +#define VPTR_TAG MKBETAG('V', 'P', 'T', 'R')
> +#define VPRZ_TAG MKBETAG('V', 'P', 'R', 'Z')
>  #define VQA_DEBUG 0
>  
>  #if VQA_DEBUG
>  #define vqa_debug printf
>  #else
> -static inline void vqa_debug(const char *format, ...) { }
> +static inline void vqa_debug(const char *format, ...)
> +{
> +}
>  #endif
>  
>  typedef struct VqaContext {
> @@ -108,15 +111,15 @@
>  
>      uint32_t palette[PALETTE_COUNT];
>  
> -    int width;   /* width of a frame */
> -    int height;   /* height of a frame */
> -    int vector_width;  /* width of individual vector */
> -    int vector_height;  /* height of individual vector */
> -    int vqa_version;  /* this should be either 1, 2 or 3 */
> +    int width;                  /* width of a frame */
> +    int height;                 /* height of a frame */
> +    int vector_width;           /* width of individual vector */
> +    int vector_height;          /* height of individual vector */
> +    int vqa_version;            /* this should be either 1, 2 or 3 */


Cosmetics.

>  
> -    unsigned char *codebook;         /* the current codebook */
> +    unsigned char *codebook;    /* the current codebook */


Cosmetics.

>      int codebook_size;
> -    unsigned char *next_codebook_buffer;  /* accumulator for next codebook */
> +    unsigned char *next_codebook_buffer;        /* accumulator for next codebook */
>      int next_codebook_buffer_index;
>  


Cosmetics.

>      unsigned char *decode_buffer;
> @@ -128,7 +131,7 @@
>  
>  } VqaContext;
>  
> -static av_cold int vqa_decode_init(AVCodecContext *avctx)
> +static av_cold int vqa_decode_init(AVCodecContext * avctx)


Cosmetics.


>  {
>      VqaContext *s = avctx->priv_data;
>      unsigned char *vqa_header;
> @@ -139,17 +142,19 @@
>  
>      /* make sure the extradata made it */
>      if (s->avctx->extradata_size != VQA_HEADER_SIZE) {
> -        av_log(s->avctx, AV_LOG_ERROR, "  VQA video: expected extradata size of %d\n", VQA_HEADER_SIZE);
> +        av_log(s->avctx, AV_LOG_ERROR,
> +               "  VQA video: expected extradata size of %d\n",
> +               VQA_HEADER_SIZE);


Cosmetics.

>          return -1;
>      }
>  
> +    vqa_header = (unsigned char *) s->avctx->extradata;
>      /* load up the VQA parameters from the header */
> -    vqa_header = (unsigned char *)s->avctx->extradata;


Cosmetics.

>      s->vqa_version = vqa_header[0];
>      s->width = AV_RL16(&vqa_header[6]);
>      s->height = AV_RL16(&vqa_header[8]);
> -    if(avcodec_check_dimensions(avctx, s->width, s->height)){
> -        s->width= s->height= 0;
> +    if (avcodec_check_dimensions(avctx, s->width, s->height)) {
> +        s->width = s->height = 0;


Cosmetics.

>          return -1;
>      }
>      s->vector_width = vqa_header[10];
> @@ -201,7 +206,9 @@
>      }
>  
>  static void decode_format80(const unsigned char *src, int src_size,
> -    unsigned char *dest, int dest_size, int check_size) {
> +                            unsigned char *dest, int dest_size,
> +                            int check_size, int check_modification)
> +{


Cosmetics.

>  
>      int src_index = 0;
>      int dest_index = 0;
> @@ -219,8 +226,9 @@
>              return;
>  
>          if (dest_index >= dest_size) {
> -            av_log(NULL, AV_LOG_ERROR, "  VQA video: decode_format80 problem: dest_index (%d) exceeded dest_size (%d)\n",
> -                dest_index, dest_size);
> +            av_log(NULL, AV_LOG_ERROR,
> +                   "  VQA video: decode_format80 problem: dest_index (%d) exceeded dest_size (%d)\n",
> +                   dest_index, dest_size);


Cosmetics.


>              return;
>          }
>  
> @@ -231,7 +239,8 @@
>              src_index += 2;
>              src_pos = AV_RL16(&src[src_index]);
>              src_index += 2;
> -            vqa_debug("(1) copy %X bytes from absolute pos %X\n", count, src_pos);
> +            vqa_debug("(1) copy %X bytes from absolute pos %X\n", count,
> +                      src_pos);


Cosmetics.

>              CHECK_COUNT();
>              for (i = 0; i < count; i++)
>                  dest[dest_index + i] = dest[src_pos + i];
> @@ -251,9 +260,20 @@
>          } else if ((src[src_index] & 0xC0) == 0xC0) {
>  
>              count = (src[src_index++] & 0x3F) + 3;
> -            src_pos = AV_RL16(&src[src_index]);
> +            if (check_modification == 1) {
> +                src_pos = dest_index - AV_RL16(&src[src_index]);
> +                if (src_pos < 0) {
> +                    av_log(NULL, AV_LOG_ERROR,
> +                           "  VQA video: decode_format80 problem: src_pos (%d) underflow\n",
> +                           src_pos);
> +                    return;
> +                }
> +            } else {
> +                src_pos = AV_RL16(&src[src_index]);
> +            }

First actual change. This is what you first line of diff should look like.

>              src_index += 2;
> -            vqa_debug("(3) copy %X bytes from absolute pos %X\n", count, src_pos);
> +            vqa_debug("(3) copy %X bytes from absolute pos %X\n", count,
> +                      src_pos);


Cosmetics. And it goes on forever like this.

[...]

It's hard to see what is code changes and what is not. Get rid of all
cosmetics, then send the patch for review.

MvH
Benjamin Larsson



More information about the ffmpeg-devel mailing list