Smalltalk › Squeak › Squeak - Dev

A nitpick

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

18 messages Options

Yoshiki Ohshima-3

A nitpick

This can be a nitpick of the year, but there is a bug in
InflateStream>>decompressBlock:with:

There is a part that reads (toward the end):

------------------------
collection
replaceFrom: readLimit+1
to: readLimit + length + 1
with: collection
startingAt: readLimit - distance + 1.
------------------------

but here we want to replace "length" bytes in collection, as opposed
to length + 1 bytes, so it has to say:

------------------------
collection
replaceFrom: readLimit+1
to: readLimit + length
with: collection
startingAt: readLimit - distance + 1.
------------------------

(So is the non-primitive version in FastInflateStream.)

The reason it does not matter is that replaceFrom:to:with:startingAt:
does not care about the source and destination being the same and
overlap between them. Even when the above bug puts one extra bytes in
collection temporarily, the extra byte will be wiped out in the next
iteration of the loop surrounding it. However, if you try to port
this to a language that does the "right thing" when there is an
overlap, it manifests as a bug.

-- Yoshiki

P.S.
It was a "rare" delight to point out a bug in Andreas' code^^; We miss
you.

Eliot Miranda-2

Re: A nitpick

Thanks Yoshiki,

I'm committing this now

On Tue, May 6, 2014 at 2:09 PM, Yoshiki Ohshima <[hidden email]> wrote:

This can be a nitpick of the year, but there is a bug in
InflateStream>>decompressBlock:with:

There is a part that reads (toward the end):

------------------------
collection
replaceFrom: readLimit+1
to: readLimit + length + 1
with: collection
startingAt: readLimit - distance + 1.
------------------------

but here we want to replace "length" bytes in collection, as opposed
to length + 1 bytes, so it has to say:

------------------------
collection
replaceFrom: readLimit+1
to: readLimit + length
with: collection
startingAt: readLimit - distance + 1.
------------------------

(So is the non-primitive version in FastInflateStream.)

The reason it does not matter is that replaceFrom:to:with:startingAt:
does not care about the source and destination being the same and
overlap between them. Even when the above bug puts one extra bytes in
collection temporarily, the extra byte will be wiped out in the next
iteration of the loop surrounding it. However, if you try to port
this to a language that does the "right thing" when there is an
overlap, it manifests as a bug.

-- Yoshiki

P.S.
It was a "rare" delight to point out a bug in Andreas' code^^; We miss
you.

--
best,

Eliot

Yoshiki Ohshima-3

Re: A nitpick

Thank you Eliot!

The one in my previous email was not a real problem, but I noticed
that Squeak's PNGReader cannot load certain kind of valid PNG files.
For example, when colorType=0 (grayscale), bits per channel = 16, and
has tRNS chunk. An example of such is 'tbwn0g16.png' on
http://www.schaik.com/pngsuite/pngsuite_trn_png.html.

On Tue, May 6, 2014 at 3:02 PM, Eliot Miranda <[hidden email]> wrote:

> Thanks Yoshiki,
>
> I'm committing this now
>
>
> On Tue, May 6, 2014 at 2:09 PM, Yoshiki Ohshima <[hidden email]>
> wrote:
>>
>> This can be a nitpick of the year, but there is a bug in
>> InflateStream>>decompressBlock:with:
>>
>> There is a part that reads (toward the end):
>>
>> ------------------------
>> collection
>> replaceFrom: readLimit+1
>> to: readLimit + length + 1
>> with: collection
>> startingAt: readLimit - distance + 1.
>> ------------------------
>>
>> but here we want to replace "length" bytes in collection, as opposed
>> to length + 1 bytes, so it has to say:
>>
>> ------------------------
>> collection
>> replaceFrom: readLimit+1
>> to: readLimit + length
>> with: collection
>> startingAt: readLimit - distance + 1.
>> ------------------------
>>
>> (So is the non-primitive version in FastInflateStream.)
>>
>> The reason it does not matter is that replaceFrom:to:with:startingAt:
>> does not care about the source and destination being the same and
>> overlap between them. Even when the above bug puts one extra bytes in
>> collection temporarily, the extra byte will be wiped out in the next
>> iteration of the loop surrounding it. However, if you try to port
>> this to a language that does the "right thing" when there is an
>> overlap, it manifests as a bug.
>>
>> -- Yoshiki
>>
>> P.S.
>> It was a "rare" delight to point out a bug in Andreas' code^^; We miss
>> you.
>>
>
>
>
> --
> best,
> Eliot

