Skip to content

Conversation

@will3942
Copy link
Contributor

iOS does not support loading an image from HTTP, this ensures the file both exists (to avoid a panic) and is a file:// URL

@Sinono3
Copy link
Owner

Sinono3 commented Jun 23, 2025

Hm. Because we are not 100% sure what behavior the OS implements, I'm not comfortable with adding this std::fs check as it is currently. For example, it was recently found out that Base64-encoded images work on MacOS. This PR removes that functionality by assuming the URL must start with file://. May I get write access to the branch? I'd like to add a simple if around what you implemented, to check if the URL starts with file:// or not before making the std::fs call. I think this is the best way to reconcile safety and functionality.

@will3942
Copy link
Contributor Author

@Sinono3 The reason why I enforced the check is that otherwise a panic occurs on iOS if you pass in a HTTP URL which is not a great experience. If we can load via Base64-encoding then we should definitely allow that path, perhaps an additional check to ensure HTTP URLs cannot be passed could be beneficial here too?

@Sinono3
Copy link
Owner

Sinono3 commented Jun 23, 2025

Is it possible for you to check if base64-encoded images work in iOS? If so, then we can do what you suggested: on iOS, check whether the provided URL is HTTP, and if so, return a null image. (We could print a warning here as well.)

This issues convinces me that the entire approach to platform differences are handled in souvlaki should be changed. The current approach is prone to mistakes.

@Sinono3
Copy link
Owner

Sinono3 commented Jun 28, 2025

@will3942 Sorry, I missed the fact that the actual iOS implementation does use base64, my bad. To make myself clear, I meant base64-encoded data URL images, not base64-encoded images, which the implementation already uses.

I'm currently rewriting a lot of the macOS implementation, and I have a few questions for you regarding the iOS functions:

  1. In mp_artwork, why use MPMediaItemArtwork.init(image: UIImage)? It's deprecated. The original MPMediaItemArtwork.init(boundsSize: CGSize, requestHandler: (CGSize) -> UIImage) should (theoretically) work fine on iOS. Has this been different in your experience?
  2. In load_image_from_url, why not directly use a pointer to load the information instead of using base64? Maybe doing it that way may reduce copies and memory usage. I know that that the base64 method is safer though. I could try to implement the pointer method but sadly I have no way to verify it's functioning. (I have no iOS device.) What about UIImage.init(contentsOfFile:)
    , isn't this simpler?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants