After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 722696 - gupnp_device_info_get_icon_url() returns low quality icon
gupnp_device_info_get_icon_url() returns low quality icon
Status: RESOLVED FIXED
Product: GUPnP
Classification: Other
Component: gupnp
unspecified
Other Mac OS
: Normal normal
: ---
Assigned To: GUPnP Maintainers
GUPnP Maintainers
Depends on:
Blocks: 722607
 
 
Reported: 2014-01-21 14:26 UTC by Bastien Nocera
Modified: 2014-07-18 19:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
device.xml (2.53 KB, text/xml)
2014-01-21 14:26 UTC, Bastien Nocera
  Details
Respect 'prefer_bigger' even if icon size is not requested (5.84 KB, patch)
2014-07-09 11:17 UTC, Jussi Kukkonen
none Details | Review
tests: Add test for #722696 (icon size request issue) (3.98 KB, patch)
2014-07-09 11:17 UTC, Jussi Kukkonen
committed Details | Review
Respect 'prefer_bigger' even if icon size is not requested (5.84 KB, patch)
2014-07-09 11:18 UTC, Jussi Kukkonen
committed Details | Review

Description Bastien Nocera 2014-01-21 14:26:19 UTC
Created attachment 266872 [details]
device.xml

gupnp-0.20.9-1.fc20.x86_64

I run:
gupnp_device_info_get_icon_url (GUPNP_DEVICE_INFO (device),
                                         NULL,
                                         -1, -1, -1,
                                         TRUE,
                                         NULL, NULL, NULL, NULL);

And I get:
http://192.168.0.11:50001/tmp_icon/dmsicon48.png
The last icon in the list.

Instead of the bigger 120x120 icon.

I've attached the device.xml for my Diskstation.
Comment 1 Jens Georg 2014-01-21 16:03:01 UTC
Looks like "prefer_bigger" is only used when requested_width and/or requested_height is used.

That's probably not what was intended.
Comment 2 Jussi Kukkonen 2014-01-21 16:34:02 UTC
So the expectation is: "prefer_bigger" should lead to the largest icon to be selected if no width or height is given? That sounds reasonable.

I'm pretty sure you can already workaround this by setting large values for width and/or height with prefer_bigger=TRUE.
Comment 3 Jussi Kukkonen 2014-07-09 11:17:19 UTC
Created attachment 280244 [details] [review]
Respect 'prefer_bigger' even if icon size is not requested

If gupnp_device_info_get_icon_url() was called without a specific size
request, it returned the last icon in the list.

This patch makes the function return in those circumstances either the
largest icon or the smallest icon, based on 'prefer_bigger' argument
value.
Comment 4 Jussi Kukkonen 2014-07-09 11:17:59 UTC
Created attachment 280245 [details] [review]
tests: Add test for #722696 (icon size request issue)
Comment 5 Jussi Kukkonen 2014-07-09 11:18:04 UTC
Created attachment 280246 [details] [review]
Respect 'prefer_bigger' even if icon size is not requested

If gupnp_device_info_get_icon_url() was called without a specific size
request, it returned the last icon in the list.

This patch makes the function return in those circumstances either the
largest icon or the smallest icon, based on 'prefer_bigger' argument
value.
Comment 6 Jussi Kukkonen 2014-07-09 11:20:56 UTC
Comment on attachment 280246 [details] [review]
Respect 'prefer_bigger' even if icon size is not requested

meh, this still has one problem... just a sec.
Comment 7 Jussi Kukkonen 2014-07-09 11:26:37 UTC
Comment on attachment 280246 [details] [review]
Respect 'prefer_bigger' even if icon size is not requested

Actually, that was just me reading wrong: it is correct.

When no size request is included, icon->weight is set at icon pixel count. Then selection is just a case of largest or smallest weight based on 'prefer_bigger' value.
Comment 8 Jens Georg 2014-07-11 17:22:06 UTC
Review of attachment 280245 [details] [review]:

+1
Comment 9 Jens Georg 2014-07-11 17:49:14 UTC
Review of attachment 280246 [details] [review]:

+1

::: libgupnp/gupnp-device-info.c
@@ +767,3 @@
                          * incorrect depth. */
+                        /* Note: Meaning of 'weight' changes when no
+                         * size request is included. */

Why does it change meaning?
Comment 10 Jussi Kukkonen 2014-07-14 10:45:04 UTC
(In reply to comment #9)
> Review of attachment 280246 [details] [review]:
> 
> +1
> 
> ::: libgupnp/gupnp-device-info.c
> @@ +767,3 @@
>                           * incorrect depth. */
> +                        /* Note: Meaning of 'weight' changes when no
> +                         * size request is included. */
> 
> Why does it change meaning?

When a size request is set, 'weight' is a sort of a distance from the optimal values of width, height and density (and smallest weight wins). When size request is not set, that doesn't work as the optimal value would be "infinite" in the case of prefer_bigger. So in that case weight is just number of pixels and largest weight wins (or smallest in case of !prefer_bigger).
Comment 11 Jens Georg 2014-07-18 19:19:10 UTC
well still kind of a weight, but ok.
Comment 12 Jens Georg 2014-07-18 19:21:43 UTC
Attachment 280245 [details] pushed as b6e3169 - tests: Add test for #722696 (icon size request issue)
Attachment 280246 [details] pushed as ce83d1c - Respect 'prefer_bigger' even if icon size is not requested