Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] ext/gd: support creation of animated GIFs

Posted by Christoph M. Becker 
Christoph M. Becker
[PHP-DEV] ext/gd: support creation of animated GIFs
January 09, 2017 01:00AM
Hi!

A while ago I've grabbed up https://bugs.php.net/32803 and submitted
PR #2024[1]. This lay dormant until recently when Joe had a look at so
many PRs (thanks!), so I'm bringing this issue to your attention.

I presume nobody is opposed that ext/gd should support the creation of
animated GIFs (they appear to be still in fashion again), but I'm not
sure about the API. My first shot (as implemented in the PR) was to
offer a rather minimal layer over gdImageGifAnimBeginCtx(),
gdImageGifAnimAddCtx() and gdImageGifAnimEndCtx()[2] (you can see an
usage example in the submitted PHPT[3]).

Kalle raised some objections regarding the direct use of streams in the
API, namely that imagegifanimbegin() expects an open stream to be passed
as parameter, so perhaps it would be better to accept a stream URL
instead and manage the stream behind the scenes. That appears to be
much more solid (as the developer couldn't fiddle with the stream), but
would require some encapsulation mechanism, either a resource, what
would be in line with the other GD resource types[4], or an (opaque)
object, what appears more suitable for PHP 7.

Currently, I'd prefer an *opaque* object which would be created by
imagegifanimbegin() (creating the respective stream internally), passed
to imageanimadd() and be destroyed by imagegifanimend().

Before proceeding to the implementation, I'd like to hear your thoughts
about that!

[1] https://github.com/php/php-src/pull/2024
[2] https://libgd.github.io/manuals/2.2.3/files/gd_gif_out-c.html
[3]
<https://github.com/php/php-src/pull/2024/files#diff-bbb4c7a8497032f98d062ac418d9fbbe>;
[4] http://php.net/manual/en/image.resources.php

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Joe Watkins
Re: [PHP-DEV] ext/gd: support creation of animated GIFs
January 09, 2017 07:30AM
Morning Christoph,

We have to face facts: Resources are still a thing.

While there is talk of switching resources to use opaque objects, I'm
worried about introducing a mixture of objects and resources in random
places throughout core exts, and having exts with both seems very strange
to me.

I know that this has already started to happen, but before it goes any
further I would implore you to stick to resources for now.

There needs to be an organised effort for some future version of PHP to
remove the resource type and replace it with opaque (or not) objects.

Cheers
Joe

On Sun, Jan 8, 2017 at 11:53 PM, Christoph M. Becker <[email protected]>
wrote:

> Hi!
>
> A while ago I've grabbed up https://bugs.php.net/32803 and submitted
> PR #2024[1]. This lay dormant until recently when Joe had a look at so
> many PRs (thanks!), so I'm bringing this issue to your attention.
>
> I presume nobody is opposed that ext/gd should support the creation of
> animated GIFs (they appear to be still in fashion again), but I'm not
> sure about the API. My first shot (as implemented in the PR) was to
> offer a rather minimal layer over gdImageGifAnimBeginCtx(),
> gdImageGifAnimAddCtx() and gdImageGifAnimEndCtx()[2] (you can see an
> usage example in the submitted PHPT[3]).
>
> Kalle raised some objections regarding the direct use of streams in the
> API, namely that imagegifanimbegin() expects an open stream to be passed
> as parameter, so perhaps it would be better to accept a stream URL
> instead and manage the stream behind the scenes. That appears to be
> much more solid (as the developer couldn't fiddle with the stream), but
> would require some encapsulation mechanism, either a resource, what
> would be in line with the other GD resource types[4], or an (opaque)
> object, what appears more suitable for PHP 7.
>
> Currently, I'd prefer an *opaque* object which would be created by
> imagegifanimbegin() (creating the respective stream internally), passed
> to imageanimadd() and be destroyed by imagegifanimend().
>
> Before proceeding to the implementation, I'd like to hear your thoughts
> about that!
>
> [1] https://github.com/php/php-src/pull/2024
> [2] https://libgd.github.io/manuals/2.2.3/files/gd_gif_out-c.html
> [3]
> <https://github.com/php/php-src/pull/2024/files#diff-
> bbb4c7a8497032f98d062ac418d9fbbe>
> [4] http://php.net/manual/en/image.resources.php
>
> --
> Christoph M. Becker
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>
Andrea Faulds
[PHP-DEV] Re: ext/gd: support creation of animated GIFs
January 10, 2017 08:30PM
Hi,

Christoph M. Becker wrote:
> On 09.01.2017 at 21:19, Andrea Faulds wrote:
>
>> Would this opaque object still allow you to use an arbitrary stream of
>> your choice?
>>
>> Also, like with imagepng() etc., could you stream the output to uh…
>> PHP's default output stream? (I'm not sure what it's called. The thing
>> that `echo` goes to.)
>
> It would be possible to allow either a stream URL or a stream to given,
> what would fit to image(png|jpeg|…)(). However, these are "stand-alone"
> functions which close the stream when they're finished, while the
> animated GIF writing functions would *allow* the stream to be
> manipulated by the userland developer, even though I can't see practical
> reason to do so.
>
> And, of course, we could also let the functions accept NULL to directly
> write to stdout. Passing in 'php://stdout' should have the same effect.

Alright.

>
>> If it does both, I don't have much objection.
>>
>> Mind you… is it really necessary to hide the stream from the user?
>> fopen() is not particularly hard to use, and there are (admittedly
>> niche) cases where you might want to handle the stream yourself.
>
> Are there really such cases? Should we sacrifice the safety of internal
> stream handling to support some (hypothetical?) use cases?
>

In fairness, I'm not sure. I know animated GIF streaming has been tried
before, though I doubt if it's actually used at all these days. A more
reasonable case might be embedding an animated GIF in other binary data.
But that's a hypothetical.

What is the actual “safety” gain of not using streams? That you don't
need to use fopen() yourself? (PHP will close files for you.) I'm just
not super convinced there's a need for adding this extra layer.

Thanks for your response.

--
Andrea Faulds
https://ajf.me/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Christoph M. Becker
[PHP-DEV] Re: ext/gd: support creation of animated GIFs
January 13, 2017 07:00PM
Hi!

On 10.01.2017 at 20:24, Andrea Faulds wrote:

> Christoph M. Becker wrote:
>
>> On 09.01.2017 at 21:19, Andrea Faulds wrote:
>>
>>> Would this opaque object still allow you to use an arbitrary stream of
>>> your choice?
>>>
>>> Also, like with imagepng() etc., could you stream the output to uh…
>>> PHP's default output stream? (I'm not sure what it's called. The thing
>>> that `echo` goes to.)
>>
>> It would be possible to allow either a stream URL or a stream to given,
>> what would fit to image(png|jpeg|…)(). However, these are "stand-alone"
>> functions which close the stream when they're finished, while the
>> animated GIF writing functions would *allow* the stream to be
>> manipulated by the userland developer, even though I can't see practical
>> reason to do so.
>>
>> And, of course, we could also let the functions accept NULL to directly
>> write to stdout. Passing in 'php://stdout' should have the same effect.
>
> Alright.
>
>>
>>> If it does both, I don't have much objection.
>>>
>>> Mind you… is it really necessary to hide the stream from the user?
>>> fopen() is not particularly hard to use, and there are (admittedly
>>> niche) cases where you might want to handle the stream yourself.
>>
>> Are there really such cases? Should we sacrifice the safety of internal
>> stream handling to support some (hypothetical?) use cases?
>
> In fairness, I'm not sure. I know animated GIF streaming has been tried
> before, though I doubt if it's actually used at all these days. A more
> reasonable case might be embedding an animated GIF in other binary data.
> But that's a hypothetical.

