[FFmpeg-devel] GSoC VQA v3 patch

The Deep Explorer thedeepexplorer
Mon Apr 13 18:21:02 CEST 2009


On Mon, Apr 13, 2009 at 11:40 AM, Stefano Sabatini
<stefano.sabatini-lala at poste.it> wrote:
> On date Monday 2009-04-13 06:18:11 -0400, The Deep Explorer encoded:
>> Hi,
>>    Please find the patch attached with this mail.
>> I have used the patchcheck took , very helpful...
>> removed the trailing white space, tabs and the style
>> is KR.
>>
>> Hope , I will get some feedbacks now on the algorithm part so that I
>> can atleast make
>> a working submission ...
>>
>> I do need the vector indexing algorithm, could not find it in the docs...
>> thats the part left...
>>
>> Need feedback on the CBFZ, modified format 80, the vprz , the vptr (
>> thanks to kostya ,
>> he helped me a lot)
>>
>> Look forward to the responses.
>
> Tip: run tools/patcheck on your patch.
>
> For example this is the result of running patcheck on your patch:
>
> stefano at geppetto ~/s/ffmpeg> tools/patcheck patches/vqa_v3_mod.diff
> patCHeck 1e10.0
> This tool is intended to help a human check/review patches it is very far from
> being free of false positives and negatives, its output are just hints of what
> may or may not be bad. When you use it and it misses something or detects
> something wrong, fix it and send a patch to the ffmpeg-dev ML
> License:GPL Autor: Michael Niedermayer
>
> common typos
> patches/vqa_v3_mod.diff:271:+        // assuming that the temp buffer cant be more than the decode buffer
>
> Missing context in av_log
> patches/vqa_v3_mod.diff:53:+                    av_log(NULL, AV_LOG_ERROR, "  VQA video: decode_format80 problem: src_pos (%d) underflow\n",
> patches/vqa_v3_mod.diff:74:+                  av_log(NULL, AV_LOG_ERROR, "  VQA video: decode_format80 problem: src_pos (%d) underflow\n",
> patches/vqa_v3_mod.diff:105:+        av_log(NULL,AV_LOG_ERROR,"Actual value of code %d Code Value is %d \n",code_buf,code);
> patches/vqa_v3_mod.diff:112:+             av_log(NULL,AV_LOG_ERROR,"Code is 000 %d \n",code);
> patches/vqa_v3_mod.diff:120:+             av_log(NULL,AV_LOG_ERROR,"Code is 001 %d \n",code);
> patches/vqa_v3_mod.diff:132:+             av_log(NULL,AV_LOG_ERROR,"Code is 010 %d \n",code);
> patches/vqa_v3_mod.diff:139:+             av_log(NULL,AV_LOG_ERROR,"Code is 011 %d\n",code);
> patches/vqa_v3_mod.diff:150:+             av_log(NULL,AV_LOG_ERROR,"Code is 101 %d\n",code);
> patches/vqa_v3_mod.diff:153:+             av_log(NULL,AV_LOG_ERROR,"Code %d is unknown and not supported \n",code);
> patches/vqa_v3_mod.diff:160:+        av_log(NULL,AV_LOG_ERROR,"VQA : deocde buffer overflow ");
> patches/vqa_v3_mod.diff:162:+    av_log(NULL,AV_LOG_ERROR,"src_index  %d src_size = %d \n",src_index,src_size);
>
> divide by 2^x could use >> maybe
> patches/vqa_v3_mod.diff:116:+             count = (((code_buf/256) & 0x1f)+1)*2;
> patches/vqa_v3_mod.diff:124:+              the VPTR chunk. Count is (((Val/256) & 0x1f)+1)*2.  Again, the block numbers range from 0-255.*/
> patches/vqa_v3_mod.diff:126:+             count = (((code_buf/256) & 0x1f)+1)*2;
>
> missing } prior to else
> patches/vqa_v3_mod.diff:58:+             else{
> patches/vqa_v3_mod.diff:79:+            else{
> patches/vqa_v3_mod.diff:228:+        else{
>
>  Non doxy comments
> patches/vqa_v3_mod.diff-123-+            /* 010 - Write block number (Val & 0xff) and then write Count blocks getting their indexes by reading next Count bytes from
> patches/vqa_v3_mod.diff:124:+              the VPTR chunk. Count is (((Val/256) & 0x1f)+1)*2.  Again, the block numbers range from 0-255.*/
> --
> --
> patches/vqa_v3_mod.diff-142-+        /*101 - Write block (Val & 0x1fff) Count times. Count is the next
> patches/vqa_v3_mod.diff:143:+                byte from the VPTR chunk.*/
> --
>
> possibly unused variables
> possibly never written:dest
> possibly constant     :dest
> possibly never written:dest_size
> possibly constant     :dest_size
> possibly never written:check_size
> possibly never read   :check_size
> possibly constant     :check_size
> possibly never written:dest
> possibly constant     :dest
>
> Missing changelog entry (ignore if minor change)
>
> Mergeable calls
> av_log(s->avctx, AV_LOG_ERROR, "  VQA video: problem: no VPTZ chunk found\n");
> +        av_log
>
> [...]
>
> Regards.
> --
> FFmpeg = Foolish & Faboulous Mega Prodigious EnGine

Thanks for pointing those, I have redone and resend the patch,

Regards,
-tde



More information about the ffmpeg-devel mailing list