--
-- Yoshiki

Bert Freudenberg

16 bit GIFs (was: A nitpick)

Do we support any 16 bit-per-channel gifs?

- Bert -

On 09.05.2014, at 06:15, Yoshiki Ohshima <[hidden email]> wrote:

> Thank you Eliot!
>
> The one in my previous email was not a real problem, but I noticed
> that Squeak's PNGReader cannot load certain kind of valid PNG files.
> For example, when colorType=0 (grayscale), bits per channel = 16, and
> has tRNS chunk. An example of such is 'tbwn0g16.png' on
> http://www.schaik.com/pngsuite/pngsuite_trn_png.html.
>
> On Tue, May 6, 2014 at 3:02 PM, Eliot Miranda <[hidden email]> wrote:
>> Thanks Yoshiki,
>>
>> I'm committing this now
>>
>>
>> On Tue, May 6, 2014 at 2:09 PM, Yoshiki Ohshima <[hidden email]>
>> wrote:
>>>
>>> This can be a nitpick of the year, but there is a bug in
>>> InflateStream>>decompressBlock:with:
>>>
>>> There is a part that reads (toward the end):
>>>
>>> ------------------------
>>> collection
>>> replaceFrom: readLimit+1
>>> to: readLimit + length + 1
>>> with: collection
>>> startingAt: readLimit - distance + 1.
>>> ------------------------
>>>
>>> but here we want to replace "length" bytes in collection, as opposed
>>> to length + 1 bytes, so it has to say:
>>>
>>> ------------------------
>>> collection
>>> replaceFrom: readLimit+1
>>> to: readLimit + length
>>> with: collection
>>> startingAt: readLimit - distance + 1.
>>> ------------------------
>>>
>>> (So is the non-primitive version in FastInflateStream.)
>>>
>>> The reason it does not matter is that replaceFrom:to:with:startingAt:
>>> does not care about the source and destination being the same and
>>> overlap between them. Even when the above bug puts one extra bytes in
>>> collection temporarily, the extra byte will be wiped out in the next
>>> iteration of the loop surrounding it. However, if you try to port
>>> this to a language that does the "right thing" when there is an
>>> overlap, it manifests as a bug.
>>>
>>> -- Yoshiki
>>>
>>> P.S.
>>> It was a "rare" delight to point out a bug in Andreas' code^^; We miss
>>> you.
>>>
>>
>>
>>
>> --
>> best,
>> Eliot
>
>
>
> --
> -- Yoshiki
>

smime.p7s (5K) Download Attachment

Yoshiki Ohshima-3

Re: 16 bit GIFs (was: A nitpick)

On Fri, May 9, 2014 at 1:14 AM, Bert Freudenberg <[hidden email]> wrote:
> Do we support any 16 bit-per-channel gifs?

Not sure what you mean by "gifs" (I am specifically talking about
PNGs), but sure, I think Squeak should be able to read a PNG file, as
long as it is a valid one. If it is 16 bits per channel, it should
downscale the depth to 8 and create a 32 bpp Forms. This does happen
in most of the cases, but when the PNG file specifies the transparent
pixel (it is also in 16 bit value), it causes an error.
--
-- Yoshiki

Nicolas Cellier

Re: 16 bit GIFs (was: A nitpick)

I just committed a fix.

The PNGReadWriter currenlty has a hack to support 16 bits per channel gray scale (with a slow bit poker loop), but not with transparency...
- transparency is handled with a palette
- ColorForm only supports depth <= 8

Since the Form is forced to depth 8, and since we must choose an entry in the palette for transparency, I forced the transparency to be stored at 1st palette entry (value 0) in case of 16 bitsPerChannel.

It seems to me that there was another bug even for bitsPerPixel <= 8: the palette is 1-based indexed so transparent color index was one off.

It also seems to me that the inversion 255-pixelValue is wrong in the 16 bitsPerChannel bitPoker loop , I did change it.

2014-05-09 19:36 GMT+02:00 Yoshiki Ohshima <[hidden email]>:

On Fri, May 9, 2014 at 1:14 AM, Bert Freudenberg <[hidden email]> wrote:
> Do we support any 16 bit-per-channel gifs?

