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

Jimmy Christensen jimmy
Thu May 28 07:19:48 CEST 2009


On 2009-05-27 21:18, Vitor Sessak wrote:
> Jimmy Christensen wrote:
>> On 2009-05-27 14:30, Michael Niedermayer wrote:
>>> On Tue, May 26, 2009 at 05:51:25AM +0200, Jimmy Christensen wrote:
>>>> On 2009-05-25 23:58, Michael Niedermayer wrote:
>>>>> On Mon, May 25, 2009 at 08:54:12AM +0200, Jimmy Christensen wrote:
>>>>>> On 2009-05-15 03:51, Michael Niedermayer wrote:
>>>>>>> On Mon, May 11, 2009 at 11:31:25AM +0200, Jimmy Christensen wrote:
>>>>> [...]
>>>>>>>
>>>>>>>
>>>>>>> [...]
>>>>>>>> + for (x = 0; x< s->height; x++) {
>>>>>>>> + uint8_t *dst = ptr;
>>>>>>>> + for (y = 0; y< s->width; y++) {
>>>>>>>
>>>>>>>> + rgbBuffer = AV_RB32(buf);
>>>>>>>> + memcpy(&rgb10Field,&rgbBuffer, 4);
>>>>>>>> + rgb16Field.R = rgb10Field.R * 64; // 10-bit> 16-bit
>>>>>>>> + rgb16Field.G = rgb10Field.G * 64; // 10-bit> 16-bit
>>>>>>>> + rgb16Field.B = rgb10Field.B * 64; // 10-bit> 16-bit
>>>>>>>> + memcpy(dst,&rgb16Field, dstBpp);
>>>>>>>
>>>>>>> not portable
>>>>>>>
>>>>>>
>>>>>> I rewrote a little, but still uses memcpy from rgb16Field to dst,
>>>>>> since
>>>>>> that should actually be portable.
>>>>>
>>>>> you will have to quote page and pararaph of the C standard that
>>>>> gurantees
>>>>> bitfields to work the way you seem to belive they do
>>>>>
>>>>
>>>> So the bit masking is fine (RED10, GREEN10 and BLUE10), but I need to
>>>> change the memcpy?
>>>
>>> we need C code that works, any construct that has undefined or
>>> implementation
>>> defined behavior is unacceptable. That is with a few exceptions that
>>> happen
>>> to be exceedingly inconvenient to avoid or would cause speedloss and
>>> happen
>>> to work on all for us relevant implementations, like
>>> signed>> or CHAR_BIT == 8
>>>
>>
>> I'm not familiar with coding on other platforms and what code is
>> portable or not. I'm just seeing that it works here and try to get
>> some input on how to fix the code to make it portable. So forgive my
>> lack of knowledge. I was merely asking for a yes or no.
>
> I suggest you remove completely the RGB16Field struct and just use
>
>> + for (x = 0; x < s->height; x++) {
>> + uint16_t *dst = ptr;
>
> instead of
>
>> + for (x = 0; x < s->height; x++) {
>> + uint8_t *dst = ptr;
>
> as a secondary benefit, the code should be simpler (no memcpy()).
>
> -Vitor

Thank you. Didn't know about this.

Solved my problem and the code should be alot better and cleaner. 
Although there is 1 warning about changing format from uint8_t to 
uint16_t. It seems that avctx->priv_data->picture->data is always 
uint8_t so either way it seems I'll get a warning no matter what. Also 
since avctx->priv_data->picture->linesize is based on a uint8_t I'll 
need it as uint8_t anyway.

I also forced the avctx->pix_fmt to PIX_FMT_RGB48BE to avoid any 
portable issues and also since this is the only format which is 
supported by any encoders(pnm).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpegDPX-rev11.diff
Type: text/x-patch
Size: 8627 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090528/2c5fe214/attachment.bin>



More information about the ffmpeg-devel mailing list