| <<<Back 1 day (to 2019/02/11) | 20190212 |
sebras | chrisl: may I trouble you with two reviews? | 17:03.43 |
| chrisl: http://git.ghostscript.com/?p=user/sebras/ghostpdl.git;a=commitdiff;h=be1a327733c10aafa99f7772b0f3aa287df58a1f which is a precondition for | 17:03.51 |
| chrisl: this one http://git.ghostscript.com/?p=user/sebras/ghostpdl.git;a=commitdiff;h=2486669a97238eef7a23dcbe4d68ef733820170a | 17:04.01 |
chrisl | sebras: give me a few moments to finish a commit message | 17:04.32 |
sebras | the aim is to have the jbig2 decoders use a custom allocator based on gs' | 17:04.33 |
| chrisl: no worries. | 17:04.52 |
chrisl | sebras: So..... Is there a specific reason you opted to have jbig2_global_ctx_free() return the allocator, rather than just have it incumbent on the caller to store it's allocator? (I'm trying to remember how the other shared libs do this) | 17:14.34 |
sebras | if I did it that way s_jbig2decode_release() can get the allocator back without having it stored elsewhere. | 17:15.41 |
| besides the mem pointer the allocator can be constant really. | 17:16.16 |
| so another option would be to change jbig2dec so that jbig2_ctx_new() takes a separate allocator_data and stores it on its own. | 17:16.44 |
chrisl | My only worry is the change to the ABI might irritate some folks | 17:16.53 |
sebras | a problem in s_jbig2decode_make_global_data() is that there is really nowhere to store it. | 17:17.49 |
| perhaps to return something other than Jbig2GlobalCtx in *result | 17:18.12 |
| basically wrap Jbig2GlobalCtx and Jbig2Allocator in a new struct and have s_jbig2decode_make_global_data() and s_jbig2decode_free_global_data() cast to/from it upon alloc/release. | 17:19.28 |
chrisl | TBH, I like the way you have it, I'm just wondering about the hassle we'll get for changing the interface again | 17:20.17 |
sebras | I like this way because it is clear at what point the library will stop using the allocator. | 17:20.46 |
| I'll whip up a patch with the alternative just to see if it is feasible. | 17:21.19 |
| chrisl: I remember you being wary of the API/ABI last time and then I changed it nonetheless and then people complained... | 17:21.41 |
| ergh... 6 differences in the cluster. I'm bmpcmping and hoping that they amount to nothing. | 17:23.18 |
| chrisl: btw, I ran the current version with the gpdlsanitize target on a pdf that I know has jbig2 images in it and it works fine now. | 17:25.01 |
chrisl | sebras: I think we should go with what you have, but if you can add a comment to CHANGES - even if it's just something to remind me to write something more detailed. | 17:26.02 |
sebras | chrisl: so you advice against the wrapping/unwrapping of the Jbig2Allocator and global context? | 17:26.48 |
chrisl | sebras: I think that complicates things more than I like | 17:27.12 |
sebras | alright. | 17:27.24 |
| I'll add something to CHANGES under version 0.16 in that case. | 17:27.54 |
| chrisl: thanks for reviewing btw. | 17:28.08 |
chrisl | NP | 17:28.20 |
sebras | chrisl: yey, the cluster and bmpcmp went through without issues. | 17:44.13 |
| chrisl: then I'll add something to CHANGES, spell check it and merge..? | 17:44.31 |
chrisl | sebras: Sounds good | 17:44.40 |
sebras | sebras: since the changed functions used to return void I don't forsee that this would be a major issue. | 17:45.32 |
| chrisl: btw, I'll just leave the date for 0.16 empty. | 17:46.06 |
| there might be more changes... | 17:46.09 |
chrisl | Sure - as I said, I'll probably add extra words for the release. I just need you to add enough in there to remind me. Otherwise I'll end up writing "Stuff changed....." | 17:47.18 |
sebras | "Bug fix release." seems to be popular... :) | 17:47.39 |
chrisl | Yeh, but if we change the interface, I feel we need to be more explicit. | 17:48.16 |
sebras | I think we did last time, but we weren't. or I wasn't and I that certainly didn't help you. | 17:49.29 |
| chrisl: how does the change in c972f9f3a look? | 17:50.12 |
| s/change/CHANGES/ | 17:50.22 |
chrisl | Can I suggest adding "API change" or something? Again, mainly as a reminder | 17:51.35 |
sebras | as far as I can tell this change will be backwards compatible. | 17:57.13 |
| do you agree? | 17:57.16 |
chrisl | I suspect it'll depend on compiler options | 18:00.25 |
| But yeh, normally it should only introduce a warning | 18:01.04 |
sebras | chrisl: alright, "API change" has been added. | 18:02.12 |
| chrisl: I'll wait for you to update to 0.16 at release time, I hope that's ok? | 18:02.47 |
chrisl | sebras: Yeh, as I said, I just need a reminder that more was required than "bug fix release" :-) | 18:04.54 |
sebras | ok, then I'll merge to master now. yey! :) | 18:05.16 |
| Forward 1 day (to 2019/02/13)>>> | |