Review Request: Added search path in FindFFmpeg.cmake

[Alex Merry]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100014/#review65
-----------------------------------------------------------

Ship it!


I think this is fine to be committed, providing the path order issues are fixed.

The other comments I had apply equally to the existing file.


cmake/modules/FindFFmpeg.cmake
<http://git.reviewboard.kde.org/r/100014/#comment44>

    You've changed the order of paths to search for includes, but not for libraries.  It should be the same for both, or people may run into issues if they have different versions in different locations.



cmake/modules/FindFFmpeg.cmake
<http://git.reviewboard.kde.org/r/100014/#comment46>

    Do we need all these random paths at all?
    
    As far as I'm aware, /usr/include, /usr/local/include and /opt/local/include should be included automatically, and anything else should probably be specified by users with CMAKE_PREFIX_PATH or CMAKE_INCLUDE_PATH environment variables or cmake arguments.
    
    Also, what's the reasoning behing searching all subdirectories, given we've got the subdirectory hints?



cmake/modules/FindFFmpeg.cmake
<http://git.reviewboard.kde.org/r/100014/#comment45>

    As above, with respect to path orders.


- Alex


On 2010-10-03 19:44:55, James Duncan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100014/
> -----------------------------------------------------------
> 
> (Updated 2010-10-03 19:44:55)
> 
> 
> Review request for amarok.
> 
> 
> Summary
> -------
> 
> Added another search path suffix for when CMake is looking for
> libavcodec and libavformat.  Also, cleaned up the module a little.
> 
> 
> Diffs
> -----
> 
>   cmake/modules/FindFFmpeg.cmake 6767d1b 
>   src/CMakeLists.txt 07e08b9 
> 
> Diff: http://git.reviewboard.kde.org/r/100014/diff
> 
> 
> Testing
> -------
> 
> I can still configure and compile successfully, so this fix should
> not affect currently working systems.
> 
> 
> Thanks,
> 
> James
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/amarok-devel/attachments/20101003/455cfe2e/attachment-0001.htm