-2

Elaborating on the title a little bit more, we have some code that loads an image file from disk, then converts it into a BitmapImage for display on screen. We then need to persist that data to a different storage mechanism as a byte array so it can be reloaded as-is later.

The issue is from what I understand, to convert a BitmapImage back into a byte array, you have to use an encoder, but if you use an encoder, you're re-encoding the image on every save so unless you use a lossless format, you could be downgrading the quality of your image, but to not use a lossless format means you're potentially doubling storage space over something like jpeg data.

My thought is to create a 'holder' type that holds the original, raw data (again, say from a jpg file) and use it to create a BitmapImage on-demand, caching it for future retrieval.

Here's what I have so far for the 'holder' type. (Note the implicit converter so I can use it wherever a BitmapImage is accepted):

#nullable enable

class PersistableBitmapImage{

    public PersistableBitmapImage(byte[] bytes)
        => _bytes = bytes;

    private byte[] _bytes;
    public byte[] Bytes {
        get => _bytes;
        set{
            _bytes = value;
            _bitmapImage = null;
        }
    }

    private BitmapImage? _bitmapImage;
    public BitmapImage BitmapImage
        => _bitmapImage ?? (_bitmapImage = Bytes.MakeBitmapImage());

    #region Implicit Operators

        public static implicit operator BitmapImage(PersistableBitmapImage persistableBitmapImage)
            => persistableBitmapImage.BitmapImage;

    #endregion Implicit Operators
}

Here's the helper extension to create the BitmapImage:

public static class ByteArrayExtensions {

    public static BitmapImage MakeBitmapImage(this byte[] bytes){

        using var ms = new MemoryStream(bytes);

        var bitmapImage = new BitmapImage();

        bitmapImage.BeginInit();
            bitmapImage.CacheOption = BitmapCacheOption.OnLoad;
            bitmapImage.StreamSource = ms;
        bitmapImage.EndInit();

        return bitmapImage;
    }
}

The issue I have with this design is in cases where that BitmapImage is accessed, I'm now holding the image data twice, once in compressed format (the Data property) and another in the realized/expanded BitmapImage property. The only up-side is the latter is only 'realized' if accessed and the compressed former is what's persisted.

Still feels a little 'icky' to me. Is there a way to do this without holding the image data twice?

TylerH
  • 20,799
  • 66
  • 75
  • 101