Not sure what you mean by "gifs" (I am specifically talking about
PNGs), but sure, I think Squeak should be able to read a PNG file, as
long as it is a valid one. If it is 16 bits per channel, it should
downscale the depth to 8 and create a 32 bpp Forms. This does happen
in most of the cases, but when the PNG file specifies the transparent
pixel (it is also in 16 bit value), it causes an error.
--
-- Yoshiki

Nicolas Cellier

Re: 16 bit GIFs (was: A nitpick)

After further fixes i tested these forms successfully:

#(
pal_bk pal rgba8_bk rgba8 rgba16_bk rgba16
rgb8_t_bk rgb8_t rgb16_t_bk rgb16_t
gray8a_bk gray8a gray16a_bk gray16a
gray8b_bk gray8b gray16b_bk gray16b
pal_bk_notrns palb
)
do: [:e |
(ImageReadWriter formFromStream: ('http://entropymine.com/jason/testbed/pngtrans/',e,'.png') asUrl retrieveContents contentStream) asMorph openInWorld].

2014-05-10 12:36 GMT+02:00 Nicolas Cellier <[hidden email]>:

I just committed a fix.

The PNGReadWriter currenlty has a hack to support 16 bits per channel gray scale (with a slow bit poker loop), but not with transparency...
- transparency is handled with a palette
- ColorForm only supports depth <= 8
Since the Form is forced to depth 8, and since we must choose an entry in the palette for transparency, I forced the transparency to be stored at 1st palette entry (value 0) in case of 16 bitsPerChannel.

It seems to me that there was another bug even for bitsPerPixel <= 8: the palette is 1-based indexed so transparent color index was one off.

It also seems to me that the inversion 255-pixelValue is wrong in the 16 bitsPerChannel bitPoker loop , I did change it.

2014-05-09 19:36 GMT+02:00 Yoshiki Ohshima <[hidden email]>:

On Fri, May 9, 2014 at 1:14 AM, Bert Freudenberg <[hidden email]> wrote:
> Do we support any 16 bit-per-channel gifs?

Not sure what you mean by "gifs" (I am specifically talking about
PNGs), but sure, I think Squeak should be able to read a PNG file, as
long as it is a valid one. If it is 16 bits per channel, it should
downscale the depth to 8 and create a 32 bpp Forms. This does happen
in most of the cases, but when the PNG file specifies the transparent
pixel (it is also in 16 bit value), it causes an error.
--
-- Yoshiki

timrowledge

Re: 16 bit GIFs (was: A nitpick)

In reply to this post by Nicolas Cellier

There’s a related issue that could do with a fix sometime - the jpeg library we used to make the jepegreadwriteplugin is out of date and Debian (at least) considers that a security risk and thus they mangle the sources to not use those file but link to the ‘standard’ library. Which unfortunately doesn’t actually work within our plugin!

Leaving aside the sheer rudeness of throwing unapproved ‘patches’ into somebody’s code (without ever letting Ian know, for example) and resulting in a broken function, I imagine that if looked into we would be able to tweak our plugin code to actually work with the new jpeg library code. This would be a Good Thing. If anyone knows enough about jpeg library, Debian, blah blah blah to look into this then it might make a nice project that would benefit the Raspberry Pi Scratch using world - several tens of thousands of kids in the UK alone.

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
I am the computer your mother warned you about.

David T. Lewis

VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

On Sat, May 10, 2014 at 03:00:41PM -0700, tim Rowledge wrote:
> There?s a related issue that could do with a fix sometime - the jpeg library we used to make the jepegreadwriteplugin is out of date and Debian (at least) considers that a security risk and thus they mangle the sources to not use those file but link to the ?standard? library. Which unfortunately doesn?t actually work within our plugin!
>
> Leaving aside the sheer rudeness of throwing unapproved ?patches? into somebody?s code (without ever letting Ian know, for example) and resulting in a broken function, I imagine that if looked into we would be able to tweak our plugin code to actually work with the new jpeg library code. This would be a Good Thing. If anyone knows enough about jpeg library, Debian, blah blah blah to look into this then it might make a nice project that would benefit the Raspberry Pi Scratch using world - several tens of thousands of kids in the UK alone.
>

This issue is tracked in Mantis 7539: "Plugins should use platform libraries
where possible. Gentoo removed Squeak for this reason."

http://bugs.squeak.org/view.php?id=7539

It was originally reported based on Gentoo rejecting Squeak, and it affects Debian
and other Linux distributions that (quite reasonably) expect that supported
applications should link to current versions of system libraries.

