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

Michael Niedermayer michaelni
Sat May 30 12:53:24 CEST 2009


On Fri, May 29, 2009 at 08:43:00AM +0200, Jimmy Christensen wrote:
> On 2009-05-28 12:52, Michael Niedermayer wrote:
>> On Thu, May 28, 2009 at 07:19:48AM +0200, Jimmy Christensen wrote:
[...]
>> [...]
>>> +    for (x = 0; x<  s->height; x++) {
>>> +        uint16_t *dst = ptr;
>>> +        for (y = 0; y<  s->width; y++) {
>>> +            rgbBuffer = AV_RB32(buf);
>>
>>> +            red16   = RED10(rgbBuffer) * 64; // 10-bit>  16-bit
>>> +            green16 = GREEN10(rgbBuffer) * 64; // 10-bit>  16-bit
>>> +            blue16  = BLUE10(rgbBuffer) * 64; // 10-bit>  16-bit
>>
>> there are 3 avoidable operations in there at least
>>
>
> Changed

no they are still there, the maximum is a bit wise and, and a shift for simple
convertion, you use a multiplication in addition. Also thats not to say this cant
be done with even less.
That said more correct would be a 
(x<<6)+(x>>4)
[the difference being that white (1023) is kept white (65535)]


[...]
> +#define RED10(x)   ((x & 0xFFC00000) >> 22)
> +#define GREEN10(x) ((x & 0x003FF000) >> 12)
> +#define BLUE10(x)  ((x & 0x00000FFC) >> 2)
> +
> +static int decode_frame(AVCodecContext *avctx,
> +                        void *data,
> +                        int *data_size,
> +                        AVPacket *avpkt)
> +{
> +
> +    const uint8_t *headerBuffer = avpkt->data;
> +    const uint8_t *buf          = avpkt->data;
> +    int buf_size                = avpkt->size;
> +    DPXContext *const s = avctx->priv_data;
> +    AVFrame *picture  = data;
> +    AVFrame *const p = &s->picture;
> +    uint8_t *ptr;
> +
> +    int magic_num, offset;
> +    char version[8];
> +    int x, y;
> +    int w, h, stride;
> +
> +    unsigned int rgbBuffer;
> +    int endian = 0;
> +
> +    magic_num = AV_RB32(headerBuffer);
> +    headerBuffer += 4;
> +
> +    /* Check if the files "magic number" is "SDPX" which means it uses
> +     * big-endian or XPDS which is for little-endian files */


> +    if (magic_num == MKTAG('S', 'D', 'P', 'X'))
> +        endian = 0;
> +    else if (magic_num == MKTAG('X', 'P', 'D', 'S'))

these things can be written like AV_RB32("SDPX")
IMHO thats slightly more readable


[...]
> +    buf += offset;
> +
> +    ptr    = p->data[0];
> +    stride = p->linesize[0];
> +
> +    for (x = 0; x < s->height; x++) {
> +        uint8_t *dst = ptr;
> +        for (y = 0; y < s->width; y++) {
> +            rgbBuffer = AV_RB32(buf);

> +            if(PIX_FMT_RGB48 == PIX_FMT_RGB48BE) {
> +                bytestream_put_be16(&dst, RED10(rgbBuffer) * 64); // 10-bit > 16-bit
> +                bytestream_put_be16(&dst, GREEN10(rgbBuffer) * 64); // 10-bit > 16-bit
> +                bytestream_put_be16(&dst, BLUE10(rgbBuffer) * 64); // 10-bit > 16-bit
> +            } else {
> +                bytestream_put_le16(&dst, RED10(rgbBuffer) * 64); // 10-bit > 16-bit
> +                bytestream_put_le16(&dst, GREEN10(rgbBuffer) * 64); // 10-bit > 16-bit
> +                bytestream_put_le16(&dst, BLUE10(rgbBuffer) * 64); // 10-bit > 16-bit
> +            }

*dst16++ = (rgbBuffer & 0x...) >> ..;
*dst16++ = (rgbBuffer & 0x...) >> ..;
*dst16++ = (rgbBuffer & 0x...) >> ..;


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090530/0356e6f5/attachment.pgp>



More information about the ffmpeg-devel mailing list