Mark A. Donohoe
  • 28,442
  • 25
  • 137
  • 286
  • I don't understand this question at all. If you want to "persist" the bitmap in its original form, why don't you just make a copy of the original data when you first load the bitmap? What is the actual _problem_ you are trying to solve here? Your question has [XY Problem](https://meta.stackexchange.com/questions/66377/what-is-the-xy-problem) written all over it. :( Stack Overflow cannot usefully answer question for which the only stated problem is "icky". – Peter Duniho Oct 30 '20 at 06:03
  • Do you need that property? Can you replace it with a method that returns the result of `MakeBitmapImage()` instead? – Jimi Oct 30 '20 at 06:11
  • What about `bitmapImage.StreamSource` property? It has getter and I'd expect it to store original memory stream you passed to it, which you can then access and get your byte array back (probably first need to reset position to 0, because BitmapImage has read it) – Evk Oct 30 '20 at 06:13
  • If you do need a property, you don't need the backing field. – Jimi Oct 30 '20 at 06:17
  • @PeterDuniho, that is exactly what my solution above does... it keeps the original image data, pre-decompress specifically for storage. – Mark A. Donohoe Oct 30 '20 at 06:39
  • @Jimi, yes you need that lazy-loading-style property or every time you access it, you will be decoding it again and again and again. Larger images don't decode instantly so you could have a performance hit there. That's the point of a lazy-loading pattern. – Mark A. Donohoe Oct 30 '20 at 06:40
  • @Evk, that's interesting! I thought that was consumed/destroyed immediately upon creation, but if it keeps that value around, then that would solve my issue, at least for the case where I create it *from* a stream first, which is what I'm doing here. – Mark A. Donohoe Oct 30 '20 at 06:43
  • You do not even dispose of the source stream - which would usually by done by a `using` block, so you can obviously access and use it. Note that it is just a MemoryStream over the original Bytes array. You could as well have a `Stream SourceStream { get; set; }` property in your wrapper class. – Clemens Oct 30 '20 at 06:57
  • @Clemens, actually, the very first line in the extension *is* a `using` statement *does* dispose/close the stream when the method goes out of scope. That's a new format for `using` that the scope is implicit by the enclosing method. It's the same as if everything under it was wrapped in brackets (i.e. a block). But... I think I just have to remove the `using` keyword and it will give me what I want. Thanks! :) – Mark A. Donohoe Oct 30 '20 at 07:00
  • Sorry, I overlooked that. Still, you could just keep the stream open. However, you already have the source buffer, so what. Makes no real difference. – Clemens Oct 30 '20 at 07:02
  • That's just it... I thought I had to keep the source buffer around, which is why I came up with the `PersistableBitmapImage` class, specifically to hang onto it. But maybe the real solution is that isn't needed and to instead just keep the stream itself around. There is one advantage to my `PersistableBitmapImage` class though, and that's it only decodes/expands the image if you actually access it, which wouldn't be the case if you say, read in a hundred images, but only displayed one or two. Something to think about. – Mark A. Donohoe Oct 30 '20 at 07:05
  • As said, turn `public byte[] Bytes` into `public Stream SourceStream` and you get rid of the source buffer (at least as a byte array), but still have lazy loading. – Clemens Oct 30 '20 at 07:07
  • But what would be the benefit of keeping the stream around if you're only interested in the raw data? The solution here only creates the stream for the duration of the image creation. In other words, if I was just creating the stream, then storing it in the BitmapImage, then I could get rid of my class, but if I want to keep the lazy-loading, I think it's more beneficial to keep the byte array as-is as that's what ultimately gets persisted. – Mark A. Donohoe Oct 30 '20 at 07:10
  • It would make no difference at all, as said. And you would actually have to make your class implement IDisposable, because it has to dispose of a property that is IDisposable. The answer to your question "Is there a better way?" is clearly no, but you already know that. – Clemens Oct 30 '20 at 07:13
  • Actually, ironically in the specific case of `MemoryStream`, it only implements `IDisposable` because its abstract base class does, but `MemoryStream` itself has nothing to actually dispose of. The documentation even specifically says you don't have to dispose of it because it's a no-op. Still, I do it out of good patterns/practice in case someone refactors something else in, seeing that interface and making an assumption it will be properly called. And I wasn't sure what I had was the best way, but these comments make me feel a lot better about what I do have, so thanks! :) – Mark A. Donohoe Oct 30 '20 at 07:34
  • @Evk, since you brought up the 'keep the stream around' solution first, mind putting that in an answer so I can accept it to close this out? – Mark A. Donohoe Oct 30 '20 at 07:35

1 Answers1

1

You can avoid disposing MemoryStream here (disposing of MemoryStream while good practice doesn't do anything interesting anyway, there are no resources to dispose):

using var ms = new MemoryStream(bytes);

And then be able to access that same memory stream later by accessing bitmapImage.StreamSource. Note that returned stream Position might be not 0, because BitmapImage might have read it already, so don't forget to reset position to 0 if necessary. Usually it's bad idea to call MemoryStream.GetBuffer() (ToArray() is preferred), but in this particular case it might be appropriate to avoid unnecessary copy.

Evk
  • 98,527
  • 8
  • 141
  • 191
  • Perfect! Thanks! There still may be a reason to keep my `PersistentBitmapImage` class around for the lazy-loading aspect, but for simpler implementations, this will work great! – Mark A. Donohoe Oct 30 '20 at 07:41