Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image not always resizing on cast #75

Open
nickrout opened this issue Oct 17, 2021 · 36 comments
Open

image not always resizing on cast #75

nickrout opened this issue Oct 17, 2021 · 36 comments
Assignees
Labels
enhancement New feature or request

Comments

@nickrout
Copy link

nickrout commented Oct 17, 2021

Version of the custom_component

0.63

Configuration

Nothing in yaml yet I am just testing image casting via thgis service call

service: openhasp.push_image
data:
  image: http://hass.lan:8123{{state_attr('media_player.kitchen', 'entity_picture')}}
  obj: p3b1
  width: 320
  height: 320
target:
  entity_id: openhasp.plate50

## Describe the bug
I am casting from my media player entity (it is a logitech media server using the HA squeezebox client). I have set up a 320x320 image on my wt32-sc01 device as a test.

Usually the pic is scaled to 320x320, but sometimes it is not, and appears quite a bit smaller.


## Debug log

<!-- To enable debug logs check this https://www.home-assistant.io/components/logger/ -->

```text
[00:23:20.420][104036/106664  2][36544/36592  1] MQTT RCV: hasp/plate50/command/p3b1.src = http://192.168.20.254:8123/api/openhasp/serve/bb7520125e85dc25c6c33aff637b79c9




@nickrout
Copy link
Author

nickrout commented Oct 17, 2021

Adding a bit of debug log after figuring out how

2021-10-17 17:48:11 DEBUG (SyncWorker_11) [custom_components.openhasp.image] image_to_rgb565 out_image: /tmp/tmprqj8cuy4
2021-10-17 17:48:11 DEBUG (MainThread) [custom_components.openhasp] Push hasp/plate50/command/p3b1.src with http://192.168.20.254:8123/api/openhasp/serve/573c838f57c7ad4548f2ea662a815962
2021-10-17 17:48:11 DEBUG (MainThread) [custom_components.openhasp.image] Get Image 573c838f57c7ad4548f2ea662a815962 form /tmp/tmprqj8cuy4

That was the right dimensions, this one was not

2021-10-17 17:55:53 DEBUG (SyncWorker_10) [custom_components.openhasp.image] image_to_rgb565 out_image: /tmp/tmpjdzlfwid
2021-10-17 17:55:53 DEBUG (MainThread) [custom_components.openhasp] Push hasp/plate50/command/p3b1.src with http://192.168.20.254:8123/api/openhasp/serve/d5b1a9ec19d5dc2f03d3d2a82d01c246
2021-10-17 17:55:53 DEBUG (MainThread) [custom_components.openhasp.image] Get Image d5b1a9ec19d5dc2f03d3d2a82d01c246 form /tmp/tmpjdzlfwid

@fvanroie
Copy link
Collaborator

It seems images are only scaled down and not scaled up. So what is the resolution of the input image?

@nickrout
Copy link
Author

nickrout commented Oct 17, 2021

The images are collected from all over the place as my music collection grows, but that will probably explain it. Thanks. I did look at the source, but didn't quite understand it.

@htvekov
Copy link

htvekov commented Dec 10, 2021

I'm also in the process of 'rewriting' my entire Sonos media player page to include images. Actually took me quite some time before I realized that upscaling was not in the scope for push image 😆 Could be really nice if this could be implemented though, as online images for TuneIn Radiostations in the Sonos app. are only some 144x144 (appx.). My fix for now is to upscale these static appx. 8-10 images and store locally in flash.

@htvekov
Copy link

htvekov commented Feb 13, 2022

Is is possible from within the HA side (config/automations) to tell an image actual size either before or after conversion ?
Are the bcWidth / bcHeight words stored so they could become accesible ?

I'm having issues identifying all TuneIn's images, as some, but not all, of their URL's include an identifier.
And what make's the job even harder is that the full pre-init URL presented from HA's Sonos integration is only present for a few seconds. So it's pretty difficult to pinpoint when exactly to zoom a dynamic TuneIn image to fit the 200 x 200 res. object size I've chosen.

The no. 1 solution would of course be that push_image could do upscaling as well 😉🙋‍♂️

@fvanroie
Copy link
Collaborator

The no. 1 solution would of course be that push_image could do upscaling as well 😉🙋‍♂️

I think commenting these lines can achieve that:

    width = min(w for w in [width, original_width] if w is not None and w > 0)
    height = min(h for h in [height, original_height] if h is not None and h > 0)

@fvanroie
Copy link
Collaborator

@dgomes Can we allow up-scaling as well or is there a particular reason this is now allowed?

@fvanroie fvanroie added the enhancement New feature or request label Jan 27, 2023
@dgomes
Copy link
Collaborator

dgomes commented Jan 28, 2023

only reason is that upscaling usually makes the image loose much quality

@fvanroie
Copy link
Collaborator

I think that is the expected result when asking for a larger size of a small image...
Otherwise there could be a flag to allow up-scaling, but I don't see a compelling reason not to upscale it by default.

@htvekov
Copy link

htvekov commented Jan 29, 2023

I think that is the expected result when asking for a larger size of a small image... Otherwise there could be a flag to allow up-scaling, but I don't see a compelling reason not to upscale it by default.

I agree. Even if user is not knowing the source image dimensions (quite common when doing something dynamic with media meta data), the desired end result is to have a fixed size image - No matter the quality.

Currently I'm using templates in order to decide whether or not to zoom an image or not in my media player config.
Calculating the zoom size and image position manually to get image placed appx. at the same position as an unzoomed image within the parent object.

A tedious process, not dynamic at all and hardcoded into the templates. And I can only template 'known' repeated source images. Unknown source images from other family members Spotify accounts can result in the strangest images on the display.

@fvanroie
Copy link
Collaborator

Currently I'm using templates in order to decide whether or not to zoom an image or not in my media player config.

That works around the problem, but zoom adds the burden of scaling the image onto the ESP32. I think it is better to save these resources and let a PC do the scaling instead.

@htvekov
Copy link

htvekov commented Jan 29, 2023

Currently I'm using templates in order to decide whether or not to zoom an image or not in my media player config.

That works around the problem, but zoom adds the burden of scaling the image onto the ESP32. I think it is better to save these resources and let a PC do the scaling instead.

Yep. Using zoom on large images, is clearly straining the mcu. Like local openHASP png converting is
It's, for obvious reasons, also notisable slow (especially using high zoom values)
As I wrote it's possible to tweak images in templates to get a decent end result
But it wil never be a dynamic solution, which upscaling on the PC side is.

It would be a great overall enhancement, if CC could upscale images as well.
I for sure would love to see this 🙂

@htvekov
Copy link

htvekov commented Jan 29, 2023

Also much more needed now, as capable openHASP device screen resolution have increased quite a lot last six months.
'Standard' device sizes are now typical above 320x480, which will only increase users need to upscale various images.

@fvanroie
Copy link
Collaborator

fvanroie commented Jan 29, 2023

    width = min(w for w in [width, original_width] if w is not None and w > 0)
    height = min(h for h in [height, original_height] if h is not None and h > 0)

Can you try commenting these two lines?

My Python skills aren't advanced, but I think those two lines are limiting the upscaling.
I don't think there is a downside to commenting those... but @dgomes can confirm.

@htvekov
Copy link

htvekov commented Jan 29, 2023

I actually believe i tried that back then, but not really sure.
I'll give it a spin 🙂

@htvekov
Copy link

htvekov commented Jan 29, 2023

Nope, no upscaling I'm afraid with these two lines commented out

service: openhasp.push_image
data:
  image: http://192.xxx.x.xx:8123/local/transmission-tower.png
  obj: p2b40
  width: 320
  height: 320
target:
  entity_id: openhasp.wt32_01_plus

image

@htvekov
Copy link

htvekov commented Jan 29, 2023

Commenting out these two lines only has effect on images that aren't square:

image

image

@fvanroie
Copy link
Collaborator

Also change the next line to:

    im.resize((height, width), Image.ANTIALIAS)

thumbnail will only allow images to downscale, never upscale. So you need to use resize instead.
Downside to resize is that the aspect ratio is not maintained, so we need to add that in if required.

@htvekov
Copy link

htvekov commented Jan 29, 2023

Tested that one as well - still a no go.
And now downsizing the image also fails. It'll just keep it's original size no matter what width and height parameters are set to

@dgomes
Copy link
Collaborator

dgomes commented Jan 29, 2023

```python
    width = min(w for w in [width, original_width] if w is not None and w > 0)
    height = min(h for h in [height, original_height] if h is not None and h > 0)

