Extension LLava with in memory images

[zsogitbe]

No description provided.

[SignalRT]

@zsogitbe, thank you very much for your work. I would like to share my thoughts:

• The interface is defined in Llama.Abstraction.ILLamaExecutor. I think that if you change the interface should be done in ILLamaExecutor.
• I would prefer not to have a duplicate property with the image in another format. At this point I think that would be better to break the interface to do it only once. My preference would be something like:

    class ImageData
    {
        enum dataType { imagePath, imageBytes, imageURL }
        public dataType DataType { get; set; }
        public object Data { get; set; }
    }

And to change the property ImagePaths for something like:

public List<ImageData > ImageData { get; set; }

Anyway that's only my opinion.

[zsogitbe]

SignalRT, it is a good idea. I did not do it because it means much more code (I have updated the PR, you can see the changes needed). I usually prefer simplicity, but in this case the standardization worth it.

[SignalRT]

Thank you @zsogitbe, I will review the changes as soon as possible.

[martindevans]

Don't use Result here (or as a rule, anywhere). use await instead.

[martindevans]

Can all of this logic be moved out of the executor? Maybe a base interface with separate classes for the different variations.

For example:

e.g.

interface IClipImage { Task<SafeLlavaImageEmbedHandle> GetEmbed(); }

class ImageDataFromUrl(string url)
{
    public async Task<SafeLlavaImageEmbedHandle> GetEmbed()
    {
        return SafeLlavaImageEmbedHandle.CreateFromMemory(await GetImageBytes());
    }

    private async Task<byte[]> GetImageBytes()
    {
        return await DownloadThatUrl(url);
    }
}

[zsogitbe]

I was thinking the same after adding more and more to the code. The reading/downloading of the image should be the task of the user of the library.

[zsogitbe]

This is ready for merging.

[martindevans]

Looks good to me. I'll leave the final review to @SignalRT since he knows more about llava than me.

[SignalRT]

I would have clearly preferred to keep the option to allow paths to files and images in memory, but it is blocking another PR with some change in key management, so I think that can be merge as is.

[zsogitbe]

SignalRT, , Please think about this once more. I think that the most optimal way to support images in the library is to have in memory byte array as the core. The users can easily convert any image from anywhere (HD, internet, DB, ...) to this byte array. The in memory image (byte array) works with all possible image locations! This is the reason for having this is the best standardized way of working.

[SignalRT]

@zsogitbe, The llava API support both approach and I think there is a reason to support this. For now I didn´t change the approach. Just include the capability to include new images in the conversation and a reset strategy in the example.

#664