[FFmpeg-devel] [Patch] New filter -- dejudder

Lukasz Marek lukasz.m.luki at gmail.com
Wed Jan 29 23:05:59 CET 2014


>>>   +typedef struct {
>>>   +    const AVClass *class;
>>>   +    int64_t *ringbuff;
>>>   +    int a,b,c,d;
>>
>> Can they be more meaningful?
>
> They are indices to a circular buffer. I wanted them to be short. If it is preferable I could change them to two_previous, previous, ultimate, penultimate. It will make the "new_pts +=..." line in filter_frame longer.

OK. I don't mind, but a,b,c,d looks awful.

>
>
>>>   +    int64_t new,next;
>>
>> I know this is C, but I try to not use reserved words for C++. new can
>> be replaced anyway with something more specific, like new_pts?
>> And also, new can be moved to local variable in filter_frame, and next
>> variable seems to be not used at all.
>
> new changed to new_pts, it is carried over from one frame to the next, so can't be local to filter_frame. next removed.

My mistake, I overlooked there is += (I saw only assignments).

>
> Thank you for the review. As I said before, this is my first time contributing to a project like ffmpeg, so please pardon any rookie mistakes. I'm happy to learn how you all do things. I've attached the new patch.

It seems you attached old patch :P

-- 
Best Regards,
Lukasz Marek

Microsoft isn't evil, they just make really crappy operating systems. - 
Linus Torvalds


More information about the ffmpeg-devel mailing list