If anyone is interested in working to improve this, please do. It would be a great
way for someone to contribute a really useful update to the VM.

Dave

Nicolas Cellier

Re: VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

2014-05-11 3:48 GMT+02:00 David T. Lewis <[hidden email]>:

On Sat, May 10, 2014 at 03:00:41PM -0700, tim Rowledge wrote:
> There?s a related issue that could do with a fix sometime - the jpeg library we used to make the jepegreadwriteplugin is out of date and Debian (at least) considers that a security risk and thus they mangle the sources to not use those file but link to the ?standard? library. Which unfortunately doesn?t actually work within our plugin!
>
> Leaving aside the sheer rudeness of throwing unapproved ?patches? into somebody?s code (without ever letting Ian know, for example) and resulting in a broken function, I imagine that if looked into we would be able to tweak our plugin code to actually work with the new jpeg library code. This would be a Good Thing. If anyone knows enough about jpeg library, Debian, blah blah blah to look into this then it might make a nice project that would benefit the Raspberry Pi Scratch using world - several tens of thousands of kids in the UK alone.
>

This issue is tracked in Mantis 7539: "Plugins should use platform libraries
where possible. Gentoo removed Squeak for this reason."

http://bugs.squeak.org/view.php?id=7539

It was originally reported based on Gentoo rejecting Squeak, and it affects Debian
and other Linux distributions that (quite reasonably) expect that supported
applications should link to current versions of system libraries.

If anyone is interested in working to improve this, please do. It would be a great
way for someone to contribute a really useful update to the VM.

Dave

A general idea would be to proceed in two steps:

-1) upgrade the library in squeak svn repository, so as to fix the API evolutions mismatch

-2) provide a way to link to existing library (via cmake/configure in VMMaker and apt/rpm package dependency at distrib level)

Good plan, but what are the starting points?

Which library exactly should be used?

libjpeg - http://sourceforge.net/projects/libjpeg/ - which is the successor of the library used in squeak is licenced GPL v2, should we taint the whole VM?

There is also an opened issue (http://sourceforge.net/p/libjpeg/bugs/5/ - 5 years old!!!) telling it does not compile on win32 (not sure which compiler though, it might be MSVC only), so this might be a hurdle for step 1).

So what is the plan in details?

Nicolas Cellier

Re: VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

2014-05-11 9:18 GMT+02:00 Nicolas Cellier <[hidden email]>:

2014-05-11 3:48 GMT+02:00 David T. Lewis <[hidden email]>:

On Sat, May 10, 2014 at 03:00:41PM -0700, tim Rowledge wrote:
> There?s a related issue that could do with a fix sometime - the jpeg library we used to make the jepegreadwriteplugin is out of date and Debian (at least) considers that a security risk and thus they mangle the sources to not use those file but link to the ?standard? library. Which unfortunately doesn?t actually work within our plugin!
>
> Leaving aside the sheer rudeness of throwing unapproved ?patches? into somebody?s code (without ever letting Ian know, for example) and resulting in a broken function, I imagine that if looked into we would be able to tweak our plugin code to actually work with the new jpeg library code. This would be a Good Thing. If anyone knows enough about jpeg library, Debian, blah blah blah to look into this then it might make a nice project that would benefit the Raspberry Pi Scratch using world - several tens of thousands of kids in the UK alone.
>

This issue is tracked in Mantis 7539: "Plugins should use platform libraries
where possible. Gentoo removed Squeak for this reason."

http://bugs.squeak.org/view.php?id=7539

It was originally reported based on Gentoo rejecting Squeak, and it affects Debian
and other Linux distributions that (quite reasonably) expect that supported
applications should link to current versions of system libraries.

If anyone is interested in working to improve this, please do. It would be a great
way for someone to contribute a really useful update to the VM.

Dave

A general idea would be to proceed in two steps:
-1) upgrade the library in squeak svn repository, so as to fix the API evolutions mismatch

-2) provide a way to link to existing library (via cmake/configure in VMMaker and apt/rpm package dependency at distrib level)

Good plan, but what are the starting points?
Which library exactly should be used?
libjpeg - http://sourceforge.net/projects/libjpeg/ - which is the successor of the library used in squeak is licenced GPL v2, should we taint the whole VM?

