[Bug 1413646] Review Request: librealsense - Cross-platform camera capture for Intel RealSense

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1413646



--- Comment #3 from Till Hofmann <till.hofmann@xxxxxxxxx> ---
Spec URL: https://thofmann.fedorapeople.org/librealsense.spec
SRPM URL: https://thofmann.fedorapeople.org/librealsense-1.12.1-4.fc25.src.rpm

Thank you for your comments!

(In reply to Mamoru TASAKA from comment #1)
> Some initial comments:
> 
> * License
>  
> https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/
> FAQ#How_should_I_handle_multiple_licensing_situations.3F
>   - src/libuvc/ is under BSD and librealsense uses (links) files under
>     this directory, so the license tag should be "ASL 2.0 and BSD"

Missed that, fixed.

> 
> * Compiler flags
>   https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
>   - Changing optimization level is discouraged. Currently -Ofast
>     overrides -O2 in %optflags. e.g.
> 
> /usr/bin/c++   -DRS_USE_V4L2_BACKEND -DUNICODE -Drealsense_EXPORTS -isystem
> /usr/include/libusb-1.0 -I/builddir/build/BUILD/librealsense-1.12.1/include 
> -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2
> -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4
> -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64
> -mtune=generic -std=c++11 -fPIC -pedantic -g -Ofast
> -Wno-missing-field-initializers -Wno-switch -Wno-multichar -mssse3 -fPIC  
> -o CMakeFiles/realsense.dir/src/context.cpp.o -c
> /builddir/build/BUILD/librealsense-1.12.1/src/context.cpp
> 
>     If you really needs -Ofast, you need to write such reason on
>     the spec file, otherwide please remove this.
>     ! Note that -O3 enables some standard-NONcompliant optimizations
>       such as -funsafe-math-optimizations and it is highly discouraged.
> 
>   - Also -mssse3 is added by default, this make it impossible to use
>     this library on the platform not supporting ssse3, and to my
>     understanding Fedora bans this type of changing supported 
>     architecture.

I'm currently discussing this in
https://github.com/IntelRealSense/librealsense/pull/422 whether they can add a
flag that disables any modification of the compiler flags. If not, I'll stick
with the new patch which simply removes all CFLAGS related stuff from
CMakeLists (except the C++11 check).

> 
> * No Trademark
>  
> https://fedoraproject.org/wiki/Packaging:
> Guidelines#Trademarks_in_Summary_or_Description
>   - Remove ® ™ or so from %description

Oops, fixed.

I didn't think the CLA would be an issue as long as the code itself is still
licensed under a good license.

But let's wait for FE-Legal on their opinion about the CLA.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]