-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6312/#review9607
-----------------------------------------------------------


When disabling of the video part is cleaned up / fixed, you can simplify your 
patch a lot:

Just do

  if ( NOT DISABLE_VIDEOSUPPORT )
    find_package(Jasper)
    macro_log_feature(...)
  endif

in CMakeLists.txt and skip (do not include) webcamimgformat.*



- Frank


On Jan. 9, 2011, 10:35 p.m., Cristi P wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6312/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2011, 10:35 p.m.)
> 
> 
> Review request for Kopete.
> 
> 
> Summary
> -------
> 
> I saw that for yahoo - webcam images are converted by running an external 
> program. Personally I am not ok with this since running an external program 
> every 0.X seconds is a bit expensive. Also, not to mention using disk files 
> (at least pushing directly into the program and reading its output would have 
> been better).
> Also - I saw that inviteWebcam action was checking for presence of jasper 
> program - but requestWebcam should also have done that. Not to mention that 
> if a yahoo contact just invites you to see his cam, you might get a feedback 
> of missing program after accepting to invitation. Not necessarily a big deal, 
> though.
> 
> Note that this will mean that having jasper lib at compile time will get to 
> be mandatory - and needs to also be added as dependency by package managers.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/CMakeLists.txt 
> 1212994 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/webcamimgformat.h 
> PRE-CREATION 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/webcamimgformat.cpp 
> PRE-CREATION 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/libkyahoo/webcamtask.cpp 
> 1212994 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/yahoocontact.cpp 1212994 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/yahoowebcam.h 1212994 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/yahoowebcam.cpp 1212994 
>   trunk/KDE/kdenetwork/kopete/CMakeLists.txt 1212994 
>   trunk/KDE/kdenetwork/kopete/config-kopete.h.cmake 1212994 
>   trunk/KDE/kdenetwork/kopete/protocols/yahoo/CMakeLists.txt 1212994 
> 
> Diff: http://svn.reviewboard.kde.org/r/6312/diff
> 
> 
> Testing
> -------
> 
> compiled and ran it. Communicating with another linux kopete. Tried both 
> directions of seeing the cam.
> 
> 
> Thanks,
> 
> Cristi
> 
>

_______________________________________________
kopete-devel mailing list
kopete-devel@kde.org
https://mail.kde.org/mailman/listinfo/kopete-devel

Reply via email to