Review Request: Simple and noticeable improvements to the capture demos

Casian Andrei skeletk13 at gmail.com
Wed Feb 16 16:39:55 GMT 2011



> On Feb. 14, 2011, 4:24 p.m., Trever Fischer wrote:
> >

I actually merged the changes yesterday. I'll delete this request soon.

Thx for comments :)


> On Feb. 14, 2011, 4:24 p.m., Trever Fischer wrote:
> > cmake/FindPhononInternal.cmake, line 340
> > <http://git.reviewboard.kde.org/r/100653/diff/1/?file=9136#file9136line340>
> >
> >     This bit might be better left for a separate commit, or at least explain the rationale for such a change. I'm not a windows dev so I can't say if this was warranted or not.

I didn't make that code removal. Maybe it's a diff glitch or something. It has no sense in this context, as you say.


> On Feb. 14, 2011, 4:24 p.m., Trever Fischer wrote:
> > demos/CMakeLists.txt, line 5
> > <http://git.reviewboard.kde.org/r/100653/diff/1/?file=9137#file9137line5>
> >
> >     It isn't immediately clear what the difference is between captureapp and captureapp2. Demos are supposed to enlighten, not confuse. Perhaps merge the two?

You are correct. However, they actually do the same thing in two different ways. It would be nice to merge them, when I'll have time.


> On Feb. 14, 2011, 4:24 p.m., Trever Fischer wrote:
> > demos/captureapp/CMakeLists.txt, line 4
> > <http://git.reviewboard.kde.org/r/100653/diff/1/?file=9138#file9138line4>
> >
> >     -g is already set when CMAKE_BUILD_TYPE is set to Debug.

ooops, will take care of this


- Casian


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


On Feb. 14, 2011, 8:50 a.m., Casian Andrei wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100653/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2011, 8:50 a.m.)
> 
> 
> Review request for Phonon.
> 
> 
> Summary
> -------
> 
> Some trivial but noticeable improvements for the capture stuff.
> 
> Moved the "test applications" to demos, fixed a couple of includes, fix build with capture disabled, and some small improvements in CMakeLists.
> 
> It should be okay, however this should be tested. Who knows what strange compilation errors may occur on other machines, although there's not much modification.
> 
> Added simple descriptions to the capture demos.
> 
> And modified the ugly QT_NO_PHONON_VIDEOCAPTURE to the more sensible PHONON_NO_VIDEOCAPTURE. Same for the audio one.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt b3c76e7 
>   cmake/FindPhononInternal.cmake 80c0f23 
>   demos/CMakeLists.txt b4690c8 
>   demos/captureapp/CMakeLists.txt PRE-CREATION 
>   demos/captureapp/captureapp.h PRE-CREATION 
>   demos/captureapp/captureapp.cpp PRE-CREATION 
>   demos/captureapp/captureapp_main.cpp PRE-CREATION 
>   demos/captureapp2/CMakeLists.txt PRE-CREATION 
>   demos/captureapp2/captureapp2.h PRE-CREATION 
>   demos/captureapp2/captureapp2.cpp PRE-CREATION 
>   demos/captureapp2/captureapp2_main.cpp PRE-CREATION 
>   phonon/CMakeLists.txt 3f1fd26 
>   phonon/backendcapabilities.h bb70a5b 
>   phonon/backendcapabilities.cpp 8c950b1 
>   phonon/experimental/avcapture.h a2d6af8 
>   phonon/experimental/avcapture.cpp 4fb76e2 
>   phonon/experimental/backendcapabilities.h 80ec00b 
>   phonon/experimental/backendcapabilities.cpp c5b1b04 
>   phonon/experimental/globalconfig.h db89b80 
>   phonon/experimental/globalconfig.cpp 1e985c3 
>   phonon/experimental/mediasource.h a68446f 
>   phonon/experimental/mediasource.cpp df48baa 
>   phonon/experimental/objectdescription.h 643d105 
>   phonon/experimental/objectdescription.cpp 31fd74f 
>   phonon/experimental/tests/CMakeLists.txt 3b13f7b 
>   phonon/experimental/tests/avcaptureapptest/CMakeLists.txt 264d016 
>   phonon/experimental/tests/avcaptureapptest/capture_test.h 3854a96 
>   phonon/experimental/tests/avcaptureapptest/capture_test.cpp a078a40 
>   phonon/experimental/tests/avcaptureapptest/capture_test_main.cpp 29674b4 
>   phonon/experimental/tests/avcapturetest.cpp e0449aa 
>   phonon/globalconfig.h d5ac877 
>   phonon/globalconfig.cpp b2aab76 
>   phonon/mediasource.h 766e091 
>   phonon/mediasource.cpp 1432d8d 
>   phonon/mediasource_p.h a5be6d8 
>   phonon/objectdescription.h f2df1ca 
>   phonon/tests/CMakeLists.txt bad7c08 
>   phonon/tests/captureapptest/CMakeLists.txt 821ab5f 
>   phonon/tests/captureapptest/capture_test.h bd7e42d 
>   phonon/tests/captureapptest/capture_test.cpp 1b882a6 
>   phonon/tests/captureapptest/capture_test_main.cpp 1e9b121 
> 
> Diff: http://git.reviewboard.kde.org/r/100653/diff
> 
> 
> Testing
> -------
> 
> Compilation and capture demos work both for capture api disabled and enabled.
> 
> 
> Thanks,
> 
> Casian
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20110216/ec2837bc/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia


More information about the kde-multimedia mailing list