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

[Michael Niedermayer]

On Wed, Jun 03, 2009 at 09:16:39AM +0200, Jimmy Christensen wrote:
> On 2009-06-02 22:21, Michael Niedermayer wrote:
>> On Tue, Jun 02, 2009 at 10:18:02AM +0200, Jimmy Christensen wrote:
>>> On 2009-06-02 10:09, Jimmy Christensen wrote:
>>>> On 2009-06-02 10:06, Diego Biurrun wrote:
>>>>> On Tue, Jun 02, 2009 at 09:16:06AM +0200, Jimmy Christensen wrote:
>>>>>> --- Changelog (revision 18866)
>>>>>> +++ Changelog (working copy)
>>>>>> @@ -18,6 +18,7 @@
>>>>>> - 8088flex TMV demuxer and decoder
>>>>>> - per-stream language-tags extraction in asfdec
>>>>>> - V210 decoder and encoder
>>>>>> +- DPX image decoder
>>>>>
>>>>> This is not HEAD.
>>>>>
>>>>> Diego
>>>>
>>>> I know. The problem is somewhat that I need to use a RGB48 patch which
>>>> doesn't work with HEAD to test the decoder. If the patch is OK'ed I will
>>>> gladly make a patch which is compatible with HEAD.
>>>
>>> Made a new patch which works with HEAD.
>>
>> [...]
>>> +typedef struct DPXContext {
>>> +    AVFrame picture;
>>
>>> +    int width;
>>> +    int height;
>>
>
> Thanks. Will remove those and change the code accordingly.
>
>> redundant with AVCodecContext.width/...
>>
>> [...]
>>> +            if(avctx->pix_fmt == PIX_FMT_RGB48BE) {
>>> +                bytestream_put_be16(&dst, MAKE10to16(RED10(rgbBuffer)));
>>> +                bytestream_put_be16(&dst, 
>>> MAKE10to16(GREEN10(rgbBuffer)));
>>> +                bytestream_put_be16(&dst, 
>>> MAKE10to16(BLUE10(rgbBuffer)));
>>> +            } else {
>>> +                bytestream_put_le16(&dst, MAKE10to16(RED10(rgbBuffer)));
>>> +                bytestream_put_le16(&dst, 
>>> MAKE10to16(GREEN10(rgbBuffer)));
>>> +                bytestream_put_le16(&dst, 
>>> MAKE10to16(BLUE10(rgbBuffer)));
>>
>> this contains at least 6 unneeded shifts
>>
> Like this instead ?
>
> *dst++ = MAKE10to16(RED10(rgbBuffer));
> *dst++ = MAKE10to16(GREEN10(rgbBuffer));
> *dst++ = MAKE10to16(BLUE10(rgbBuffer));

That is a good change as well


>
> or do you mean combining the MAKE10to16 with RED10? Personally I think that 
> would make the code less readable.

well you have to remove the unneeded operations, how you do that is your
problem, droping all these macros is the cleanest way to write this IMHO
but its up to you how you do it, it just must be optimal in terms of
speed

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20090603/d51b34e4/attachment.pgp>