Okay.

> What is the actual “safety” gain of not using streams? That you don't
> need to use fopen() yourself? (PHP will close files for you.) I'm just
> not super convinced there's a need for adding this extra layer.

It's not about having to fopen() the stream – in my opinion at least
that's a non-brainer. What is a minor issue though, is that either
imagegifanimend() closes the stream automatically (what imagepng() and
friends are doing), or not (in my opinion the proper solution, but
inconsistent). The major issue for me would be the possibility that the
user fiddles with the stream between imagegifanimbegin() and
imagegifanimend() – in the worst case closing the stream, so
imagegifanimend() would fail or had to catch that. Of course, that
would be a programmer error, but nonetheless it would have to be
documented, and I'm pretty sure that at least some devs would walk into
this trap.

Anyhow, thanks for the feedback. I'll see how to unify this in a single
RFC (probably with secondary voting options).

--
Christoph M. Becker


--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Christoph M. Becker
Re: [PHP-DEV] ext/gd: support creation of animated GIFs
January 15, 2017 02:50PM
On 15.01.2017 at 11:53, Pierre Joye wrote:

> Just got a bunch of emails and see that one. I agree about object for
> anims. Make the APIs also more friendly.
>
> I will post a detailed reply by Wednesday

Thanks!

Cheers,
Christoph

> Thanks for your work!
> Pierre
>
> On Jan 9, 2017 7:53 AM, "Christoph M. Becker" <[email protected]> wrote:
>
>> Hi!
>>
>> A while ago I've grabbed up https://bugs.php.net/32803 and submitted
>> PR #2024[1]. This lay dormant until recently when Joe had a look at so
>> many PRs (thanks!), so I'm bringing this issue to your attention.
>>
>> I presume nobody is opposed that ext/gd should support the creation of
>> animated GIFs (they appear to be still in fashion again), but I'm not
>> sure about the API. My first shot (as implemented in the PR) was to
>> offer a rather minimal layer over gdImageGifAnimBeginCtx(),
>> gdImageGifAnimAddCtx() and gdImageGifAnimEndCtx()[2] (you can see an
>> usage example in the submitted PHPT[3]).
>>
>> Kalle raised some objections regarding the direct use of streams in the
>> API, namely that imagegifanimbegin() expects an open stream to be passed
>> as parameter, so perhaps it would be better to accept a stream URL
>> instead and manage the stream behind the scenes. That appears to be
>> much more solid (as the developer couldn't fiddle with the stream), but
>> would require some encapsulation mechanism, either a resource, what
>> would be in line with the other GD resource types[4], or an (opaque)
>> object, what appears more suitable for PHP 7.
>>
>> Currently, I'd prefer an *opaque* object which would be created by
>> imagegifanimbegin() (creating the respective stream internally), passed
>> to imageanimadd() and be destroyed by imagegifanimend().
>>
>> Before proceeding to the implementation, I'd like to hear your thoughts
>> about that!
>>
>> [1] https://github.com/php/php-src/pull/2024
>> [2] https://libgd.github.io/manuals/2.2.3/files/gd_gif_out-c.html
>> [3]
>> <https://github.com/php/php-src/pull/2024/files#diff-
>> bbb4c7a8497032f98d062ac418d9fbbe>
>> [4] http://php.net/manual/en/image.resources.php
>>
>> --
>> Christoph M. Becker
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>>
>

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login