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

[Michael Niedermayer]

On Thu, Jun 04, 2009 at 07:59:04AM +0200, Jimmy Christensen wrote:
> On 2009-06-04 00:34, Michael Niedermayer wrote:
>> On Wed, Jun 03, 2009 at 08:52:42PM +0200, Jimmy Christensen wrote:
>>> On 2009-06-03 20:20, Reimar D?ffinger wrote:
>>>> Hm. I really wonder which compiler you use.
>>>> Anyway I guess Michael would accept a version using macros if it was
>>>> done without adding useless calculation.
>>>> Except that it's rather silly to use macros in cases like this when
>>>> there is absolutely no reason to.
>>>> Example:
>>>> static inline unsigned make_16bit(unsigned value)
>>>> {
>>>>       // mask away invalid bits
>>>>       value&= 0xFFC0;
>>>>       // correctly expand to 16 bits
>>>>       return value + (value>>   10);
>>>> }
>>>>
>>>>               *dst++ = make_16bit(rgbBuffer>>   16);
>>>>               *dst++ = make_16bit(rgbBuffer>>    6);
>>>>               *dst++ = make_16bit(rgbBuffer<<    4);
>>>
>>> Thanks for the code. Works perfectly and keeps it's speed. I'm using gcc
>>> 4.3.3 on Ubuntu 9.04 amd64. C is not exactly my biggest force and
>>> appreciate any help I can get.
>>>
>>> Btw. should I also upload sample images to test with?
>>
>> hmm, dont we already have pngs somewhere ...
>> anyway, uploading more surely doesnt hurt ...
>
> pngs? This is a decoder for dpx files :) Have uploaded some samples to the 

i wonder if my brain is still under warranty


> samples.mplayerhq.hu ftp. Both 10bit and 16bit samples.

thanks


>
>>
>>
>> [...]
>>> +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;
>>
>> redundant
>>
>
> It was to make jumping from the start of the file to the offset easier. 
> Found another way to do it instead.
>
>> [...]
>>> +
>>
>>> +    avctx->width  = w;
>>> +    avctx->height = h;
>> [...]
>>> +    if (w != avctx->width || h != avctx->height)
>>> +        avcodec_set_dimensions(avctx, w, h);
>>
>> this wont work in that order
>>
>
> Removed those 2 lines, as they did seem kinda rendundant aswell.
>
> Added support for 16-bit dpx files aswell. I know the code looks redundant, 
> but I didn't want to make a if or switch case for each pixel. With the 
> switch case inside the for loops it took a performance hit of about 6-7% of 
> total encode time which I found a little unacceptable. Also the 10bit files 
> needs to first go into a temporary buffer for the color information to get 
> extracted. With 16-bit files it's not necessary so that path should also be 
> a little faster. Haven't tested encodes with 16-bit sequences, only single 
> files.
>
> Removed the version variable since it wasn't used anyway.

[...]
> +static int decode_frame(AVCodecContext *avctx,
> +                        void *data,
> +                        int *data_size,
> +                        AVPacket *avpkt)
> +{
> +    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;
> +    int x, y;
> +    int w, h, stride, bpc;
> +
> +    unsigned int rgbBuffer;

> +    int endian = 0;

redundant init


> +
> +    magic_num = AV_RB32(buf);
> +    buf += 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 == AV_RL32("SDPX"))
> +        endian = 0;
> +    else if (magic_num == AV_RB32("SDPX"))
> +        endian = 1;
> +    else {
> +        av_log(avctx, AV_LOG_ERROR, "DPX marker not found");
> +        return -1;
> +    }
> +
> +    offset = read32(&buf, endian);
> +    // Need to end in 0x304 offset from start of file
> +    buf = avpkt->data + 0x304;
> +    w = read32(&buf, endian);
> +    h = read32(&buf, endian);
> +
> +    buf = avpkt->data + 0x322;

> +    bpc = AV_RB16(buf);

what is bpc? i think this is not a good name for a variable

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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/20090604/e9bb7df0/attachment.pgp>