[libavg-devel] v4l branch code review

[OXullo Intersecans]

Il giorno 22/lug/07, alle ore 00:43, Ulrich von Zadow ha scritto:

> Hi,

hi kodah!

> I finally found the time to look through the code in the v4l branch  
> and
> updated it to newest trunk. Overall, everything looks very good and I
> think it's ok to merge to trunk whenever you think everything is ok.

We're just managing our time to complete it, and I think that this  
evening we'll commit the latter relevant addition (deferred setFeature 
()) and code cleanups. Afterwards it could be considered done.

> I'll probably clean up the firewire modesetting stuff so everything is
> consistent once you've done the merge.

good

>
> Still, some minor points:
>
> - Some of the indentation is still broken.

We'll check for it

> - There are some (actually, many) coding convention violations. I  
> think
> I forgot to tell you about the coding conventions, so we can leave
> current code like it is (or maybe I'll fix it).

No problem, we'll put it on track

> Still, please read
> https://www.libavg.de/wiki/index.php/Coding_Conventions if you haven't
> already and stick to them for any new code.

ok

> - Bitmap.cpp, 832ff: Have you benchmarked this code to make sure it
> isn't eating up too much cpu time? It looks slow.

That code needs some optimization, indeed. However, if you think that  
profiling breakdown is quite reliable, it seems that it's not so much  
time consuming, comparing to other formats.

> - V4LCamera.cpp
>
> 206ff: Is anything other than IO_METHOD_MMAP still used?

we wished to have some time to get further testing but we hadn't..  
mmap seems to work good as expected, so it would be ok to linger with  
it.

> If not, the
> code here will never be tested anymore and will become buggy. Also,  
> less
> code is always good, so it should probably be removed. If you've got a
> good reason for keeping it there, add an appropriate comment.

we would cut other methods at cleanup. We'll do it!

> I'll probably buy a cheap USB camera myself just to see your stuff in
> action :-).

eheh
we wish that Ben Lau succeeded in getting his camera working. The  
only test we did was with BT878 PCI cards...
Ben, please.. any feedback?

byez!

--
OXullo Intersecans

0 2 L > Outside Standing Level
http://www.02L.net