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

[Jimmy Christensen]

On 2009-06-08 16:12, Michael Niedermayer wrote:
> On Mon, Jun 08, 2009 at 12:17:32AM +0200, Jimmy Christensen wrote:
> [...]
>> [...]
>> +    w = read32(&buf, endian);
>> +    h = read32(&buf, endian);
>> +
>> +    buf = avpkt->data + 0x320;
>
> buf+= 8 is a lot clearer here IMHO
>

Have changed it and made a comment that it needs to end in 0x320. Same 
for the 0x323.

> [...]
>> +    switch (bits_per_color) {
>> +        case 8:
>> +            if (elements == 4)
>> +                avctx->pix_fmt = PIX_FMT_RGBA;
>> +            else
>> +                avctx->pix_fmt = PIX_FMT_RGB24;
>> +            source_packet_size = elements;
>> +            target_packet_size = elements;
>> +            break;
>> +        case 10:
>> +        case 12:
>> +        case 16:
>> +            if (endian)
>> +                avctx->pix_fmt = PIX_FMT_RGB48LE;
>> +            else
>> +                avctx->pix_fmt = PIX_FMT_RGB48BE;
>> +            target_packet_size = 6;
>> +            source_packet_size = elements * 2;
>> +            break;
>> +        default:
>> +            av_log(avctx, AV_LOG_ERROR, "Unsupported color depth\n");
>> +            return -1;
>> +            break;
>> +    }
>> +
>> +    if (s->picture.data[0])
>> +        avctx->release_buffer(avctx,&s->picture);
>> +    if (avcodec_check_dimensions(avctx, w, h))
>> +        return -1;
>> +    if (w != avctx->width || h != avctx->height)
>> +        avcodec_set_dimensions(avctx, w, h);
>> +    if (avctx->get_buffer(avctx, p)<  0) {
>> +        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
>> +        return -1;
>> +    }
>> +
>> +    // Move pointer to offset from start of file
>> +    buf =  avpkt->data + offset;
>> +
>> +    ptr    = p->data[0];
>> +    stride = p->linesize[0];
>> +
>> +    switch (bits_per_color) {
>> +        case 10:
>> +            for (x = 0; x<  avctx->height; x++) {
>> +               uint16_t *dst = ptr;
>> +               for (y = 0; y<  avctx->width; y++) {
>> +                   rgbBuffer = read32(&buf, endian);
>> +                   // Read out the 10-bit colors and convert to 16-bit
>> +                   *dst++ = make_16bit(rgbBuffer>>  16);
>> +                   *dst++ = make_16bit(rgbBuffer>>   6);
>> +                   *dst++ = make_16bit(rgbBuffer<<   4);
>> +               }
>> +               ptr += stride;
>> +            }
>> +            break;
>> +        case 8:
>> +        case 12: // Treat 12-bit as 16-bit
>> +        case 16:
>> +            if (source_packet_size == target_packet_size) {
>> +                for (x = 0; x<  avctx->height; x++) {
>> +                    memcpy(ptr, buf, target_packet_size*avctx->width);
>> +                    ptr += stride;
>> +                    buf += source_packet_size*avctx->width;
>> +                }
>> +            } else {
>> +                for (x = 0; x<  avctx->height; x++) {
>> +                    uint8_t *dst = ptr;
>> +                    for (y = 0; y<  avctx->width; y++) {
>> +                        memcpy(dst, buf, target_packet_size);
>> +                        dst += target_packet_size;
>> +                        buf += source_packet_size;
>> +                    }
>> +                    ptr += stride;
>> +                }
>> +            }
>> +            break;
>> +        default:
>> +            return -1;
>> +            break;
>> +    }
>
> this code looks buggy, mixing LE&  BE up
>

You're right. I found that I had switched BE and LE in the first switch 
and in the 10-bit case I needed to use the systems native endian type. I 
have to agree that when I looked at it again, something didn't seem 
right. Thanks :)

It's supposed to make the output pixel format according to the input 
format, except for 10bit files which needs the bit shifting etc. so 
it'll end up with the systems native endian type.

Otherwise the code is solid :)

I've tested with the following dpx files :
  From Eyeon Fusion 5.31 build 80 (doesn't have any option for big vs. 
little endian output):

8-bit no alpha
8-bit with alpha
10-bit
16-bit no alpha
16-bit with alpha

  From The Foundry Nuke 5.1v5 :
8-bit no alpha big endian
8-bit no alpha little endian
8-bit with alpha little endian
8-bit with alpha big endian
10-bit no alpha big endian
10-bit no alpha little endian
16-bit no alpha big endian
16-bit no alpha little endian
16-bit with alpha little endian
16-bit with alpha big endian

They all have the same output. Can upload the samples + reference if 
you'd like.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpegDPX-rev22.diff
Type: text/x-patch
Size: 10499 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090608/4ca8e240/attachment.bin>