[FFmpeg-devel] [PATCH] Add DPX decoder rev-3

[Diego Biurrun]

On Thu, May 07, 2009 at 10:02:08AM +0200, Jimmy Christensen wrote:
> On 2009-05-06 14:27, Jimmy Christensen wrote:
> >
> >Best Regards
> >Jimmy Christensen
> >Developer
> >Ghost A/S
> >
> >
> >------------------------------------------------------------------------
> >
> >_______________________________________________
> >ffmpeg-devel mailing list
> >ffmpeg-devel at mplayerhq.hu
> >https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel

Please trim your quotes.

> Everything should be fixed now except using RGB48 instead of RGB24, 
> which seems a bit out of my reach.

Changelog and documentation updates are missing.

> --- libavcodec/dpx.c	(revision 0)
> +++ libavcodec/dpx.c	(revision 0)
> @@ -0,0 +1,166 @@
> +/*
> +
> +struct RGBField
> +{
> +
> +typedef struct DPXContext
> +{

struct declarations should have the { on the same line for K&R style

> +static int decode_frame(AVCodecContext *avctx, void *data, int *data_size,
> +        AVPacket *avpkt)

weird indentation

> +    const uint8_t *headerBuffer = avpkt->data;
> +    const uint8_t *buf = avpkt->data;
> +    int buf_size = avpkt->size;

Once more with feeling: Align columns where appropriate.  Do you intend
to continue to ignore this comment?

> +    DPXContext * const s = avctx->priv_data;
> +    AVFrame *picture = data;
> +    AVFrame * const p = (AVFrame*) &s->picture;

The cast seems pointless.

> +    // Check if the files "magic number" is "SDPX" which means it uses big endian or XPDS which is for little endian files

big-endian, little-endian

Please break overly long lines.

> +    s->width = w;
> +    s->height = h;
> +    s->bpp = 24;

align

> +AVCodec dpx_decoder =
> +{

{ on the same line

> +    .long_name = NULL_IF_CONFIG_SMALL("DPX images"),

image

Diego