On Wed, Jan 18, 2012 at 6:08 AM, Laurentiu Cristian Ion
<[email protected]> wrote:
>
> ---
> libavcodec/dxa.c | 173 +++++++++++++++++++++++++++++------------------------
> 1 files changed, 95 insertions(+), 78 deletions(-)
>
> diff --git a/libavcodec/dxa.c b/libavcodec/dxa.c
> index 97b912a..e7bd4af 100644
> --- a/libavcodec/dxa.c
> +++ b/libavcodec/dxa.c
> @@ -29,6 +29,7 @@
>
> #include "libavutil/intreadwrite.h"
> #include "avcodec.h"
> +#include "bytestream.h"
>
> #include <zlib.h>
>
> @@ -39,36 +40,52 @@ typedef struct DxaDecContext {
> AVCodecContext *avctx;
> AVFrame pic, prev;
>
> - int dsize;
> + unsigned long decomp_size;
> + int allocated_decomp_size;
> uint8_t *decomp_buf;
> uint32_t pal[256];
> + GetByteContext g;
> } DxaDecContext;
>
> static const int shift1[6] = { 0, 8, 8, 8, 4, 4 };
> static const int shift2[6] = { 0, 0, 8, 4, 0, 4 };
>
> -static int decode_13(AVCodecContext *avctx, DxaDecContext *c, uint8_t* dst,
> uint8_t *src, uint8_t *ref)
> +static int decode_13(DxaDecContext *c, uint8_t* dst, uint8_t *ref)
uint8_t *dst
>
> {
> - uint8_t *code, *data, *mv, *msk, *tmp, *tmp2;
> + GetByteContext data, mv, msk;
> + int mv_offset, msk_offset;
> + uint8_t tmp3;
> + uint8_t t[4];
> + uint8_t *tmp, *tmp2;
> int i, j, k;
> int type, x, y, d, d2;
> int stride = c->pic.linesize[0];
> uint32_t mask;
>
> - code = src + 12;
> - data = code + ((avctx->width * avctx->height) >> 4);
> - mv = data + AV_RB32(src + 0);
> - msk = mv + AV_RB32(src + 4);
> + mv_offset = bytestream2_get_be32(&c->g);
> + msk_offset = bytestream2_get_be32(&c->g);
>
>
> + bytestream2_skip(&c->g, 4);
> + data = c->g;
> + bytestream2_skip(&data, (c->avctx->width * c->avctx->height) >> 4);
> + mv = data;
> + bytestream2_skip(&mv, mv_offset);
> + msk = mv;
> + bytestream2_skip(&msk, msk_offset);
>
> - for(j = 0; j < avctx->height; j += 4){
> - for(i = 0; i < avctx->width; i += 4){
> + for(j = 0; j < c->avctx->height; j += 4){
> + for(i = 0; i < c->avctx->width; i += 4){
> tmp = dst + i;
> tmp2 = ref + i;
> - type = *code++;
> + type = bytestream2_get_byte(&c->g);
> switch(type){
> case 4: // motion compensation
> - x = (*mv) >> 4; if(x & 8) x = 8 - x;
> - y = (*mv++) & 0xF; if(y & 8) y = 8 - y;
> + tmp3 = bytestream2_get_byte(&mv);
> + x = (tmp3) >> 4;
We don't need the parenthesis.
x = tmp3 >> 4
>
> + if(x & 8)
if (condition)
>
> + x = 8 - x;
> + y = (tmp3) & 0xF;
See my above comment about parenthesis.
>
> + if(y & 8)
if (condition)
>
> + y = 8 - y;
> tmp2 += x + y*stride;
> case 0: // skip
> case 5: // skip in method 12
> @@ -85,17 +102,16 @@ static int decode_13(AVCodecContext *avctx,
> DxaDecContext *c, uint8_t* dst, uint
> case 13:
> case 14:
> case 15:
> - if(type == 1){
> - mask = AV_RB16(msk);
> - msk += 2;
> - }else{
> + if(type == 1)
Same
>
> + mask = bytestream2_get_be16(&msk);
> + else{
else {
>
> + tmp3 = bytestream2_get_byte(&msk);
> type -= 10;
> - mask = ((msk[0] & 0xF0) << shift1[type]) | ((msk[0] &
> 0xF) << shift2[type]);
> - msk++;
> + mask = ((tmp3 & 0xF0) << shift1[type]) | ((tmp3 & 0xF)
> << shift2[type]);
> }
> for(y = 0; y < 4; y++){
> for(x = 0; x < 4; x++){
> - tmp[x] = (mask & 0x8000) ? *data++ : tmp2[x];
> + tmp[x] = (mask & 0x8000) ?
> bytestream2_get_byte(&data) : tmp2[x];
> mask <<= 1;
> }
> tmp += stride;
> @@ -103,29 +119,33 @@ static int decode_13(AVCodecContext *avctx,
> DxaDecContext *c, uint8_t* dst, uint
> }
> break;
> case 2: // fill block
> + tmp3 = bytestream2_get_byte(&data);
> for(y = 0; y < 4; y++){
> - memset(tmp, data[0], 4);
> + memset(tmp, tmp3, 4);
> tmp += stride;
> }
> - data++;
> break;
> case 3: // raw block
> for(y = 0; y < 4; y++){
> - memcpy(tmp, data, 4);
> - data += 4;
> + bytestream2_get_buffer(&data, tmp, 4);
> tmp += stride;
> }
> break;
> case 8: // subblocks - method 13 only
> - mask = *msk++;
> + mask = bytestream2_get_byte(&msk);
> for(k = 0; k < 4; k++){
> d = ((k & 1) << 1) + ((k & 2) * stride);
> d2 = ((k & 1) << 1) + ((k & 2) * stride);
> tmp2 = ref + i + d2;
> switch(mask & 0xC0){
> case 0x80: // motion compensation
> - x = (*mv) >> 4; if(x & 8) x = 8 - x;
> - y = (*mv++) & 0xF; if(y & 8) y = 8 - y;
> + tmp3 = bytestream2_get_byte(&mv);
> + x = (tmp3) >> 4;
Parenthesis.
>
> + if(x & 8)
if (condition)
>
> + x = 8 - x;
> + y = (tmp3) & 0xF;
Parenthesis.
>
> + if(y & 8)
if (condition)
>
> + y = 8 - y;
> tmp2 += x + y*stride;
> case 0x00: // skip
> tmp[d + 0 ] = tmp2[0];
> @@ -134,51 +154,51 @@ static int decode_13(AVCodecContext *avctx,
> DxaDecContext *c, uint8_t* dst, uint
> tmp[d + 1 + stride] = tmp2[1 + stride];
> break;
> case 0x40: // fill
> - tmp[d + 0 ] = data[0];
> - tmp[d + 1 ] = data[0];
> - tmp[d + 0 + stride] = data[0];
> - tmp[d + 1 + stride] = data[0];
> - data++;
> + tmp3 = bytestream2_get_byte(&data);
> + tmp[d + 0 ] = tmp3;
> + tmp[d + 1 ] = tmp3;
> + tmp[d + 0 + stride] = tmp3;
> + tmp[d + 1 + stride] = tmp3;
>
> break;
>
> case 0xC0: // raw
> - tmp[d + 0 ] = *data++;
> - tmp[d + 1 ] = *data++;
> - tmp[d + 0 + stride] = *data++;
> - tmp[d + 1 + stride] = *data++;
> + tmp[d ] = bytestream2_get_be16(&data);
> + tmp[d + stride] = bytestream2_get_be16(&data);
> break;
> }
> mask <<= 2;
> }
> break;
> case 32: // vector quantization - 2 colors
> - mask = AV_RB16(msk);
> - msk += 2;
> + mask = bytestream2_get_be16(&msk);
> + t[0] = bytestream2_get_byte(&data);
> + t[1] = bytestream2_get_byte(&data);
> for(y = 0; y < 4; y++){
> for(x = 0; x < 4; x++){
> - tmp[x] = data[mask & 1];
> + tmp[x] = t[mask & 1];
> mask >>= 1;
> }
> tmp += stride;
> tmp2 += stride;
> }
> - data += 2;
> break;
> case 33: // vector quantization - 3 or 4 colors
> case 34:
> - mask = AV_RB32(msk);
> - msk += 4;
> + mask = bytestream2_get_be32(&msk);
> + t[0] = bytestream2_get_byte(&data);
> + t[1] = bytestream2_get_byte(&data);
> + t[2] = bytestream2_get_byte(&data);
> + t[3] = bytestream2_get_byte(&data);
> for(y = 0; y < 4; y++){
> for(x = 0; x < 4; x++){
> - tmp[x] = data[mask & 3];
> + tmp[x] = t[mask & 3];
> mask >>= 2;
> }
> tmp += stride;
> tmp2 += stride;
> }
> - data += type - 30;
> break;
> default:
> - av_log(avctx, AV_LOG_ERROR, "Unknown opcode %d\n", type);
> + av_log(c->avctx, AV_LOG_ERROR, "Unknown opcode %d\n", type);
> return -1;
> }
> }
> @@ -188,31 +208,23 @@ static int decode_13(AVCodecContext *avctx,
> DxaDecContext *c, uint8_t* dst, uint
> return 0;
> }
>
> -static int decode_frame(AVCodecContext *avctx, void *data, int *data_size,
> AVPacket *avpkt)
> +static int decode_frame(AVCodecContext *avctx,
> + void *data, int *data_size, AVPacket *avpkt)
> {
> - const uint8_t *buf = avpkt->data;
> - int buf_size = avpkt->size;
> DxaDecContext * const c = avctx->priv_data;
> - uint8_t *outptr, *srcptr, *tmpptr;
> - unsigned long dsize;
> + uint8_t *outptr, *tmpptr;
> int i, j, compr;
> int stride;
> - int orig_buf_size = buf_size;
> int pc = 0;
>
> - /* make the palette available on the way out */
> - if(buf[0]=='C' && buf[1]=='M' && buf[2]=='A' && buf[3]=='P'){
> - int r, g, b;
> + bytestream2_init(&c->g, avpkt->data, avpkt->size);
>
> - buf += 4;
> - for(i = 0; i < 256; i++){
> - r = *buf++;
> - g = *buf++;
> - b = *buf++;
> - c->pal[i] = (r << 16) | (g << 8) | b;
> - }
> + /* make the palette available on the way out */
> + if(bytestream2_peek_le32(&c->g) == MKTAG('C','M','A','P')) {
MKTAG('C', 'M', 'A', 'P')
Note the spaces between the arguments.
> + bytestream2_skip(&c->g, 4);
> + for(i = 0; i < 256; i++)
> + c->pal[i] = bytestream2_get_be24(&c->g);
> pc = 1;
> - buf_size -= 768+4;
> }
>
> if(avctx->get_buffer(avctx, &c->pic) < 0){
> @@ -223,20 +235,27 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *data_size, AVPac
> c->pic.palette_has_changed = pc;
>
> outptr = c->pic.data[0];
> - srcptr = c->decomp_buf;
> tmpptr = c->prev.data[0];
> stride = c->pic.linesize[0];
>
> - if(buf[0]=='N' && buf[1]=='U' && buf[2]=='L' && buf[3]=='L')
> + if(bytestream2_get_le32(&c->g) == MKTAG('N','U','L','L')) {
Same
> compr = -1;
> - else
> - compr = buf[4];
> + }
> + else {
> + compr = bytestream2_get_byte(&c->g);
> + bytestream2_skip(&c->g, 4);
> + }
>
> - dsize = c->dsize;
> - if((compr != 4 && compr != -1) && uncompress(c->decomp_buf, &dsize, buf
> + 9, buf_size - 9) != Z_OK){
> - av_log(avctx, AV_LOG_ERROR, "Uncompress failed!\n");
> - return -1;
> + if((compr != 4 && compr != -1)){
> + c->decomp_size = c->allocated_decomp_size;
> + if(uncompress(c->decomp_buf, &c->decomp_size,
> + avpkt->data + bytestream2_tell(&c->g),
> + bytestream2_get_bytes_left(&c->g)) != Z_OK) {
if(uncompress(c->decomp_buf, &c->decomp_size,
avpkt->data + bytestream2_tell(&c->g),
bytestream2_get_bytes_left(&c->g)) != Z_OK) {
See how I align the arguments , scope-wise.
> + av_log(avctx, AV_LOG_ERROR, "Uncompress failed!\n");
> + return -1;
> + }
> }
> + bytestream2_init(&c->g, c->decomp_buf, c->decomp_size);
> switch(compr){
> case -1:
> c->pic.key_frame = 0;
> @@ -258,22 +277,21 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *data_size, AVPac
> for(j = 0; j < avctx->height; j++){
> if(compr & 1){
> for(i = 0; i < avctx->width; i++)
> - outptr[i] = srcptr[i] ^ tmpptr[i];
> + outptr[i] = bytestream2_get_byte(&c->g) ^ tmpptr[i];
> tmpptr += stride;
> }else
> - memcpy(outptr, srcptr, avctx->width);
> + bytestream2_get_buffer(&c->g, outptr, avctx->width);
> outptr += stride;
> - srcptr += avctx->width;
> }
> break;
> case 12: // ScummVM coding
> case 13:
> c->pic.key_frame = 0;
> c->pic.pict_type = AV_PICTURE_TYPE_P;
> - decode_13(avctx, c, c->pic.data[0], srcptr, c->prev.data[0]);
> + decode_13(c, c->pic.data[0], c->prev.data[0]);
> break;
> default:
> - av_log(avctx, AV_LOG_ERROR, "Unknown/unsupported compression type
> %d\n", buf[4]);
> + av_log(avctx, AV_LOG_ERROR, "Unknown/unsupported compression type
> %d\n", compr);
> return -1;
> }
>
> @@ -285,7 +303,7 @@ static int decode_frame(AVCodecContext *avctx, void
> *data, int *data_size, AVPac
> *(AVFrame*)data = c->prev;
>
> /* always report that the buffer was completely consumed */
> - return orig_buf_size;
> + return avpkt->size;
> }
>
> static av_cold int decode_init(AVCodecContext *avctx)
> @@ -295,8 +313,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
> c->avctx = avctx;
> avctx->pix_fmt = PIX_FMT_PAL8;
>
> - c->dsize = avctx->width * avctx->height * 2;
> - if((c->decomp_buf = av_malloc(c->dsize)) == NULL) {
> + c->allocated_decomp_size = avctx->width * avctx->height * 2;
> + if((c->decomp_buf = av_malloc(c->allocated_decomp_size)) == NULL) {
> av_log(avctx, AV_LOG_ERROR, "Can't allocate decompression
> buffer.\n");
> return -1;
> }
> @@ -328,4 +346,3 @@ AVCodec ff_dxa_decoder = {
> .capabilities = CODEC_CAP_DR1,
> .long_name = NULL_IF_CONFIG_SMALL("Feeble Files/ScummVM DXA"),
> };
> -
> --
> 1.7.5.4
>
>
> _______________________________________________
> libav-devel mailing list
> [email protected]
> https://lists.libav.org/mailman/listinfo/libav-devel
>
I am not sure , If I pointed out all of the issues , but check for
similar issues (which I pointed above) and resend the patch and I
don't mind to do a review again.
--
Thanks
Aneesh Dogra (lionaneesh)
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
