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