There is also an opened issue (http://sourceforge.net/p/libjpeg/bugs/5/ - 5 years old!!!) telling it does not compile on win32 (not sure which compiler though, it might be MSVC only), so this might be a hurdle for step 1).

So what is the plan in details?

Maybe http://www.openjpeg.org/ is a better match, BSD license... But it's huge.

Bert Freudenberg

Re: 16 bit GIFs (was: A nitpick)

In reply to this post by Yoshiki Ohshima-3

On 2014-05-09, at 19:36, Yoshiki Ohshima <[hidden email]> wrote:

> On Fri, May 9, 2014 at 1:14 AM, Bert Freudenberg <[hidden email]> wrote:
>> Do we support any 16 bit-per-channel gifs?
>
> Not sure what you mean by "gifs"

Yes, I meant pngs. Slip of mind.

> (I am specifically talking about
> PNGs), but sure, I think Squeak should be able to read a PNG file, as
> long as it is a valid one. If it is 16 bits per channel, it should
> downscale the depth to 8 and create a 32 bpp Forms. This does happen
> in most of the cases, but when the PNG file specifies the transparent
> pixel (it is also in 16 bit value), it causes an error.
> --
> -- Yoshiki

Ah, okay. I wasn't even aware Squeak reads 16 bpp images at all, because bitblt supports at most 8 bpp.

- Bert -

smime.p7s (5K) Download Attachment

David T. Lewis

Re: VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

In reply to this post by Nicolas Cellier

On Sun, May 11, 2014 at 09:38:06AM +0200, Nicolas Cellier wrote:

> 2014-05-11 9:18 GMT+02:00 Nicolas Cellier <
> [hidden email]>:
>
> >
> > 2014-05-11 3:48 GMT+02:00 David T. Lewis <[hidden email]>:
> >
> > On Sat, May 10, 2014 at 03:00:41PM -0700, tim Rowledge wrote:
> >> > There?s a related issue that could do with a fix sometime - the jpeg
> >> library we used to make the jepegreadwriteplugin is out of date and Debian
> >> (at least) considers that a security risk and thus they mangle the sources
> >> to not use those file but link to the ?standard? library. Which
> >> unfortunately doesn?t actually work within our plugin!
> >> >
> >> > Leaving aside the sheer rudeness of throwing unapproved ?patches? into
> >> somebody?s code (without ever letting Ian know, for example) and resulting
> >> in a broken function, I imagine that if looked into we would be able to
> >> tweak our plugin code to actually work with the new jpeg library code. This
> >> would be a Good Thing. If anyone knows enough about jpeg library, Debian,
> >> blah blah blah to look into this then it might make a nice project that
> >> would benefit the Raspberry Pi Scratch using world - several tens of
> >> thousands of kids in the UK alone.
> >> >
> >>
> >> This issue is tracked in Mantis 7539: "Plugins should use platform
> >> libraries
> >> where possible. Gentoo removed Squeak for this reason."
> >>
> >> http://bugs.squeak.org/view.php?id=7539
> >>
> >> It was originally reported based on Gentoo rejecting Squeak, and it
> >> affects Debian
> >> and other Linux distributions that (quite reasonably) expect that
> >> supported
> >> applications should link to current versions of system libraries.
> >>
> >> If anyone is interested in working to improve this, please do. It would
> >> be a great
> >> way for someone to contribute a really useful update to the VM.
> >>
> >> Dave
> >>
> >>
> >>
> > A general idea would be to proceed in two steps:
> > -1) upgrade the library in squeak svn repository, so as to fix the API
> > evolutions mismatch
> > -2) provide a way to link to existing library (via cmake/configure in
> > VMMaker and apt/rpm package dependency at distrib level)
> >
> > Good plan, but what are the starting points?
> > Which library exactly should be used?
> > libjpeg - http://sourceforge.net/projects/libjpeg/ - which is the
> > successor of the library used in squeak is licenced GPL v2, should we taint
> > the whole VM?
> > There is also an opened issue (http://sourceforge.net/p/libjpeg/bugs/5/ -
> > 5 years old!!!) telling it does not compile on win32 (not sure which
> > compiler though, it might be MSVC only), so this might be a hurdle for step
> > 1).
> >
> > So what is the plan in details?
> >
>
> Maybe http://www.openjpeg.org/ is a better match, BSD license... But it's
> huge.

The Mantis entry has links to the earlier discussions, so there is a good
summary of the issues there. I do not know what the detailed plan should be
but I think that we need two things:

1) Some platforms, probably including some unix platforms, need to use the
old jpeg library code that we keep in platforms/Cross/plugins/JPEGReadWriter2Plugin.
This must continue to work for the platforms that require it.

