Include thumbnail only option for image and render saver

[moonyuet]

Changelog Description

This PR is to include the thumbnail-only option for image and render saver creators in Ayon settings

Resolve #16

Additional review information

Test along with ynput/ayon-core#1251
Please test with/without review enabled and with/without thumbnail enabled.

Testing notes:

1. Choose whether you want to include thumbnail_only in ayon+settings://fusion/create/CreateSaver/instance_attributes or ayon+settings://fusion/create/CreateImageSaver/instance_attributes
2. Publish

[BigRoy]

I think the wording on this is weird.

Include it only? Compared to what? if set to only include it, what does it actually exclude? (I mean I know what it would exclude based on this PR, but the user seeing this attribute would have no idea).

Anyway, I also don't think this should be an artist toggle at all. I think the real problem is that disabling "Review" apparently disables the generation of Thumbnails.

I'd actually opt for that EITHER:

• It is correct that something that's not reviewable does not get a thumbnail (because how do we know that it could get one?)
• Or, always enforce generation of thumbnail when we can (regardless of whether something is review or not). This way, at the very least we'd get thumbnails as often as possible. Or is there a use case to also want to explicitly disable generation of a thumbnail?

@philippe-ynput @mkolar

Whatever we choose, I think adding an attribute like this feels wrong. If we're going to do this we may better turn Review into an Enum that instead has options like:

• No previews
• Thumbnail only
• Reviewable + thumbnail

But then again, I'd rather keep things simple. The reviewable is optional, but the thumbnail would always get generated. Seems like a fine default.

[LiborBatek]

Been succesfully tested and all working well.

I do agree with @BigRoy tho as it doesnt make much sense to call it Include thumbnail only as when Review knob above been also enabled it simply wont produce thumbnail only but both.

However it does work well for occassion when Review been disabled so producing the thumbnail only then...

LGTM with some space for improvements :)

Here are my representations with just thumbnail and not review included...

[BigRoy]

Although I see how this could work. I really wonder whether it's the correct approach to have to start adding instance attributes and tags like these all over instances.

We are now introducing both instance data need_thumbnail and per represenstation tag need_thumbnail even though IF something is supported to act as thumbnail it seems like what we really want is to just always generate a thumbnail. (with that potentially being disabled.)

That to me doesn't sound like we need to be tagging instances any way, but instead just:

• if there's no thumbnail yet then:
  • if the instance has any representation that is a valid file format to generate a thumbnail from, then generate it.
    • optionally - allow this to be disabled globally.

This to me just sounds like a core plugin that should need no changes to any other repository. The fact that this PR is needed on ayon-fusion already shows that most likely we now need to consider similarly for ALL other integrations which only generates a burden to maintain the code, test the different implementations and as a studio admin to configure as well.

This should really just be:

• Does it have a video or image? -> Add a thumbnail.

With that plug-in potentially being disabled or not.

Which to me is exactly what ExtractThumbnail in core should be doing. As such, it'd just be removing the logic that requires it to have a 'review' tag... and instead:

• Use the 'review' tagged reviewable by default
• If there's still no tagged reviewable... then generate thumbnail from the first representation that can be used as thumbnail.

Simple as that.

@mkolar thoughts?

[iLLiCiTiT]

If there's still no tagged reviewable... then generate thumbnail from the first representation that can be used as thumbnail.

Overall I do agree, but I think this might be trickier than it sounds, it should not cause crash of publishing if it fails to create one.

[antirotor]

let me add something from the representation traits perspective. Overall instance can hold different kinds of data (even non-image). We can also assume that if product has image data, we want to create thumbnail from it - that becomes a new representation - linked to the representation used to create it. In theory, you can have multiple thumbnails for multiple representations within one product and that is perfectly fine (imagine texture pack). For UI purpose, you need to select one to show/present (although concepts like a film strip, etc. would allow to show all of existing thumbnails on one item).

So having plugin that will create thumbnail representation on any representation with "image" trait (unless it already has "thumbnail" trait of course) with options to disable it for certain conditions (like image without rgb channels, etc.) and then having logic that will select or mark the primary thumbnail to be shown in the UI? Btw the primary thumbnail could be trait property on thumbnail trait.

[BigRoy]

Overall I do agree, but I think this might be trickier than it sounds, it should not cause crash of publishing if it fails to create one.

Actually, I think it's totally fine to get these 'false positives' where we think we can convert the media but actually can't. We can just do subprocess call to oiiotool or ffmpeg (depending on extension) and if it happens to fail instead of raising an error just log a warning that it's failing to generate a thumbnail.

Over time we may see more logs where failure warnings are logged... but that only would mean we'd find them and we can look into more how to get even more thumbnails everywhere.

That'd totally be feasible without it 'breaking' publishing then, right?

In 99% of the cases it not having a thumbnail shouldn't be a showstopper in production, so it allowing to 'ignore it and just log a warning' seems the best approach anyway.

---

that becomes a new representation - linked to the representation used to create it. In theory, you can have multiple thumbnails for multiple representations within one product and that is perfectly fine (imagine texture pack). For UI purpose, you need to select one to show/present (although concepts like a film strip, etc. would allow to show all of existing thumbnails on one item).

As far as I know thumbnails are not intended to be representations by themselves (or at least not something that should be usable representations for load tools, etc.) I believe a change some time ago made it so that they are not integrated as representations anymore. I suppose with traits it being a representation may start making sense once again, but as far as I know.. they currently aren't.

[moonyuet]

Should we close this ticket as ynput/ayon-core#1251 is merged?

[BigRoy]

Should we close this ticket as ynput/ayon-core#1251 is merged?

Yes please.

[moonyuet]

Close it for ynput/ayon-core#1251 implementing thumbnail creation with valid representation of image contents