[FFmpeg-devel] flashsvenc.c - sampling block size too low

Jason Askew jason.askew
Thu May 17 22:52:11 CEST 2007


> > +    //if this is pass2, parse log file
> > +    if((avctx->flags&CODEC_FLAG_PASS2)){
>
> superflous pair of ()

removed

> > +        int e;
> > +
>
> Variable declarations must be at the beginning of a block.

moved

>
> > +        if(es==NULL) {
> > +            av_log(avctx, AV_LOG_ERROR, " Did not find count at end of log file.\n");
> > +            return -1;
> > +        }
> > +
> > +        int count;
> > +        e = sscanf(es, ":%d", &count);
> > +        if(e != 1) {
> > +            av_log(avctx, AV_LOG_ERROR, " Did not find count at end of log file.\n");
> > +            return -1;
> > +        }
>
> I'd propose using strtol so you can check that there's actually a number
> and not something like 123asaf, which this construction would accept...

Hmm, following other code's methods of reading the log file.  I'll
research and see.

>
> > +        int frame, x, y;
> > +        char* pNext = avctx->stats_in;
>
> pNext? That looks really ugly to me, why not just next or so?
>

fixed.

> > +        int statIndex = 0;
>
> In general, in ffmpeg using _ is preferred over (mis)using uppercase to
> mark word boundaries.
>

fixed

> > +    //setup of buffer to hold block size stat data
> > +    if((avctx->flags&CODEC_FLAG_PASS1)){
>

fixed

> > +//performs the same actions as copy_region_enc but does not commit zlib data to buffer
> > +//and returns the size
>
> And comments should be doxygen-compatible

i was going to clean up comments on the whole file as a separate diff
- trying to get my functionality changes in there first so I'm keeping
this modular.

>
> > +static int sample_block_size(FlashSVContext *s, AVFrame *p,
> > +        int block_width, int block_height, uint8_t *previous_frame, int* I_frame) {
> [...]
> > +    /* loop over all block columns */
> > +    for (j = 0; j < v_blocks + (v_part?1:0); j++)
>
> !!v_part is used in other places instead of (v_part?1:0).

copy of existing code in function.  if this needs to be revisited it
is in several places.  leaving it for now.

>
> That += 2 could factored out, thus avoiding the else. No idea if that
> has a real advantage though.
>

several additions removed in a minor refactoring

> > @@ -238,8 +388,79 @@ static int flashsv_encode_frame(AVCodecC
> >          }
> >      }
> >
> > -    opt_w=4;
> > -    opt_h=4;
> > +    pfptr = s->previous_frame;
> > +    //if linesize is negative, prep pointer to match upside down ptr movement of data[0]
> > +    if(p->linesize[0] < 0) {
> > +        pfptr = pfptr - ((s->image_height-1) * p->linesize[0]);
>
> I would suggest
> pfptr += (s->image_height-1) * -p->linesize[0];

> Because 1) + seems more clear to me 2) if image_height happens to be
> unsigned, this will fail on 64 bit systems, as I recently had to find
> out in MPlayer...

image_height can be negative, as had been stressed by Michael several
times.  calculation is only occurring if linesize is negative in this
case.  did change to -=, however.

> Hmm... can have I_frame have other values than 0 and 1? If yes, use
> defines or an enum. If not, use just I_frame and !I_frame.
>

again, copy of legacy code that I was not modifying as it was not part
of my larger patch.  leaving it for now, will revisit with another
patch to cover the whole file.

new diff attached.
-------------- next part --------------
--- flashsvenc_c.c	Thu May 17 10:32:47 2007
+++ flashsvenc.c	Thu May 17 15:45:33 2007
@@ -62,6 +62,14 @@
 #include "bitstream.h"
 #include "bytestream.h"
 
+const int TP_BLCK_SIZE = 8;
+
+//used to store multi pass stats
+typedef struct FlashSVSectionStat {
+    int frame_num;
+    int blk_size_w;
+    int blk_size_h;
+} FlashSVSectionStat;
 
 typedef struct FlashSVContext {
     AVCodecContext *avctx;
@@ -74,6 +82,10 @@ typedef struct FlashSVContext {
     int block_size;
     z_stream zstream;
     int last_key_frame;
+    int* tpSizes;
+    unsigned int key_frame_cnt;
+    FlashSVSectionStat *stats;
+    int stat_count;
 } FlashSVContext;
 
 static int copy_region_enc(uint8_t *sptr, uint8_t *dptr,
@@ -112,6 +124,66 @@ static int flashsv_encode_init(AVCodecCo
         return -1;
     }
 
+    //if this is pass2, parse log file
+    if(avctx->flags&CODEC_FLAG_PASS2){
+        int e;
+
+        if(avctx->stats_in == NULL) {
+            av_log(avctx, AV_LOG_ERROR, " Second pass flag and stats_in is NULL.\n");
+            return -1;
+        }
+
+        //find the count at the end of stats_in
+        char* es = strrchr(avctx->stats_in, ':');
+        if(es==NULL) {
+            av_log(avctx, AV_LOG_ERROR, " Did not find count at end of log file.\n");
+            return -1;
+        }
+
+        int count;
+        e = sscanf(es, ":%d", &count);
+        if(e != 1) {
+            av_log(avctx, AV_LOG_ERROR, " Did not find count at end of log file.\n");
+            return -1;
+        }
+
+        //create array to hold stats data
+        s->stats = av_mallocz(count * sizeof(FlashSVSectionStat));
+        if (!s->stats) {
+            av_log(avctx, AV_LOG_ERROR, " Memory allocation failed s->stats.\n");
+            return -1;
+        }
+
+        //fill stats array from log file
+        int frame, x, y;
+        char* ptrnext = avctx->stats_in;
+        int stat_index = 0;
+
+        while (ptrnext!=NULL) {
+            e = sscanf(ptrnext, "%d:%d:%d", &x, &y, &frame);
+            if(e != 3) {
+                av_log(avctx, AV_LOG_ERROR, " Problems parsing log file.\n");
+                return -1;
+            }
+            if(stat_index > count) {
+                av_log(avctx, AV_LOG_ERROR, " Too many frames in log file.\n");
+                return -1;
+            }
+
+            s->stats[stat_index].frame_num = stat_index+1;
+            s->stats[stat_index].blk_size_w = x;
+            s->stats[stat_index].blk_size_h = y;
+            stat_index++;
+            ptrnext=strchr(ptrnext,'\n')+1;
+            //check to see if at end of frames
+            if(frame==count) {
+                s->stat_count = count;
+                ptrnext=NULL;
+            }
+            //av_log(avctx, AV_LOG_ERROR, " frame %d   %d x %d.\n", frame, x, y);
+        }
+    }
+
     // Needed if zlib unused or init aborted before deflateInit
     memset(&(s->zstream), 0, sizeof(z_stream));
 
@@ -123,14 +195,88 @@ static int flashsv_encode_init(AVCodecCo
     s->tmpblock = av_mallocz(3*256*256);
     s->encbuffer = av_mallocz(s->image_width*s->image_height*3);
 
+    s->key_frame_cnt = 0;
+
     if (!s->tmpblock || !s->encbuffer) {
         av_log(avctx, AV_LOG_ERROR, "Memory allocation failed.\n");
         return -1;
     }
 
+    //setup of buffer to hold block size stat data
+    if(avctx->flags&CODEC_FLAG_PASS1){
+        s->tpSizes = av_mallocz(sizeof(int)*TP_BLCK_SIZE*TP_BLCK_SIZE);
+        avctx->stats_out = av_mallocz(256);
+
+        if (!s->tpSizes || !avctx->stats_out) {
+            av_log(avctx, AV_LOG_ERROR, "Memory allocation failed.\n");
+            return -1;
+        }
+    }
+
     return 0;
 }
 
+//performs the same actions as copy_region_enc but does not commit zlib data to buffer
+//and returns the size
+static int sample_block_size(FlashSVContext *s, AVFrame *p,
+        int block_width, int block_height, uint8_t *previous_frame, int* I_frame) {
+
+    int h_blocks, v_blocks, h_part, v_part, i, j;
+    //block header amount + block size over head
+    int blockSize = 6;
+    int res;
+
+    int comBnd = compressBound(block_height * block_width * 3);
+    uint8_t *buf = av_mallocz(comBnd);
+
+    if (!buf) {
+        av_log(s->avctx, AV_LOG_ERROR, "Memory allocation failed - blocksize compression buffer.\n");
+        return -1;
+    }
+
+
+    h_blocks = s->image_width / block_width;
+    h_part = s->image_width % block_width;
+    v_blocks = s->image_height / block_height;
+    v_part = s->image_height % block_height;
+
+    /* loop over all block columns */
+    for (j = 0; j < v_blocks + (v_part?1:0); j++)
+    {
+
+        int hp = j*block_height; // horiz position in frame
+        int hs = (j<v_blocks)?block_height:v_part; // size of block
+
+        /* loop over all block rows */
+        for (i = 0; i < h_blocks + (h_part?1:0); i++)
+        {
+            int wp = i*block_width; // vert position in frame
+            int ws = (i<h_blocks)?block_width:h_part; // size of block
+            int ret=Z_OK;
+
+            //copy the block to the temp buffer before compression (if it differs from the previous frame's block)
+            res = copy_region_enc(p->data[0], s->tmpblock, s->image_height-(hp+hs+1), wp, hs, ws, p->linesize[0], previous_frame);
+
+            if (res || *I_frame) {
+                unsigned long zsize;
+                zsize = comBnd;
+
+                ret = compress2(buf, &zsize, s->tmpblock, 3*ws*hs, 9);
+
+                if (ret != Z_OK)
+                    av_log(s->avctx, AV_LOG_ERROR, "error while compressing block %dx%d\n", i, j);
+
+                blockSize += zsize;
+            }
+        }
+    }
+    av_free(buf);
+
+    //av_log(s->avctx, AV_LOG_ERROR, "block size:  %d                             \n", blockSize);
+
+    return blockSize;
+}
+
 
 static int encode_bitstream(FlashSVContext *s, AVFrame *p, uint8_t *buf, int buf_size,
      int block_width, int block_height, uint8_t *previous_frame, int* I_frame) {
@@ -238,8 +384,79 @@ static int flashsv_encode_frame(AVCodecC
         }
     }
 
-    opt_w=4;
-    opt_h=4;
+    pfptr = s->previous_frame;
+    //if linesize is negative, prep pointer to match upside down ptr movement of data[0]
+    if(p->linesize[0] < 0) {
+        pfptr -= ((s->image_height-1) * p->linesize[0]);
+    }
+
+
+    int w, h;
+
+    //if first pass on a two pass, at each i-frame, record best block size for past i-frame set
+    if(avctx->flags&CODEC_FLAG_PASS1) {
+        if(avctx->frame_number != 0 && I_frame == 1){
+            //av_log(avctx, AV_LOG_INFO, "Reseting frame size counts \n");
+            int smallW, smallH;
+            smallW = 0;
+            smallH = 0;
+            int sizeIndex = 0;
+            unsigned int smallest = s->tpSizes[0];
+            for (h=0 ; h<TP_BLCK_SIZE ; h++) {
+                for (w=0 ; w<TP_BLCK_SIZE ; w++) {
+                    if (s->tpSizes[sizeIndex] < smallest) {
+                        smallest = s->tpSizes[sizeIndex];
+                        smallW = w;
+                        smallH = h;
+                    }
+                    sizeIndex++;
+                    //av_log(avctx, AV_LOG_ERROR, "[%d][%d]size = %d                       \n",w,h,s->tpSizes[h*TP_BLCK_SIZE+w]);
+                    s->tpSizes[h*TP_BLCK_SIZE+w] = 0;
+                    //av_log(avctx, AV_LOG_ERROR, "[%d][%d]size = %d                       \n",w,h,s->tpSizes[h*TP_BLCK_SIZE+w]);
+                }
+            }
+            //av_log(avctx, AV_LOG_INFO, "  [%d][%d] smallest = %d   --------\n",smallW+1,smallH+1,smallest);
+
+            s->key_frame_cnt++;
+            snprintf(avctx->stats_out, 256, "%d:%d:%d\n",smallW+1, smallH+1, s->key_frame_cnt);
+        } else {
+            avctx->stats_out[0] = 0;
+        }
+    }
+
+    //pull best block size from first past data
+    if(!(avctx->flags&CODEC_FLAG_PASS2)) {
+        opt_w=4;
+        opt_h=4;
+    } else {
+        if(avctx->frame_number != 0 && I_frame == 1){
+            s->key_frame_cnt++;
+        }
+        //av_log(avctx, AV_LOG_INFO, "  attempting to load frame stats info [%d]\n",s->key_frame_cnt);
+        if(s->key_frame_cnt < s->stat_count-1) {
+            opt_w=s->stats[s->key_frame_cnt].blk_size_w;
+            opt_h=s->stats[s->key_frame_cnt].blk_size_h;
+        } else {
+            opt_w=s->stats[s->stat_count-1].blk_size_w;
+            opt_h=s->stats[s->stat_count-1].blk_size_h;
+        }
+        if(I_frame == 1){
+            //av_log(avctx, AV_LOG_INFO, "  got %d    %d x %d \n", s->stats[s->key_frame_cnt].frame_num, opt_w, opt_h);
+        }
+    }
+
+    //calc frame size for different block sizes
+    if((avctx->flags&CODEC_FLAG_PASS1) && I_frame == 0){
+
+        //Try all possible combinations and store the encoded frame sizes
+        for (h=0 ; h<TP_BLCK_SIZE ; h++) {
+            for (w=0 ; w<TP_BLCK_SIZE ; w++) {
+                s->tpSizes[h*TP_BLCK_SIZE+w] += sample_block_size(s, p, (w+1)*16, (h+1)*16, pfptr, &I_frame);
+                //av_log(avctx, AV_LOG_ERROR, "[%d][%d]size = %d                        \n",w+1,h+1,s->tpSizes[h*TP_BLCK_SIZE+w]);
+            }
+        }
+    }
+
 
     if (buf_size < s->image_width*s->image_height*3) {
         //Conservative upper bound check for compressed data
@@ -280,6 +497,15 @@ static int flashsv_encode_end(AVCodecCon
     av_free(s->encbuffer);
     av_free(s->previous_frame);
     av_free(s->tmpblock);
+
+    if(avctx->flags&CODEC_FLAG_PASS1){
+        av_free(s->tpSizes);
+        av_free(avctx->stats_out);
+    }
+
+    if((avctx->flags&CODEC_FLAG_PASS2)){
+        av_free(s->stats);
+    }
 
     return 0;
 }



More information about the ffmpeg-devel mailing list