2) Other platforms, especially Linux distributions, would prefer to link
directly to the jpeg runtime library provided by the operating system.

There should be no GPL issues because linking to a runtime libjpeg is no
different from linking to any other runtime library on Linux.

It may be that the best way to achieve this is to provide different variants
of jpeg plugin in VMMaker. But I have not looked at this in detail, so I
cannot say what is the best approach.

Dave

Yoshiki Ohshima-3

Re: 16 bit GIFs (was: A nitpick)

In reply to this post by Nicolas Cellier

Thank you, Nicholas!

On Sat, May 10, 2014 at 6:46 AM, Nicolas Cellier
<[hidden email]> wrote:

> After further fixes i tested these forms successfully:
>
>
> #(
> pal_bk pal rgba8_bk rgba8 rgba16_bk rgba16
> rgb8_t_bk rgb8_t rgb16_t_bk rgb16_t
> gray8a_bk gray8a gray16a_bk gray16a
> gray8b_bk gray8b gray16b_bk gray16b
> pal_bk_notrns palb
> )
> do: [:e |
> (ImageReadWriter formFromStream:
> ('http://entropymine.com/jason/testbed/pngtrans/',e,'.png') asUrl
> retrieveContents contentStream) asMorph openInWorld].
>
>
>
> 2014-05-10 12:36 GMT+02:00 Nicolas Cellier
> <[hidden email]>:
>
>> I just committed a fix.
>>
>> The PNGReadWriter currenlty has a hack to support 16 bits per channel gray
>> scale (with a slow bit poker loop), but not with transparency...
>> - transparency is handled with a palette
>> - ColorForm only supports depth <= 8
>> Since the Form is forced to depth 8, and since we must choose an entry in
>> the palette for transparency, I forced the transparency to be stored at 1st
>> palette entry (value 0) in case of 16 bitsPerChannel.
>>
>> It seems to me that there was another bug even for bitsPerPixel <= 8: the
>> palette is 1-based indexed so transparent color index was one off.
>>
>> It also seems to me that the inversion 255-pixelValue is wrong in the 16
>> bitsPerChannel bitPoker loop , I did change it.
>>
>>
>> 2014-05-09 19:36 GMT+02:00 Yoshiki Ohshima <[hidden email]>:
>>
>>> On Fri, May 9, 2014 at 1:14 AM, Bert Freudenberg <[hidden email]>
>>> wrote:
>>> > Do we support any 16 bit-per-channel gifs?
>>>
>>> Not sure what you mean by "gifs" (I am specifically talking about
>>> PNGs), but sure, I think Squeak should be able to read a PNG file, as
>>> long as it is a valid one. If it is 16 bits per channel, it should
>>> downscale the depth to 8 and create a 32 bpp Forms. This does happen
>>> in most of the cases, but when the PNG file specifies the transparent
>>> pixel (it is also in 16 bit value), it causes an error.
>>> --
>>> -- Yoshiki
>>>
>>
>
>
>
>

--
-- Yoshiki

Nicolas Cellier

Re: VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

In reply to this post by David T. Lewis

2014-05-11 16:20 GMT+02:00 David T. Lewis <[hidden email]>:

