Hi On Tue, Mar 6, 2012 at 10:39 PM, Florent Delannoy <[email protected]> wrote: > Hello Einar, > > Sorry about taking so long to integrate your feedback. The amended code is > v0.2 ob GitHub.
Thank you. I look at the code later > > I addressed all the issues you mentioned, apart from three which I'm unsure > about: > >> As I understand there is no need for c41.h and c41window.h > Those headers files are needed because the two files are coupled — obviously > the plugin needs to access its window, but the window and its thread also > need to know about C41main. I don't see how to delete those headers without > moving all the code to c41.C, which would make it a way longer and harder to > read file. My knowledge of linking in C and C++ is quite rusty, so if you > know of a better way to achieve this, let me know and I'll implement it. c41.C contains all needed - including the class C41Window. Of course the c41window.C is unneeded too. > >> Allocate everything once if needed and delete in destructor > The matrices I get there have to be of the frame size, and as far as I know > the frame size isn't known at instantiation time, it depends on the frame > passed to process_buffer. The frame is described in preferences/format and may change only if user changes the format. I recommend to use something like PluginVClient::new_temp in process_buffer. The method itself can be used for one frame. > >> Use BC_Title instead of C41TextBox, to reflect the fact that min_r, >> min_g... are unchangeable > On purely subjective grounds, I think it makes more sense visually to have > an unmodifiable TextBox instead of a changing "title". That way, the user > can see immediately that these variables must be computed somehow, whereas > having them as a title could lead to thing they're static. > The rest of Cinelerra uses BC_Title - better to be consistent Einar _______________________________________________ Cinelerra mailing list [email protected] https://init.linpro.no/mailman/skolelinux.no/listinfo/cinelerra
