Re: [vdagent-win PATCH v12 3/6] Write code to decode PNG format

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

 




On 25 Jul 2017, at 20:01, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:


On Mon, Jul 24, 2017 at 01:39:35PM +0100, Frediano Ziglio wrote:
+size_t PngCoder::convert_to_dib(uint8_t *out_buf, const uint8_t *data,
size_t size)
+{
[...]
+    const unsigned int width = png_get_image_width(png, info);
+    const unsigned int height = png_get_image_height(png, info);
+    const size_t stride = compute_dib_stride(width, out_bits);
+    const size_t image_size = stride * height;
+    const int palette_colors = [=]() {

If you want to use a function, then give it a name, if you don't want to
use a function, then this can just go in convert_to_dib() body rather
than through a lambda (I know the latter means removing the 'const’).

Removing the ‘const’? Why?


Lambdas are functions.

I believe Christophe is objecting to using a lambda whereas all other
computations use named functions. But frankly, here, I would have
written:

    const int palette_colors =
          out_bits > 8  ? 0
        : bits == 2     ? 4
        : 1 << out_bits;
This was exactly the previous version, just my version has some additional comments
to make more readable.
Not that I don’t like lambdas, but here, it’s a lot of additional syntactic noise.

Out of curiosity, I submitted both variants of the code to g++, and got
exactly the same generated code.

I personally think the purpose of that code was clear even without
a name.
Using a "standard" function requires to declare parameters and pass
them manually, something that lambda capture can do automatically.


Christophe


By the way, got new version with more old style code.

Frediano

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]