On Sun, May 11, 2014 at 09:38:06AM +0200, Nicolas Cellier wrote:
> 2014-05-11 9:18 GMT+02:00 Nicolas Cellier <
> [hidden email]>:
>
> >
> > 2014-05-11 3:48 GMT+02:00 David T. Lewis <[hidden email]>:
> >
> > On Sat, May 10, 2014 at 03:00:41PM -0700, tim Rowledge wrote:
> >> > There?s a related issue that could do with a fix sometime - the jpeg
> >> library we used to make the jepegreadwriteplugin is out of date and Debian
> >> (at least) considers that a security risk and thus they mangle the sources
> >> to not use those file but link to the ?standard? library. Which
> >> unfortunately doesn?t actually work within our plugin!
> >> >
> >> > Leaving aside the sheer rudeness of throwing unapproved ?patches? into
> >> somebody?s code (without ever letting Ian know, for example) and resulting
> >> in a broken function, I imagine that if looked into we would be able to
> >> tweak our plugin code to actually work with the new jpeg library code. This
> >> would be a Good Thing. If anyone knows enough about jpeg library, Debian,
> >> blah blah blah to look into this then it might make a nice project that
> >> would benefit the Raspberry Pi Scratch using world - several tens of
> >> thousands of kids in the UK alone.
> >> >
> >>
> >> This issue is tracked in Mantis 7539: "Plugins should use platform
> >> libraries
> >> where possible. Gentoo removed Squeak for this reason."
> >>
> >> http://bugs.squeak.org/view.php?id=7539
> >>
> >> It was originally reported based on Gentoo rejecting Squeak, and it
> >> affects Debian
> >> and other Linux distributions that (quite reasonably) expect that
> >> supported
> >> applications should link to current versions of system libraries.
> >>
> >> If anyone is interested in working to improve this, please do. It would
> >> be a great
> >> way for someone to contribute a really useful update to the VM.
> >>
> >> Dave
> >>
> >>
> >>
> > A general idea would be to proceed in two steps:
> > -1) upgrade the library in squeak svn repository, so as to fix the API
> > evolutions mismatch
> > -2) provide a way to link to existing library (via cmake/configure in
> > VMMaker and apt/rpm package dependency at distrib level)
> >
> > Good plan, but what are the starting points?
> > Which library exactly should be used?
> > libjpeg - http://sourceforge.net/projects/libjpeg/ - which is the
> > successor of the library used in squeak is licenced GPL v2, should we taint
> > the whole VM?
> > There is also an opened issue (http://sourceforge.net/p/libjpeg/bugs/5/ -
> > 5 years old!!!) telling it does not compile on win32 (not sure which
> > compiler though, it might be MSVC only), so this might be a hurdle for step
> > 1).
> >
> > So what is the plan in details?
> >
>
> Maybe http://www.openjpeg.org/ is a better match, BSD license... But it's
> huge.

The Mantis entry has links to the earlier discussions, so there is a good
summary of the issues there. I do not know what the detailed plan should be
but I think that we need two things:

1) Some platforms, probably including some unix platforms, need to use the
old jpeg library code that we keep in platforms/Cross/plugins/JPEGReadWriter2Plugin.
This must continue to work for the platforms that require it.

2) Other platforms, especially Linux distributions, would prefer to link
directly to the jpeg runtime library provided by the operating system.

There should be no GPL issues because linking to a runtime libjpeg is no
different from linking to any other runtime library on Linux.

Beware, this is why LGPL was created for, no?
But GPL is different from LGPL.

I don't feel like reading those licenses again, it's just too boring, so someone should confirm that:
link with GPL and you become GPL, it's viral.

It may be that the best way to achieve this is to provide different variants
of jpeg plugin in VMMaker. But I have not looked at this in detail, so I
cannot say what is the best approach.

Dave

David T. Lewis

Re: VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

On Fri, May 16, 2014 at 10:51:17PM +0200, Nicolas Cellier wrote:

> 2014-05-11 16:20 GMT+02:00 David T. Lewis <[hidden email]>:
> >
> > The Mantis entry has links to the earlier discussions, so there is a good
> > summary of the issues there. I do not know what the detailed plan should be
> > but I think that we need two things:
> >
> > 1) Some platforms, probably including some unix platforms, need to use the
> > old jpeg library code that we keep in
> > platforms/Cross/plugins/JPEGReadWriter2Plugin.
> > This must continue to work for the platforms that require it.
> >
> > 2) Other platforms, especially Linux distributions, would prefer to link
> > directly to the jpeg runtime library provided by the operating system.
> >
> > There should be no GPL issues because linking to a runtime libjpeg is no
> > different from linking to any other runtime library on Linux.
> >
> >
> Beware, this is why LGPL was created for, no?
> But GPL is different from LGPL.
> I don't feel like reading those licenses again, it's just too boring, so
> someone should confirm that:
> link with GPL and you become GPL, it's viral.
>

Good catch. I don't know if it is a problem in this case, but I added a note
to the Mantis issue so we do not forget to check.

Thanks,
Dave

dcorking

Re: VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

License
======

Sourceforge does not appear to host the current development of
libjpeg, and seems to have caused us unnecessary worry about the
license. The SF site has version 6b, which is 6 years behind Guido
Vollbeding's version 9a. (I think SF may have trouble displaying licenses
that aren't from a standard short list, so ended up with the wrong one.)

Version 9a is at http://ijg.org/

Whether from Sourceforge or IJG or from Squeak's own repository[1],
the license is not GPL. It is this custom but liberal license:

http://directory.fsf.org/wiki?title=License:JPEG

