Feat/localized images #677
Conversation
There was a problem hiding this comment.
I am thinking that prefer_orginial_images could be a nice user level feature for v5 (#597). We might need to change some schema, but since v5 is still pretty early, this is doable.
Having multiples images parsed could also allow us to:
- randomly rotate images (some users might not like that tho idk)
- allow custom selection of images quickly in the front
I'll sit on this a bit and get back during the weekend whether we should merge this now or do something different for v5.
For iso normalization before sending to tmdb this is a bug that we should merge asap. You can either make a separate PR for this we could merge sooner or wait until we decide how we want images handled.
This is nice, but the preference for original images is something quite niche. If I were you, I would give this as a very low priority to become an user feature to avoid increasing complexity. I think the feature described in #87 is already very good and is enough for most people. |
|
I can see 2 ways of fixing #677 (comment) these empty posters problem.
|
| # Corner case: If there are no images at all, call TMDB images API. | ||
| # Although doing another API call is not ideal, this would only be called very rarely. | ||
| if not localized_images: | ||
| logger.debug( | ||
| "[Fallback] No images found for %s %s. Calling TMDB images API.", | ||
| item["media_type"], | ||
| item["id"], | ||
| ) | ||
| images = await self.get( | ||
| f"{item['media_type']}/{item['id']}/images", | ||
| ) | ||
| localized_images = sorted( | ||
| images[key], | ||
| key=lambda x: x.get("vote_average", 0), | ||
| reverse=True, | ||
| ) |
There was a problem hiding this comment.
I was able to get 100% of my library items with posters by adding this corner case (of 400, 1 item was a corner case).
I know doing a extra request is bad, and this is also increases the code complexity of this method because it needs to be async. If this is too much, I can remove.
There was a problem hiding this comment.
I prefer keeping this if it means getting more images.
Why is this call needed tho? It's for cases where a no localized, no original language's language & no no-language image exist? Basically, the only image that exist is localized to a random language?
There was a problem hiding this comment.
For those cases, is the poster_path of the base item empty or does it contain a vaild image?
There was a problem hiding this comment.
Using poster_path seems to be enough, I`ll remove the extra call
There was a problem hiding this comment.
For v5, I already planned the database to contain multiples locales (and image locales). I'll need to find a good way to query those to support prefer_original and good fallback.
I left some comments, but I'm pretty happy with the direction/state of the PR. Please request a new review when you feel this is ready, and I'll test & merge.
Thanks for your work on this!
| @@ -138,26 +140,88 @@ def to_studio(self, company: dict[str, Any]) -> Studio: | |||
| }, | |||
| ) | |||
|
|
|||
| async def get_best_image(self, item, lng, key): | |||
There was a problem hiding this comment.
add types hints instead of using doc comments here for consistency
|
|
||
| async def identify_movie(self, movie_id: str) -> Movie: | ||
| async def identify_movie( | ||
| self, movie_id: str, original_language: Optional[str] = "null" |
There was a problem hiding this comment.
You should set None as the default original_language. You might want to use an Optional[Language] instead of str also
| # Corner case: If there are no images at all, call TMDB images API. | ||
| # Although doing another API call is not ideal, this would only be called very rarely. | ||
| if not localized_images: | ||
| logger.debug( | ||
| "[Fallback] No images found for %s %s. Calling TMDB images API.", | ||
| item["media_type"], | ||
| item["id"], | ||
| ) | ||
| images = await self.get( | ||
| f"{item['media_type']}/{item['id']}/images", | ||
| ) | ||
| localized_images = sorted( | ||
| images[key], | ||
| key=lambda x: x.get("vote_average", 0), | ||
| reverse=True, | ||
| ) |
There was a problem hiding this comment.
I prefer keeping this if it means getting more images.
Why is this call needed tho? It's for cases where a no localized, no original language's language & no no-language image exist? Basically, the only image that exist is localized to a random language?
.env.example
Outdated
| # If this is true, kyoo will prefer to download the media in the original language of the item. | ||
| MEDIA_PREFER_ORIGINAL_LANGUAGE=false | ||
| # A pattern (regex) to ignore files. | ||
| LIBRARY_IGNORE_PATTERN=".*/[dD]ownloads?/.*|.*\.(mp3|srt|jpg|jpeg|png|gif|bmp|tiff|svg)$|.*[Tt][Rr][Aa][Ii][Ll][Ee][Rr].*" |
There was a problem hiding this comment.
you do not need to ignore all those file extensions, we only check for video files (it uses's guessit's mimetype to filter video/)
There was a problem hiding this comment.
In fact, they are common extensions of extra files that are usually in the same directory as the movie, as I come from jellyfin there are a lot of trickplay files that end up making the scan super slow because guessit is invoked unnecessarily for each thumbnail. I will remove this here, maybe add this as a "tip" in the documentation in another MR.
include_image_languagequerystring on tmdb request to include more image options and prevent empty posters for less known/rare media or for when{ISO-639-2}-{ISO-3166-1-alpha-2}langcodes are used on setup (eg. pt-BR).