You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
JpegEncoder producing incorrect output on linux-arm (since Beta006) #871
I have verified that I am running the latest version of ImageSharp
I have verified if the problem exist in both DEBUG and RELEASE mode
I have searched open and closed issues to ensure it has not already been reported
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
changed the title [-]ResizeProcessor not working since Beta006[/-][+]JpegEncoder producing incorrect output on linux-arm (since Beta006)[/+]on Apr 2, 2019
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 ?
@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?
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]
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
publicRgb24Rgb{// 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>(refthis);[MethodImpl(InliningOptions.ShortMethod)]
set =>Unsafe.As<Rgba32,Rgb24>(refthis)= 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
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;
}
}
@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.
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.
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. 👍
Activity
[-]ResizeProcessor not working since Beta006[/-][+]JpegEncoder producing incorrect output on linux-arm (since Beta006)[/+]antonfirsov commentedon Apr 2, 2019
info from gitter discussions:
resizing is not necessary to reproduce the issue, PNG -> Jpeg also fails, which indicates an issue with JpegEncoder
qdraw commentedon Apr 13, 2019
I have tried some things based on the release notes:
JimBobSquarePants commentedon Apr 13, 2019
#804 is all decoder work which we know is fine so the change must have taken place between #784 and there.
antonfirsov commentedon Apr 13, 2019
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 commentedon Apr 14, 2019
I’m smelling a jit issue here...
qdraw commentedon Apr 14, 2019
The state after #801 is a pink image
antonfirsov commentedon Apr 14, 2019
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 commentedon Apr 14, 2019
I have #784 checked again, and I was wrong this one is also pink. I have updated the comment above
antonfirsov commentedon Apr 14, 2019
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 commentedon Apr 14, 2019
@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 commentedon Apr 14, 2019
How should I do to test this?
antonfirsov commentedon Apr 14, 2019
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).
4 remaining items
qdraw commentedon Apr 14, 2019
@antonfirsov Ah I see f52be32#diff-e8ae3d9cc22f149b899610bb6fe4a5cb, lets get back to the latest master. This created the same pink image
qdraw commentedon Apr 14, 2019
now our focus is on #744
commit:
0dedf86b0b403c2da244b40993ae749f9d36b297is [PINK]commit:
d4be172dcc44f19136e286cce5b855985a58ffba[PINK]commit:
45c5e87fa4f3314cdd6583bb62105fc393907d7f[PINK]commit:
c0aa91d232862852f60d0dfe47ab8a3a5a1a5218- build failingcommit:
ee1ad0c01e75756a156348ed3d099422fd319e59is [PINK]commit:
6289a87e6d0ad5e4d1fda4385ea7fc722d1d56cb[PINK]commit:
6f5ebbbd6f233486ed6fb600121aba5319ccf7c5is [OK]commit:
d65cf676fafccd1285ba42bb05fba4750976e3d0is [PINK]commit:
0abb99b884700bd892f7a56a7ae722b58e2e52afis [OK]commit:
c23dbcd33088bf3054eee8dc1869d42f1a79aae5is [PINK]commit
891a1c57f855c4630b7724735b71d2819704e922is [OK]commit:
b14c35ffafe2f762f0ee19cbca34982d638ed039is [PINK]qdraw commentedon Apr 14, 2019
The combination when loading Rgb24 images with has been changed in:
b14c35ffafe2f762f0ee19cbca34982d638ed039I have tested this on the latest master build (2019-04-14)The client
using (var image = Image.Load<Rgb24>(inputStream))The change reverted
However this is not working on with the default client parameter
using (var inputStream = ReadStream(inputPath))-> Pink imageand the comments are describing issues with .NET Framework and x86-Windows
brianpopow commentedon May 24, 2019
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:
This will execute the ImageSharp.DockerSandbox project inside docker with a arm32 runtime image.
The output images will be in
docker_outputfolder.Changing the
Rgb24property in Rgba32.cs seems to fix this issue:Fix for Issue SixLabors#871 and SixLabors#914
brianpopow commentedon May 24, 2019
@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 commentedon May 24, 2019
I wonder if this can also be reproduced in a super small sample so we can report this as a bug upstream.
Fix for Issue #871 and #914 (#915)
JimBobSquarePants commentedon May 25, 2019
Would be wise to do it, raising an issue at least would make them aware.
dontspeak commentedon May 25, 2019
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 commentedon May 25, 2019
It should be absolutely fixed.
It's not the
Rgb24constructor that is the issue at all but rather how the jitter on Linux ARM handles the use ofUnsafe.Ason 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 callRgba32toRgb24in that manner.If you could download the latest build from MyGet and test that we'd really appreciate it. 👍
dontspeak commentedon May 29, 2019
MyGet Package works perfect now on my Raspberry. Thanks!
JimBobSquarePants commentedon May 29, 2019
Hooray!
WIP: #12 Synch with master and improve performance of decoders (#986)