replace min with max (but be careful not to send too large images)

@htvekov
Copy link

htvekov commented Jan 29, 2023

@fvanroie actually threw me this via Discord - and it's working 👍

I guess im = im.resize((height, width), Image.ANTIALIAS) could be it

Resizing with aspect ratios should still be dealt with though.
I've no idea what the best solution is. I'll leave that part up to you guys 😉

@htvekov
Copy link

htvekov commented Jan 29, 2023

```python
    width = min(w for w in [width, original_width] if w is not None and w > 0)
    height = min(h for h in [height, original_height] if h is not None and h > 0)

replace min with max (but be careful not to send too large images)

Just tested and using this neither up- nor downscaling is working.

original_width, original_height = im.size
    width, height = size

    #width = min(w for w in [width, original_width] if w is not None and w > 0)  # Original line
    #height = min(h for h in [height, original_height] if h is not None and h > 0)  # Original line

    width = max(w for w in [width, original_width] if w is not None and w > 0)
    height = max(h for h in [height, original_height] if h is not None and h > 0)



    #im.thumbnail((height, width), Image.ANTIALIAS) # original line
    im.resize((height, width), Image.ANTIALIAS) # not working
    # im = im.resize((height, width), Image.ANTIALIAS) ## @fvanroie ok
    width, height = im.size  # actual size after resize

@dgomes
Copy link
Collaborator

dgomes commented Jan 29, 2023

I updated the development branch with dc2d98b

This adds a new a new optional argument "fit_screen" by which the image is resized to the "width" and "height" parameters regardless of the original file.

@htvekov
Copy link

htvekov commented Jan 29, 2023

Thank you @dgomes 🙂
Fetched 8d7acda and tested with below from HA dev:

service: openhasp.push_image
data:
  image: http://192.xxx.x.xx:8123/local/Berit.png
  obj: p2b40
  fitscreen: 1
  width: 320
  height: 320
target:
  entity_id: openhasp.wt32_01_plus

No upscaling done, only downscaling.
As @fvanroie mentioned that thumbnail function didn't support upscaling I replaced

im.thumbnail((height, width), Image.ANTIALIAS) with
im = im.resize((height, width), Image.ANTIALIAS)

Then it's working. Both supporting up- and downscaling.

@fvanroie
Copy link
Collaborator

nice!

@htvekov
Copy link

htvekov commented Jan 29, 2023

For some odd reason im.resize((height, width), Image.ANTIALIAS) is not working at all.
But hey, I'm not a code guy 😉

@htvekov

This comment was marked as off-topic.

@dgomes
Copy link
Collaborator

dgomes commented Jan 29, 2023

That's a completely different issue, and none of the proposed configs can work (target is the domain of HA only, CC are restricted to data)

@htvekov
Copy link

htvekov commented Jan 29, 2023

Yep, sorry about that. Don't mix up issues 🤦‍♂️🙄
I've revised and added above to CC issue #84 where it actually belongs.

@htvekov
Copy link

htvekov commented Jan 30, 2023

Confirmed new fitscreen argument works as expected with commit b2ed186, thank you 🙂

Unless im.resize function needs to be further improved with additional checks to ensure image aspect ratio integrity, I guess issue could be closed.

I'm not really sure if that check is actually wanted ?
If implemented, I guess an additional argument should be used for that.

@dgomes
Copy link
Collaborator

dgomes commented Jan 30, 2023

Thumbnail assures aspect ratio, resize does not...

The new option needs to be documented and eventually get a new name 😓

@htvekov
Copy link

htvekov commented Jan 30, 2023

I don't know if a starting point could be just check the aspect ratio (AR = W / H) and if that is higher or equal to 1 then calculate height in reference to the width change ratio, If AR is less than one, then calculate width from height change ratio instead.

Example - height calc from width change ratio:
Original image size (h x w): 145 x 345
Resize to (h1 x w1): 480 x 480
If both h < h1 and w<w1 then calculate
Aspect ratio change: 480 / 345 = 1,3913
w1 = 480
h1 = 145 x 1,3913 = 202

I don't know if it's really that simple or there are other 'traps' ?

@dgomes
Copy link
Collaborator

dgomes commented Jan 30, 2023

can be done but the point was to leave it up to the user to decide :)

BTW: The legacy option (without fitscreen) assures correct aspect ratio

@htvekov
Copy link

htvekov commented Jan 30, 2023

can be done but the point was to leave it up to the user to decide :)

BTW: The legacy option (without fitscreen) assures correct aspect ratio

Yep. I'm quite happy with current plain upscaling functionality. That saves me some tedious zoom templates 🙂👍
And legacy option maintaining thumbnail function ensures 'old' previous behaviour.

@TNTLarsn
Copy link
Contributor

TNTLarsn commented Aug 4, 2024

I don't know if a starting point could be just check the aspect ratio (AR = W / H) and if that is higher or equal to 1 then calculate height in reference to the width change ratio, If AR is less than one, then calculate width from height change ratio instead.

Example - height calc from width change ratio: Original image size (h x w): 145 x 345 Resize to (h1 x w1): 480 x 480 If both h < h1 and w<w1 then calculate Aspect ratio change: 480 / 345 = 1,3913 w1 = 480 h1 = 145 x 1,3913 = 202

I don't know if it's really that simple or there are other 'traps' ?

According to: https://stackoverflow.com/questions/273946/how-do-i-resize-an-image-using-pil-and-maintain-its-aspect-ratio
I came up with something like this:

    if not fitscreen:
        width = min(w for w in [width, original_width] if w is not None and w > 0)
        height = min(h for h in [height, original_height] if h is not None and h > 0)
        im.thumbnail((width, height), Image.LANCZOS)
    else:
        wpercent = (width / float(original_width))
        hsize = int((float(original_height) * float(wpercent)))
        im = im.resize((width, hsize), Image.Resampling.LANCZOS)

then only variable w is needed, when using fit screen.
The image it self can maintain its aspect ratio, but it is not said that all available height will be used. (Fine for me)
Still need to test images with height>width...

@xNUTx
Copy link
Contributor

xNUTx commented Aug 19, 2024

With the recent update to the image resizing to fitscreen=1 to fix a problem I was facing (the source image changes size often and the same image is converted for multiple displays), I implemented the ImageOps from PIL (which was used for Image anyway) which does a aspect ratio preserving resize without using thumbnail and does size up if required to get to the size you set it to.

This change only kicks in when fitscreen=1, if that is not set at all or to 0 it will still use the thumbnail function.

Maybe we need to implement ImageOps in the other use case too...

@fvanroie and @dgomes your opinions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants