Imagesharp: JpegEncoder producing incorrect output on linux-arm (since Beta006)

[qdraw]

Prerequisites

• [x] I have written a descriptive issue title
• [x] I have verified that I am running the latest version of ImageSharp
• [x] I have verified if the problem exist in both DEBUG and RELEASE mode
• [x] I have searched open and closed issues to ensure it has not already been reported
• [x] I have discussed this on gitter

Description

I have some Image encoding issues on linux-arm. This code bellow works with beta-0005, but not beta-0006 or newer. I have also tested it with the latest nightly build 1.0.0-dev002459 The issues are only visible on linux-arm, on other platforms it is working well

UPDATE (22/05/2019, @antonfirsov)

Resizing is not relevant here, it's the issue of the jpeg encoder.

TL;DR:

• load jpeg -> resize -> save jpeg [PINK]
• load jpeg -> save jpeg [PINK]
• load png -> save jpeg [PINK]
• load gif -> save jpeg [PINK]
• load jpeg -> resize -> save png [OK]
• load jpeg -> save png [OK]

The images are looking like this, I have tried it with multiple jpeg images

Source of the image:
https://raw.githubusercontent.com/SixLabors/ImageSharp/master/tests/Images/Input/Jpg/baseline/Lake.jpg

Steps to Reproduce

I'm initially used it for resizing but it happens also when you write the image:
I'm using the following code:

using (var inputStream = _iStorage.ReadStream(subPath))
using (var image = Image.Load(inputStream))
{
    image.Save(outputStream, new JpegEncoder{
        Quality = 100,
    });

    _iStorage.WriteStream(outputStream, outputHash);  
}

public Stream ReadStream(string path, int maxRead = -1)
{
    if ( ! ExistFile(path) ) throw new FileNotFoundException(path);

    FileStream fileStream;
    fileStream = new FileStream(path, FileMode.Open, FileAccess.Read,FileShare.Read, maxRead, false);
    return fileStream;
}

public bool WriteStream(Stream stream, string path)
{
    if ( !stream.CanRead ) return false;

    stream.Seek(0, SeekOrigin.Begin);

    using (var fileStream = new FileStream(path, FileMode.Create, FileAccess.Write))
    {
        stream.CopyTo(fileStream);
    }
    stream.Dispose();
    return true;
}

System Configuration

When running on linux-arm as release and debug

• ImageSharp version: 0006 and newer
• Other ImageSharp packages and versions:
• Environment (Operating system, version and so on): linux-arm
• .NET Framework version: build as: 2.2.104, runtime: 2.1.8
• Additional information: running on a Raspberry Pi 2 or 3

--
Edit: tested based on the feedback of @JimBobSquarePants:, this produces a 'normal' image

image.Save(outputStream, new PngEncoder{
    ColorType = PngColorType.Rgb, 
   CompressionLevel = 9, 
});

[antonfirsov]

info from gitter discussions:
resizing is not necessary to reproduce the issue, PNG -> Jpeg also fails, which indicates an issue with JpegEncoder

[qdraw]

I have tried some things based on the release notes:

[x] commit: 8f3658da6c0040198c01dbf892a9ea77d0556407  PR: #804 - pink - 2019-01-09
[x] commit: f62f6e82e13310e0e625d40d9eb9653f874895d8 PR: (#722) - good (<Rgb24> + default) - 2018-10-04
[x] commit: 63ee5970220c6889177749dc14228cac035fcee9 PR (#724) - good - 2018-10-06
--[x]  commit: 537dbcf78bf83ef62c5be11fe8bd2db1c598867d PR (#784) - pink - 2018-12-30-- UPDATED
[x] commit: 2db6bd2e2a9a887dd9b7379e80d36908f5c089d7 PR: (#698) - good - 2018-09-12

added:
[x] commit: 93bb37394a484c03ba84d59d60e39f90677240c7 PR: (#804) - pink - 2019-01-09
[x] commit:  59d76a0f7b465c27330af6717df479146be3dda9 PR: (#801)  - pink - 2019-01-05
[x] commit:  575c23d929ff81b41d89ef3499125c1899ec2e72 PR: (#801)  - pink - 2019-01-04
[x] commit:  db0fc0dd7df4b23236120312419f21916b6c6d8a PR: (#874) - pink - 2018-12-31
[x] commit:  537dbcf78bf83ef62c5be11fe8bd2db1c598867d PR (#784) - pink - 2018-12-30
[x] commit:  133c85489daa305a0c5db9662e070329d8c60989 - pink - 2018-12-21
[x] commit: 8cba23f42dcd8bde55a011cd382a394fe7a69b78 - pink - 2018-12-21
[x] commit: abaf5581b6e230a14f9765433ca74f234ba01cdd - pink - 2018-12-20
[x] commit: 0f5df545be7d314f25fb6cb3d96bbf4c382ed053 - pink - 2018-12-18
[x] commit: 4568a4bfceeb3aaa242314daa11d6c2c86f0c379 - PR (#783)- pink - 2018-12-12 
[x] commit: ffba817bcae3a2895534c037798bf8e30232e06d - pink 2018-11-26
[x] commit: 43b227638931a02867c28ec8cf7fa4eb5bae8a71 - pink 2018-11-23
[x] commit: 321cca6e815ff0abb73d3a7b2168bfa9a670fdf3 #771:  - pink 2018-11-07
[x] commit: 7e25ecd4564169d1531b08a130d5517775a9deba #768 - pink 2018-11-05

[JimBobSquarePants]

804 is all decoder work which we know is fine so the change must have taken place between #784 and there.

[antonfirsov]

We are getting close! However it's very strange, that no PR between 784 and 804 touches anything related to Jpeg encoding. What is the state after merging #801 ?

[JimBobSquarePants]

I’m smelling a jit issue here...

[qdraw]

The state after #801 is a pink image

[antonfirsov]

Only 6 PR-s left to binary search ... if we did no mistake so far. Are you super sure, that #784 is OK?
If yes, let's point to the middle .. #789?

[qdraw]

I have #784 checked again, and I was wrong this one is also pink. I have updated the comment above

[antonfirsov]

Another idea:

What if you wire out the pooling memory allocator? Configuration.Default.MemoryAllocator = new SimpleGcMemoryAllocator()

Should I provide a detailed plan to track down the breaking PR? (Or are you alredy doing it? :) )

[qdraw]

@antonfirsov I'm currently checking PR's using a very simple application that only convert the image ('Lake.jpg' from the examples) to a image on device. Do you have suggestions which PR I should track?

[qdraw]

What if you wire out the pooling memory allocator? Configuration.Default.MemoryAllocator = new SimpleGcMemoryAllocator()

How should I do to test this?

[antonfirsov]

What should I do to test this?

Same thing: pink/not pink. (Just make sure you switch the allocator before running other code.)

Re tracking down the PR-s:
It should be between 724-801 now. Try pointing to the middle (binary search FTW).

[antonfirsov]

A search tree that might be useful:

            #727 
        #729 
    #731 
            #742
        #744
#746 
            #747 
        #751
    #768
        #771
            #783

PR-s that are more to the left, should be checked first. PR-s which leave JpegEncoder unaffected (in theory) are not listed. You should be able to track down the breaking PR in <=4 further checks!

[qdraw]

@antonfirsov thanks for the list, I will binary search the commits and update this comment

            #727 - ccfabf8fe7ccdb6c7cd8ca13b5fa153365f39ec2 - normal image
        #729
    #731
            #742 - 90c7153a6ebd8e4c8d0c24a0837d1f7aa7c340c2 - normal image
        #744     - 15415ef3c4bb6f34856eba961fd28c438ba58db5 - pink image 2018-10-24 <==== focus 
#746             - cd68f80e6c23825ebed252d32895e6cfd9743edb - pink image 2018-10-24 
            #747 - 47037b4fb1a578d4e4171d4cfafc7dbf46459b7b - pink image
        #751
    #768
        #771
            #783

[qdraw]

@antonfirsov
Q1: what is the value of Vector.IsHardwareAccelerated on your device?

Vector.IsHardwareAccelerated ->
False

Q2: [EDITED] what if you undefine SUPPORTS_EXTENDED_INTRINSICS in ImageSharp.csproj?
I can't find this value. How does this work?

[antonfirsov]

@qdraw open ImageSharp.csproj with a text editor (or in VS: right click -> edit csproj), and delete this PropertyGroup.

[qdraw]

@antonfirsov Ah I see https://github.com/SixLabors/ImageSharp/commit/f52be32abafa7d164d9c31f2ce4851576c667ac9#diff-e8ae3d9cc22f149b899610bb6fe4a5cb, lets get back to the latest master. This created the same pink image

[qdraw]

now our focus is on #744
commit: 0dedf86b0b403c2da244b40993ae749f9d36b297 is [PINK]
commit: d4be172dcc44f19136e286cce5b855985a58ffba [PINK]
commit: 45c5e87fa4f3314cdd6583bb62105fc393907d7f [PINK]
commit: c0aa91d232862852f60d0dfe47ab8a3a5a1a5218 - build failing
commit: ee1ad0c01e75756a156348ed3d099422fd319e59 is [PINK]
commit: 6289a87e6d0ad5e4d1fda4385ea7fc722d1d56cb [PINK]
commit: 6f5ebbbd6f233486ed6fb600121aba5319ccf7c5 is [OK]
commit: d65cf676fafccd1285ba42bb05fba4750976e3d0 is [PINK]
commit: 0abb99b884700bd892f7a56a7ae722b58e2e52af is [OK]
commit: c23dbcd33088bf3054eee8dc1869d42f1a79aae5 is [PINK]
commit 891a1c57f855c4630b7724735b71d2819704e922 is [OK]
commit: b14c35ffafe2f762f0ee19cbca34982d638ed039 is [PINK]

[qdraw]

The combination when loading Rgb24 images with has been changed in: b14c35ffafe2f762f0ee19cbca34982d638ed039 I have tested this on the latest master build (2019-04-14)

__The client__
using (var image = Image.Load<Rgb24>(inputStream))

__The change reverted__

        public Rgb24 Rgb
        {
            // If this is changed to ShortMethod then several jpeg encoding tests fail
            // on 32 bit Net 4.6.2 and NET 4.7.1
            [MethodImpl(InliningOptions.ShortMethod)]
            get => Unsafe.As<Rgba32, Rgb24>(ref this);

            [MethodImpl(InliningOptions.ShortMethod)]
            set => Unsafe.As<Rgba32, Rgb24>(ref this) = value;
        }

However this is not working on with the default client parameter
using (var inputStream = ReadStream(inputPath)) -> Pink image
and the comments are describing issues with .NET Framework and x86-Windows

[brianpopow]

As mentioned in gitter, i was able to reproduce this issue with docker. I have create a branch in my fork, so others can reproduce that too: Issue871

This adds an additional Project ImageSharp.DockerSandbox which will load and save the images from #871 and #914. To execute this with docker just use following commands in the root directory of ImageSharp:

docker-compose build
docker-compose up

This will execute the ImageSharp.DockerSandbox project inside docker with a arm32 runtime image.
The output images will be in docker_output folder.

Changing the Rgb24 property in Rgba32.cs seems to fix this issue:

public Rgb24 Rgb
{
    get => new Rgb24(this.R, this.G, this.B);

    set
    {
        this.R = value.R;
        this.G = value.G;
        this.B = value.B;
    }
}

[brianpopow]

@qdraw it seems you are able to run tests on a real device. Would it be possible for you to run my changes from pull/915 on real hardware? It would be great to verify, that this change indeed fixes the issue.

[dlemstra]

I wonder if this can also be reproduced in a super small sample so we can report this as a bug upstream.

[JimBobSquarePants]

Would be wise to do it, raising an issue at least would make them aware.

[dontspeak]

Nice that there is progress with this issue. I tried to debug into the issue on my rasperry, but as of now I did not got it managed to create a valid linux-arm compile with visual studio using the ImageSharp source code.

Is this issue completely fixed or do we only have a workaround? If the root cause really only was the RGB24 Contructur, why then we not have these issues all over ImageSharp, but only when encoding JPEGs.

Is there still testing needed? I can support.

[JimBobSquarePants]

It _should_ be absolutely fixed.

It's not the Rgb24 constructor that is the issue at all but rather how the jitter on Linux ARM handles the use of Unsafe.As on a non-inlined property when used in combination with bulk operations. It only affected the jpeg encoder because that was the only area in which we call Rgba32 to Rgb24 in that manner.

Is there still testing needed? I can support.

If you could download the latest build from MyGet and test that we'd really appreciate it. 👍

[dontspeak]

MyGet Package works perfect now on my Raspberry. Thanks!

[JimBobSquarePants]

Hooray!