[FFmpeg-devel] [PATCH] CCITT Group 3 and 4 decompression

Michael Niedermayer michaelni
Wed Dec 24 23:00:08 CET 2008


On Wed, Dec 24, 2008 at 08:06:13AM +0200, Kostya wrote:
> On Tue, Dec 23, 2008 at 08:27:31PM +0100, Michael Niedermayer wrote:
> > On Tue, Dec 23, 2008 at 09:00:57AM +0200, Kostya wrote:
> > > On Mon, Dec 22, 2008 at 09:06:37PM +0100, Michael Niedermayer wrote:
> > > > On Mon, Dec 22, 2008 at 08:15:06PM +0200, Kostya wrote:
> > > > > On Mon, Dec 22, 2008 at 01:13:53PM +0100, Michael Niedermayer wrote:
> > > > > > On Mon, Dec 22, 2008 at 08:31:55AM +0200, Kostya wrote:
> > > > > > > I need that for resolving issue 750 on roundup.
> [...]
> > > > 
> > > > how is this supposed to work?
> > > > once a line is damaged and lost there can be 1 line less thus this can
> > > > still run out of data and fail.
> > > 
> > > it's the question of what to do with damaged data 
> > 
> > what do you prefer, no image or a image with a missing line?
> > or did i misunderstand you?
>  
> we have three cases here:
> 1. group 3 1-d only - we have syncmarkers and each line is decoded independently
> 2. group 3 2-d - we have lines coded independently followed by K lines coded
>     depending on the previous line
> 3. group 4 - all lines are coded depending on previous ones
> 
> so, in first two cases we can show broken image, in the third one it's better
> to give up (which current code does now)

[...]

> +static int decode_group3_1d_line(AVCodecContext *avctx, GetBitContext *gb,
> +                                 int pix_left, int *runs)
> +{
> +    int mode = 0, run = 0;
> +    unsigned int t;

> +    while(pix_left){

first check

> +        t = get_vlc2(gb, ccitt_vlc[mode].table, 9, 2);
> +        run += t;

> +        if(t < 64){

second check


> +            pix_left -= run;

> +            if(pix_left < 0){

third check

thats still 1 more than needed


[...]
> +
> +static void put_line(uint8_t *dst, int size, int width, const int *runs)
> +{
> +    PutBitContext pb;
> +    int run, mode = 0, pix_left = width, run_idx = 0;
> +
> +    init_put_bits(&pb, dst, size*8);
> +    if(!runs[0]){
> +        run_idx++;
> +        mode = !mode;
> +    }

> +    while(pix_left > 0){
> +        run = runs[run_idx++];
> +        if(!run || run_idx >= width)
> +            return;

how can run_idx >= width happen here?


> +        pix_left -= run;
> +        for(; run > 16; run -= 16)
> +            put_sbits(&pb, 16, -mode);
> +        put_sbits(&pb, run, -mode);
> +        mode = !mode;
> +    }
> +}
> +

> +static int find_group3_syncmarker(GetBitContext *gb, int srcsize)
> +{
> +    int state = get_bits(gb, 12);
> +    int rem = srcsize - get_bits_count(gb);
> +    while((state & 0xFFF) != 1){
> +        state = (state << 1) | get_bits1(gb);
> +        if(--rem <= 0)
> +            return -1;
> +    }
> +    return 0;
> +}

you only need 1 get_bits call in this function
besides this code is buggy, it will miss the sync code when its at
the end, that is it will never check the last read bit
Ive already provided a working implementation, why dont you use it?


> +
> +int ff_ccitt_unpack_group3_1d(AVCodecContext *avctx,
> +                              const uint8_t *src, int srcsize,
> +                              uint8_t *dst, int height, int stride)
> +{
> +    int j;
> +    GetBitContext gb;
> +    int *runs;
> +
> +    runs = av_malloc(avctx->width * sizeof(runs[0]));
> +    init_get_bits(&gb, src, srcsize*8);
> +    for(j = 0; j < height; j++){
> +        if(find_group3_syncmarker(&gb, srcsize*8) < 0)
> +            break;
> +        if(decode_group3_1d_line(avctx, &gb, avctx->width, runs) < 0){
> +            av_log(avctx, AV_LOG_ERROR, "Error decoding line\n");
> +            continue;
> +        }
> +        put_line(dst, stride, avctx->width, runs);
> +        dst += stride;
> +    }

the code should ideally duplicate the last good line for broken lines instead
of loosing lines.


> +    av_free(runs);
> +    return 0;
> +}
> +

> +static int ccitt_unpack2d(AVCodecContext *avctx,
> +                          const uint8_t *src, int srcsize,
> +                          uint8_t *dst, int height, int stride,
> +                          int g4)
> +{
> +    int j;
> +    GetBitContext gb;
> +    int *runs, *ref;
> +    int ret;
> +
> +    runs = av_malloc((avctx->width + 1) * sizeof(runs[0]));
> +    ref  = av_malloc((avctx->width + 1) * sizeof(ref[0]));
> +    runs[0] = 0;
> +    ref[0] = 0;
> +    ref[1] = avctx->width;
> +    init_get_bits(&gb, src, srcsize*8);
> +    for(j = 0; j < height; j++){

> +        if(g4){
> +            ret = decode_group3_2d_line(avctx, &gb, avctx->width, runs + 1, ref);
> +        }else{
> +            if(find_group3_syncmarker(&gb, srcsize*8) < 0)
> +                break;
> +            if(get_bits1(&gb))
> +                ret = decode_group3_1d_line(avctx, &gb, avctx->width, runs + 1);
> +            else
> +                ret = decode_group3_2d_line(avctx, &gb, avctx->width, runs + 1, ref);
> +        }
> +        if(ret < 0 && g4){
> +            av_free(runs);
> +            av_free(ref);
> +            return -1;
> +        }

this ret<0 check can be simplified by moving it under the if(g4) above


> +        put_line(dst, stride, avctx->width, runs + 1);
> +        FFSWAP(int*, runs, ref);
> +        dst += stride;
> +    }

this is missing an error message, ff_ccitt_unpack_group3_1d() prints one ...


> +    av_free(runs);
> +    av_free(ref);
> +    return 0;
> +}
> +

> +int ff_ccitt_unpack_group3_2d(AVCodecContext *avctx,
> +                              const uint8_t *src, int srcsize,
> +                              uint8_t *dst, int height, int stride)
> +{
> +    return ccitt_unpack2d(avctx, src, srcsize, dst, height, stride, 0);
> +}
> +
> +int ff_ccitt_unpack_group4(AVCodecContext *avctx,
> +                           const uint8_t *src, int srcsize,
> +                           uint8_t *dst, int height, int stride)
> +{
> +    return ccitt_unpack2d(avctx, src, srcsize, dst, height, stride, 1);
> +}

useless wraper functions

btw, do you have some url where i can find files to test this code?

PS: merry christmas :)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- 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/20081224/31468c8f/attachment.pgp>



More information about the ffmpeg-devel mailing list