I am not a lawyer. I can't imagine incompatibilities with Squeak's
Apache and MIT licenses.

Compatibility
=========
Ubuntu and Fedora prefer libjpeg-turbo, a fork which uses the same
liberal license and, I hope, the same API.

libjpeg-turbo really is on Sourceforge:

http://sourceforge.net/p/libjpeg-turbo/code/HEAD/tree/trunk/

I am not in any way a C or SLANG hacker, though I would like to learn.
Yet a quick browse of change.log in libjpegsr9a gives me some hope
that the API has not had many breaking changes since version 6a, which
I guess is the version that is in the Squeak platform sources.

While I have them handy, here are the version numbers as I found them
installed on my machines:
Debian 7.1 libjpeg8 8d-1 (binary is libjpeg.so.8.4.0)
Fedora 20 libjpeg-turbo 1.3.1 (binary is libjpeg.so.62.1.0)
Ubuntu 12.04 (old I know) libjpeg-turbo 1.1.90+svn733-0ubuntu4.3
(binary is libjpeg.so.8.4.0)

Guido's site links to this one, which hopefully will offer more
specific clues about what Squeak would need to change to work with the
newer platform libraries..

http://jpegclub.org/support/

Hope that helps. Have fun!

David

[1]: http://squeakvm.org/cgi-bin/viewvc.cgi/squeak/trunk/platforms/Cross/plugins/JPEGReadWriter2Plugin/
as the other David mentioned

PS:
It looks like the libjpeg README didn't make it into the Squeak repo,
unless I looked in the wrong part of the tree. Would you like me to
post a request for this in Mantis?

David T. Lewis

Re: VM should use platform libraries where possible (was: 16 bit GIFs (was: A nitpick))

David,

Thanks very much for this information. I have added it to the Mantis entry.

Regarding the README for libjpeg, I think that the only README that should
be added to the platforms source tree would be the one from 12 years ago
that would match the actual source we are using in Cross. It's probably not
worth worrying about at this point.

Dave

On Mon, May 19, 2014 at 08:33:11PM +0100, David Corking wrote:

> License
> ======
>
> Sourceforge does not appear to host the current development of
> libjpeg, and seems to have caused us unnecessary worry about the
> license. The SF site has version 6b, which is 6 years behind Guido
> Vollbeding's version 9a. (I think SF may have trouble displaying licenses
> that aren't from a standard short list, so ended up with the wrong one.)
>
> Version 9a is at http://ijg.org/
>
> Whether from Sourceforge or IJG or from Squeak's own repository[1],
> the license is not GPL. It is this custom but liberal license:
>
> http://directory.fsf.org/wiki?title=License:JPEG
>
> I am not a lawyer. I can't imagine incompatibilities with Squeak's
> Apache and MIT licenses.
>
> Compatibility
> =========
> Ubuntu and Fedora prefer libjpeg-turbo, a fork which uses the same
> liberal license and, I hope, the same API.
>
> libjpeg-turbo really is on Sourceforge:
>
> http://sourceforge.net/p/libjpeg-turbo/code/HEAD/tree/trunk/
>
> I am not in any way a C or SLANG hacker, though I would like to learn.
> Yet a quick browse of change.log in libjpegsr9a gives me some hope
> that the API has not had many breaking changes since version 6a, which
> I guess is the version that is in the Squeak platform sources.
>
> While I have them handy, here are the version numbers as I found them
> installed on my machines:
> Debian 7.1 libjpeg8 8d-1 (binary is libjpeg.so.8.4.0)
> Fedora 20 libjpeg-turbo 1.3.1 (binary is libjpeg.so.62.1.0)
> Ubuntu 12.04 (old I know) libjpeg-turbo 1.1.90+svn733-0ubuntu4.3
> (binary is libjpeg.so.8.4.0)
>
> Guido's site links to this one, which hopefully will offer more
> specific clues about what Squeak would need to change to work with the
> newer platform libraries..
>
> http://jpegclub.org/support/
>
> Hope that helps. Have fun!
>
> David
>
> [1]: http://squeakvm.org/cgi-bin/viewvc.cgi/squeak/trunk/platforms/Cross/plugins/JPEGReadWriter2Plugin/
> as the other David mentioned
>
> PS:
> It looks like the libjpeg README didn't make it into the Squeak repo,
> unless I looked in the wrong part of the tree. Would you like me to
> post a request for this in Mantis?