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

The Deep Explorer thedeepexplorer
Mon Apr 13 18:39:54 CEST 2009


On Mon, Apr 13, 2009 at 12:37 PM, Benjamin Larsson <banan at ludd.ltu.se> wrote:
> 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

Thanks Benjamin for pointing it out , I will resend the patch.

Regards,
-tde



More information about the ffmpeg-devel mailing list