[FFmpeg-devel] [PATCH] Add DPX decoder rev-13
Vitor Sessak
vitor1001
Tue Jun 2 19:36:01 CEST 2009
Jimmy Christensen wrote:
> On 2009-05-30 12:53, Michael Niedermayer wrote:
>> 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)]
>>
>>
>> [...]
>
> Misunderstood the first time, but you are are indeed correct. Would
> still keep it out of the RED/GREEN/BLUE10 macros so it's easier to
> change the output format to eg. RGB96 (32-bit per color) if/when that
> gets added.
>
>>
>>
>>> + 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
>>
>
> True, changed that.
>
>>
>> [...]
>>> + 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...)>> ..;
>>
>
> Does makes code simpler, but can it always be guarantied that
> PIX_FMT_RGB48 is PIX_FMT_RGB48BE on big endian systems and
> PIX_FMT_RGB48LE on little endian systems?
Quoting libavutils/pix_fmt.h
> #ifdef WORDS_BIGENDIAN
> # define PIX_FMT_NE(be, le) PIX_FMT_##be
> #else
> # define PIX_FMT_NE(be, le) PIX_FMT_##le
> #endif
>
> #define PIX_FMT_RGB32 PIX_FMT_NE(ARGB, BGRA)
> #define PIX_FMT_RGB32_1 PIX_FMT_NE(RGBA, ABGR)
> #define PIX_FMT_BGR32 PIX_FMT_NE(ABGR, RGBA)
> #define PIX_FMT_BGR32_1 PIX_FMT_NE(BGRA, ARGB)
>
> #define PIX_FMT_GRAY16 PIX_FMT_NE(GRAY16BE, GRAY16LE)
> #define PIX_FMT_RGB48 PIX_FMT_NE(RGB48BE, RGB48LE)
The whole point of having three pix formats is having one for big
endian, one for little endian and one for native endianness.
-Vitor
More information about the ffmpeg-devel
mailing list