On Mon, Mar 10, 2014 at 06:00:53PM +0100, Vittorio Giovara wrote:
> From: Paul B Mahol <[email protected]>
> 
> Signed-off-by: Vittorio Giovara <[email protected]>
> ---
>  doc/general.texi       |    2 +
>  libavcodec/Makefile    |    1 +
>  libavcodec/allcodecs.c |    1 +
>  libavcodec/avcodec.h   |    1 +
>  libavcodec/sanm.c      | 1307 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/sanm_data.h |  254 ++++++++++
>  libavcodec/version.h   |    4 +-
>  7 files changed, 1568 insertions(+), 2 deletions(-)
>  create mode 100644 libavcodec/sanm.c
>  create mode 100644 libavcodec/sanm_data.h

Does this compile standalone?

> diff --git a/doc/general.texi b/doc/general.texi
> index 6bc7fda..42f7b98 100644
> --- a/doc/general.texi
> +++ b/doc/general.texi
> @@ -575,6 +575,8 @@ following image formats are supported:
>  @item LCL (LossLess Codec Library) MSZH  @tab     @tab  X
>  @item LCL (LossLess Codec Library) ZLIB  @tab  E  @tab  E
>  @item LOCO                   @tab     @tab  X
> +@item LucasArts SANM         @tab     @tab  X
> +    @ Used in LucasArts SMUSH animations.

@tab

> --- /dev/null
> +++ b/libavcodec/sanm.c
> @@ -0,0 +1,1307 @@
> +
> +#define NGLYPHS 256
> +
> +typedef struct {
> +} SANMVideoContext;
> +
> +typedef struct {
> +} SANMFrameHeader;

Please name the structs.

