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

[Jimmy Christensen]

On 2009-06-03 12:28, Michael Niedermayer wrote:
> On Wed, Jun 03, 2009 at 11:56:44AM +0200, Jimmy Christensen wrote:
>> On 2009-06-03 11:33, Michael Niedermayer wrote:
>>> 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:
>>>>> [...]
>>>>>> +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
>>>
>>
>> Here is the patch with the changes. I'm not good at working with
>> bitshifting so I don't know how I can combine the bit shifting and masking
>> in 1 go instead of 2.
>
> heres an example:
>
> RRRGGGBBB&  000111000 ->000GGG000
> 000GGG000>>  2 ->  00000GGG0
> 000GGG000>>  5 ->  00000000G
> 00000GGG0 + 00000000G ->  00000GGGG
>
> does that help or should i fix the mask&shifts?
>

Thanks! That actually helped alot and I think I know what to do now.

Hopefully this would be the last change needed to be accepted :)