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

[Michael Niedermayer]

On Thu, Jun 11, 2009 at 05:59:56AM +0200, Jimmy Christensen wrote:
> On 2009-06-11 00:51, Michael Niedermayer wrote:
>> On Wed, Jun 10, 2009 at 06:17:00PM +0200, Jimmy Christensen wrote:
>>> On 2009-06-10 16:48, Diego Biurrun wrote:
>>>> On Wed, Jun 10, 2009 at 02:36:58PM +0200, Jimmy Christensen wrote:
>>>>>
>>>>> I was wondering how close this DPX patch is to get accepted. Is the
>>>>> general quality good enough or does it need to be approved a lot more?
>>>>
>>>> Close.
>>>>
>>>
>>> Thanks. Have started looking into the 3D lut filter for doing log/lin
>>> conversion. A bit challenging and slow as hell but have already gotten 
>>> some
>>> promising results.
>>>
>>>>> --- libavcodec/dpx.c	(revision 0)
>>>>> +++ libavcodec/dpx.c	(revision 0)
>>>>> @@ -0,0 +1,227 @@
>>>>> +static unsigned int read32(const uint8_t **ptr, int is_big)
>>>>> +{
>>>>> +    unsigned int temp;
>>>>> +    if(is_big)
>>>>
>>>> if (
>>>>
>>>
>>> Fixed.
>>>
>> [...]
>>
>>> +    /* Check if the files "magic number" is "SDPX" which means it uses
>>> +     * big-endian or XPDS which is for little-endian files */
>>> +    if (magic_num == AV_RL32("SDPX"))
>>> +        endian = 0;
>>> +    else if (magic_num == AV_RB32("SDPX"))
>>> +        endian = 1;
>>> +    else {
>>
>> please add {} here they need no extra lines and they make future patches
>> smaller
>
> Added, although in the beginning I was told to remove more or less those 
> exact {} by Diego :)
>
> From Diego :
>>> > +	if(magic_num == 0x53445058) {
>>> > +		endian = 0x0;
>>> > +	}
>> useless {}
>
> But does that go for all one line if's or just that pair?

it goes for all one line if() without a following else

[...]

>  Changelog              |    1 
>  doc/general.texi       |    2 
>  libavcodec/Makefile    |    1 
>  libavcodec/allcodecs.c |    1 
>  libavcodec/avcodec.h   |    1 
>  libavcodec/dpx.c       |  225 +++++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/img2.c     |    1 
>  libavformat/isom.c     |    1 
>  libavformat/riff.c     |    1 
>  9 files changed, 234 insertions(+)
> 74baf44cae0502adb12e7cbd8e788d9f94483997  ffmpegDPX-rev25.diff

looks ok assuming noone else has any more comments

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- 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/20090611/2865f1cb/attachment.pgp>