> +static enum GlyphEdge which_edge(int x, int y, int edge_size)
> +{
> +    const int edge_max = edge_size - 1;
> +
> +    if (!y) {
> +        return BOTTOM_EDGE;
> +    } else if (y == edge_max) {
> +        return TOP_EDGE;
> +    } else if (!x) {
> +        return LEFT_EDGE;
> +    } else if (x == edge_max) {
> +        return RIGHT_EDGE;
> +    } else {
> +        return NO_EDGE;
> +    }

Drop {}.

> +static enum GlyphDir which_direction(enum GlyphEdge edge0, enum GlyphEdge 
> edge1)
> +{
> +    if ((edge0 == LEFT_EDGE && edge1 == RIGHT_EDGE) ||
> +        (edge1 == LEFT_EDGE && edge0 == RIGHT_EDGE) ||
> +        (edge0 == BOTTOM_EDGE && edge1 != TOP_EDGE) ||
> +        (edge1 == BOTTOM_EDGE && edge0 != TOP_EDGE)) {
> +        return DIR_UP;
> +    } else if ((edge0 == TOP_EDGE && edge1 != BOTTOM_EDGE) ||
> +               (edge1 == TOP_EDGE && edge0 != BOTTOM_EDGE)) {
> +        return DIR_DOWN;
> +    } else if ((edge0 == LEFT_EDGE && edge1 != RIGHT_EDGE) ||
> +               (edge1 == LEFT_EDGE && edge0 != RIGHT_EDGE)) {
> +        return DIR_LEFT;
> +    } else if ((edge0 == TOP_EDGE && edge1 == BOTTOM_EDGE) ||
> +               (edge1 == TOP_EDGE && edge0 == BOTTOM_EDGE) ||
> +               (edge0 == RIGHT_EDGE && edge1 != LEFT_EDGE) ||
> +               (edge1 == RIGHT_EDGE && edge0 != LEFT_EDGE)) {
> +        return DIR_RIGHT;
> +    }

same

> +/**
> + * Interpolate two points.
> + */
> +static void interp_point(int8_t *points, int x0, int y0, int x1, int y1,
> +                         int pos, int npoints)

This should not be a Doxygen comment IMO.

> +/**
> + * Construct glyphs by iterating through vectors coordinates.

Either "a vector's coordinates" or "vector coordinates".

> + * @param pglyphs pointer to table where glyphs are stored
> + * @param xvec pointer to x component of vectors coordinates
> + * @param yvec pointer to y component of vectors coordinates

same

> +    for (i = 0; i < GLYPH_COORD_VECT_SIZE; i++) {
> +        int x0    = xvec[i];
> +        int y0    = yvec[i];
> +        enum GlyphEdge edge0 = which_edge(x0, y0, side_length);

stray extra space

> +        for (j = 0; j < GLYPH_COORD_VECT_SIZE; j++, pglyph += glyph_size) {
> +            int x1      = xvec[j];
> +            int y1      = yvec[j];

same

> +            enum GlyphEdge edge1   = which_edge(x1, y1, side_length);
> +            enum GlyphDir  dir     = which_direction(edge0, edge1);

same

> +static av_cold int init_buffers(SANMVideoContext *ctx)
> +{
> +    av_fast_padded_malloc(&ctx->frm0, &ctx->frm0_size, ctx->buf_size);
> +    av_fast_padded_malloc(&ctx->frm1, &ctx->frm1_size, ctx->buf_size);
> +    av_fast_padded_malloc(&ctx->frm2, &ctx->frm2_size, ctx->buf_size);
> +    if (!ctx->version)
> +        av_fast_padded_malloc(&ctx->stored_frame, &ctx->stored_frame_size, 
> ctx->buf_size);
> +
> +    if (!ctx->frm0 || !ctx->frm1 || !ctx->frm2 || (!ctx->stored_frame && 
> !ctx->version)) {

nit: long lines

> +static void rotate_bufs(SANMVideoContext *ctx, int rotate_code)
> +{
> +    av_dlog(ctx->avctx, "rotate %d\n", rotate_code);

Pointless leftover debug printf?

> +            if ((pos + i) < 0 || (pos + i) >= height * stride)

pointless ()

> +    dst  = ((uint8_t*)ctx->frm0) + left + top * stride;
> +    prev = ((uint8_t*)ctx->frm2) + left + top * stride;

dst  = (uint8_t *) ctx->frm0 + left + top * stride;
prev = (uint8_t *) ctx->frm2 + left + top * stride;

> +    av_dlog(ctx->avctx, "compression %d\n", compr);

Leftover debug printf?

> +                            mx = c37_mv[(mvoff * 255 + code) * 2    ];
> +                            my = c37_mv[(mvoff * 255 + code) * 2 + 1];

Drop spaces inside [] like you do below.

> +    } else {
> +        int mx = motion_vectors[code][0];
> +        int my = motion_vectors[code][1];
> +        int index = prev2 - (const uint8_t*)ctx->frm2;

space after cast and before *

> +    uint8_t *dst   = ((uint8_t*)ctx->frm0) + left + top * stride;
> +    uint8_t *prev1 = (uint8_t*)ctx->frm1;
> +    uint8_t *prev2 = (uint8_t*)ctx->frm2;

same, and drop extra parentheses

> +static int process_frame_obj(SANMVideoContext *ctx)
> +{
> +    uint16_t codec, top, left, w, h;
> +
> +    codec = bytestream2_get_le16u(&ctx->gb);
> +    left  = bytestream2_get_le16u(&ctx->gb);
> +    top   = bytestream2_get_le16u(&ctx->gb);
> +    w     = bytestream2_get_le16u(&ctx->gb);
> +    h     = bytestream2_get_le16u(&ctx->gb);

You could merge declaration and initialization here.

> +    av_dlog(ctx->avctx, "subcodec %d\n", codec);

IMO pointless leftover debug printf.

> +    default:
> +        avpriv_request_sample(ctx->avctx, "unknown subcodec %d", codec);

Capitalize "unknown".

> +static int decode_nop(SANMVideoContext *ctx)
> +{
> +    avpriv_request_sample(ctx->avctx, "unknown/unsupported compression 
> type");

same

> +static void copy_block(uint16_t *pdest, uint16_t *psrc, int block_size, int 
> pitch)
> +{
> +    uint8_t *dst = (uint8_t *)pdest;
> +    uint8_t *src = (uint8_t *)psrc;

space after cast

> +static int good_mvec(SANMVideoContext *ctx, int cx, int cy, int mx, int my,
> +                     int block_size)
> +{
> +    int start_pos = cx + mx + (cy + my) * ctx->pitch;
> +    int end_pos = start_pos + (block_size - 1) * (ctx->pitch + 1);
> +
> +    int good = start_pos >= 0 && end_pos < (ctx->buf_size >> 1);
> +
> +    if (!good) {
> +        av_log(ctx->avctx, AV_LOG_ERROR, "ignoring invalid motion vector 
> (%i, %i)->(%u, %u), block size = %u\n",
> +               cx + mx, cy + my, cx, cy, block_size);
> +    }

nit: long line, drop {}

> +    av_dlog(ctx->avctx, "opcode 0x%0X cx %d cy %d blk %d\n", opcode, cx, cy, 
> blk_size);

see above

> +static int decode_5(SANMVideoContext *ctx)
> +{
> +#if HAVE_BIGENDIAN
> +    uint16_t *frm;
> +    int npixels;
> +#endif
> +    uint8_t *dst = (uint8_t*)ctx->frm0;

space after cast

> +    if ((ret = bytestream2_get_bytes_left(&ctx->gb)) < 560) {
> +        av_log(ctx->avctx, AV_LOG_ERROR, "too short input frame (%d 
> bytes)\n",

input frame too short

> +    if (hdr->width != ctx->width || hdr->height != ctx->height) {
> +        av_log(ctx->avctx, AV_LOG_ERROR, "variable size frames are not 
> implemented\n");
> +        return AVERROR_PATCHWELCOME;

avpriv_report_missing_feature()

> +    av_dlog(ctx->avctx, "subcodec %d\n", hdr->codec);

see above

> +                    if (size < 768 * 2 + 4) {
> +                    }
> +                    for (i = 0; i < 768; i++)
> +                        ctx->delta_pal[i] = bytestream2_get_le16u(&ctx->gb);
> +                    if (size >= 768 * 5 + 4) {
> +                        for (i = 0; i < 256; i++)

The 768 and a few other numbers look pretty magicky to me.

> +    } else {
> +        } else {
> +            avpriv_request_sample(avctx, "subcodec %d",
> +                                  header.codec);

This fits on one line and capitalize.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to