On Wed, Jun 20, 2012 at 11:24:21AM -0400, Derek Buitenhuis wrote:
> > +#define MODEL_MIN_SYMS 2
> > +#define MODEL_MAX_SYMS 256
>
> y u no align?
It is, just on left edge ;)
I'll realign before committing
> > +#define THRESH_ADAPTIVE -1
> > +#define THRESH_LOW 15
> > +#define THRESH_HIGH 50
> > +#define THRESH_KOSTYA 1985
>
> Will this be in the final push? I don't actually see
> it being used anywhere. :P
on IRC ;)
> > +static int arith_get_bit(ArithCoder *c)
> > +{
> > + int range = c->high - c->low + 1;
> > + int bit = (((c->value - c->low) << 1) + 1) / range;
> > +
> > + if (bit)
> > + c->low += range >> 1;
> > + else
> > + c->high = c->low + (range >> 1) - 1;
> > +
> > + arith_normalise(c);
> > +
> > + return bit;
> > +}
> > +
> > +static int arith_get_bits(ArithCoder *c, int bits)
> > +{
> > + int range = c->high - c->low + 1;
> > + int val = (((c->value - c->low + 1) << bits) - 1) / range;
> > + int prob = range * val;
> > +
> > + c->high = ((prob + range) >> bits) + c->low - 1;
> > + c->low += prob >> bits;
> > +
> > + arith_normalise(c);
> > +
> > + return val;
> > +}
> > +
> > +static int arith_get_number(ArithCoder *c, int mod_val)
> > +{
> > + int range = c->high - c->low + 1;
> > + int val = ((c->value - c->low + 1) * mod_val - 1) / range;
> > + int prob = range * val;
> > +
> > + c->high = (prob + range) / mod_val + c->low - 1;
> > + c->low += prob / mod_val;
> > +
> > + arith_normalise(c);
> > +
> > + return val;
> > +}
> > +
> > +static int arith_get_prob(ArithCoder *c, int *probs)
> > +{
> > + int range = c->high - c->low + 1;
> > + int val = ((c->value - c->low + 1) * probs[0] - 1) / range;
> > + int sym = 1;
> > +
> > + while (probs[sym] > val)
> > + sym++;
> > +
> > + c->high = range * probs[sym - 1] / probs[0] + c->low - 1;
> > + c->low += range * probs[sym] / probs[0];
> > +
> > + return sym;
> > +}
>
> Is there any way to merge parts of these?
There is:
int arith_decode(){
int range = c->high - c->low + 1;
int val = ((c->value - c->low + 1) * SOMEVAL - 1) / range;
// determine symbol
// calculate SOMEVAL1
// calculate SOMEVAL2
c->high = range * SOMEVAL1 / SOMEVAL + c->low - 1;
c->low += range * SOMEVAL2 / SOMEVAL;
return sym;
}
In my opinion it won't give much and will obfuscate the code greatly.
> > +enum ContextDirection {
> > + TOP_LEFT = 0,
> > + TOP,
> > + TOP_RIGHT,
> > + LEFT
> > +};
>
> This should be moved to the top of the file, no?
Maybe. I'll move it there.
> > +static int decode_region(MSS1Context *ctx, ArithCoder *acoder, uint8_t
> > *dst,
> > + int x, int y, int width, int height, int stride,
> > + PixContext *pctx)
> > +{
> > + int i, j;
> > +
> > + dst += x + y * stride;
> > +
> > + dst[0] = decode_top_left_pixel(acoder, pctx);
> > + for (j = 0; j < height; j++) {
> > + for (i = 0; i < width; i++) {
> > + if (!i && !j)
> > + continue;
> > +
> > + dst[i] = decode_pixel_in_context(acoder, pctx, dst + i, stride,
> > + i, j, width - i - 1);
> > + }
> > + dst += stride;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int decode_region_masked(MSS1Context *ctx, ArithCoder *acoder,
> > + uint8_t *dst, int stride, uint8_t *mask,
> > + int mask_stride, int x, int y,
> > + int width, int height,
> > + PixContext *pctx)
> > +{
> > + int i, j;
> > +
> > + dst += x + y * stride;
> > + mask += x + y * mask_stride;
> > +
> > + if (mask[0] != 0xFF)
> > + dst[0] = decode_top_left_pixel(acoder, pctx);
> > + for (j = 0; j < height; j++) {
> > + for (i = 0; i < width; i++) {
> > + if (!i && !j || mask[i] != 0xFF)
> > + continue;
> > +
> > + dst[i] = decode_pixel_in_context(acoder, pctx, dst + i, stride,
> > + i, j, width - i - 1);
> > + }
> > + dst += stride;
> > + mask += mask_stride;
> > + }
> > +
> > + return 0;
> > +}
>
> Possible merge?
Yes, but for speed reasons it's better to keep them separate.
> > +AVCodec ff_mss1_decoder = {
> > + .name = "mss1",
> > + .type = AVMEDIA_TYPE_VIDEO,
> > + .id = CODEC_ID_MSS1,
> > + .priv_data_size = sizeof(MSS1Context),
> > + .init = mss1_decode_init,
> > + .close = mss1_decode_end,
> > + .decode = mss1_decode_frame,
> > + .capabilities = CODEC_CAP_DR1,
> > + .long_name = NULL_IF_CONFIG_SMALL("MS Screen 1"),
> > +};
>
> I thought it was called MS Screen Codec 1? I'm probably misremembering.
It's known under many names, and quite probably 'codec' is not an official
part of it. msscds32.ax calls itself "Microsoft Screen Video Decompressor"
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
