[poppler] Toward to JBIG2 support in CairoOutputDev
suzuki toshiya
mpsuzuki at hiroshima-u.ac.jp
Mon Jan 19 08:12:32 PST 2015
Dear Carlos,
Sorry! I had a silly mistake, I should pass ref->getRef()
(in my previous patch, I passed ref->isRef()).
I attach corrected patch.
Regards,
mpsuzuki
suzuki toshiya wrote:
> Dear Carlos,
>
> Thank you for finding out the points! I attached revised patch.
> The difference from my previous revision is following.
>
> Regards,
> mpsuzuki
>
>
> diff --git a/poppler/CairoOutputDev.cc b/poppler/CairoOutputDev.cc
> index a428d2c..17b6b3b 100644
> --- a/poppler/CairoOutputDev.cc
> +++ b/poppler/CairoOutputDev.cc
> @@ -2833,13 +2833,12 @@ void CairoOutputDev::setMimeData(GfxState *state, Stream *str, Object *ref,
> #endif
>
> if (getStreamData (str->getNextStream(), &strBuffer, &len)) {
> - cairo_status_t status;
> + cairo_status_t status = CAIRO_STATUS_SUCCESS;
>
> #if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 11, 2)
> if (ref && ref->isRef()) {
> - Ref imgRef = ref->getRef();
> status = setMimeIdFromRef(image, CAIRO_MIME_TYPE_UNIQUE_ID,
> - "poppler-surface-", imgRef);
> + "poppler-surface-", ref->isRef());
> }
> #endif
> if (!status) {
>
>
>
> Carlos Garcia Campos wrote:
>> Adrian Johnson <ajohnson at redneon.com> writes:
>>
>>> The latest patch looks good. I'll wait a week before committing to see
>>> if anyone else has any comments. This will give me time to do some
>>> testing of the patch.
>> Looks good to me as well in general. There's only one thing I've
>> noticed.
>>
>>> @@ -2746,31 +2827,28 @@ void CairoOutputDev::setMimeData(GfxState *state, Stream *str, Object *ref,
>>> if (!colorMapHasIdentityDecodeMap(colorMap))
>>> return;
>>>
>>> +#if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 14, 0)
>>> + if (strKind == strJBIG2 && !setMimeDataForJBIG2Globals(str, image))
>>> + return;
>>> +#endif
>>> +
>>> if (getStreamData (str->getNextStream(), &strBuffer, &len)) {
>>> - cairo_status_t st;
>>> + cairo_status_t status;
>>>
>>> #if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 11, 2)
>>> if (ref && ref->isRef()) {
>>> Ref imgRef = ref->getRef();
>>> - GooString *surfaceId = new GooString("poppler-surface-");
>>> - surfaceId->appendf("{0:d}-{1:d}", imgRef.gen, imgRef.num);
>>> - char *idBuffer = copyString(surfaceId->getCString());
>>> - st = cairo_surface_set_mime_data (image, CAIRO_MIME_TYPE_UNIQUE_ID,
>>> - (const unsigned char *)idBuffer,
>>> - surfaceId->getLength(),
>>> - gfree, idBuffer);
>>> - if (st)
>>> - gfree(idBuffer);
>>> - delete surfaceId;
>>> + status = setMimeIdFromRef(image, CAIRO_MIME_TYPE_UNIQUE_ID,
>>> + "poppler-surface-", imgRef);
>> We could pass ref->getRef() directly here.
>>
>>> }
>>> #endif
>>> + if (!status) {
>> status might be uninitialized at this point if cairo version is not
>> recent enough or the ref && ref->isRef() condition is False.
>>
>>> + status = cairo_surface_set_mime_data (image, mime_type,
>>> + (const unsigned char *)strBuffer, len,
>>> + gfree, strBuffer);
>>> + }
>>
>>> - st = cairo_surface_set_mime_data (image,
>>> - str->getKind() == strDCT ?
>>> - CAIRO_MIME_TYPE_JPEG : CAIRO_MIME_TYPE_JP2,
>>> - (const unsigned char *)strBuffer, len,
>>> - gfree, strBuffer);
>>> - if (st)
>>> + if (status)
>>> gfree (strBuffer);
>>> }
>>> }
>>
>>> On 11/01/15 23:05, suzuki toshiya wrote:
>>>> Dear Adrian,
>>>>
>>>> OK - considering that there is a lag to the day that Linux
>>>> distribution ship the poppler stable branch including
>>>> CairoOutputDev with JBIG2 support even if I support some
>>>> snapshot of Cairo in current development branch of poppler
>>>> (and there would be no need to support Cairo snapshot in
>>>> the day), I decided to follow original version checking.
>>>>
>>>> Also I removed the default value (NULL) for Ref object to
>>>> JBIG2Stream constructor and removed NULL pointer checking
>>>> in JBIG2Stream constructor (as same as the argument to pass
>>>> Globals object).
>>>>
>>>> I replaced "st" in CairoOutputDev, by "status".
>>>>
>>>> Here I attached revised patch - oh, I'm quite sorry, my
>>>> previous posts were misunderstanding this year as 2014.
>>>>
>>>> Regards,
>>>> mpsuzuki
>>>>
>>>> Adrian Johnson wrote:
>>>>> On 08/01/15 13:35, suzuki toshiya wrote:
>>>>>> Dear Adrian,
>>>>>>
>>>>>> Great Thank you for prompt review! I attached the revised patch,
>>>>>> but please let me discuss a few points.
>>>>>>
>>>>>> Adrian Johnson wrote:
>>>>>>> On 07/01/15 02:58, suzuki toshiya wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> Here I submit a preliminary patch for git head,
>>>>>>>> which enables JBIG2 embedding to PDF surface via Cairo.
>>>>>>> +static cairo_status_t setJBIG2GlobalsIdByRefNums(int refNum, int
>>>>>>> refGen,
>>>>>>> + cairo_surface_t
>>>>>>> *image)
>>>>>>> +{
>>>>>>> + GooString *globalsStrId;
>>>>>>> + char *idBuffer;
>>>>>>> + cairo_status_t st;
>>>>>>> +
>>>>>>> +
>>>>>>> + globalsStrId = new GooString;
>>>>>>> + globalsStrId->appendf("{0:d}-{1:d}", refNum, refGen);
>>>>>>> + idBuffer = copyString(globalsStrId->getCString());
>>>>>>> + st = cairo_surface_set_mime_data (image,
>>>>>>> CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID,
>>>>>>>
>>>>>>> The cairo JBIG2 mime types are new to 1.14.0 so will need to be
>>>>>>> inside a
>>>>>>> #if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 14, 0).
>>>>>> If it's acceptable, I want to use the conditional like
>>>>>> #ifdef CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID
>>>>>> The official release of cairo including JBIG2 support is 1.14.0,
>>>>>> however, some distributions use a snapshot before 1.14 which
>>>>>> supports JBIG2. For example, Ubuntu 14.04 (long time support)
>>>>>> ships with 1.13.0~20140204.
>>>>>>
>>>>>> http://packages.ubuntu.com/search?suite=trusty§ion=all&arch=any&keywords=libcairo2&searchon=names
>>>>>>
>>>>>>
>>>>>> It includes JBIG2 support. Thus I prefer checking by the JBIG2
>>>>>> macros, instead of the version checking.
>>>>> I'm not keen on making an exception for this case. The target audience
>>>>> for this is very small. It would only help users that compile a new
>>>>> version of poppler on an old distribution without also recompiling
>>>>> cairo. JBIG2 support for pdftocairo is a very minor feature that not
>>>>> many people would be using (so far no one else has requested this
>>>>> feature).
>>>>>
>>>>> If you really want to support distributions using a cairo 1.13 snapshot
>>>>> I suggest adding a comment before the first occurrence of
>>>>> #ifdef CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID to explain why the version macro
>>>>> is not used. Also include
>>>>> "#if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 14, 0)" in the comment so
>>>>> that:
>>>>> a) searching for CAIRO_VERSION will find it, and
>>>>> b) to document that JBIG2 support is in 1.14 or later.
>>>>>
>>>>>> BTW, when the string for UNIQUE_ID is constructed, its syntax
>>>>>> is RefGen-RefNum. In PDF data, RefNum then RefGen is popular
>>>>>> syntax. Is there any reason why the order is reverted?
>>>>> No reason. Feel free to change it to num-gen order.
>>>>>
>>>>>>> +GBool CairoOutputDev::setMimeDataForJBIG2Globals(Stream *str,
>>>>>>> + cairo_surface_t
>>>>>>> *image)
>>>>>>> +{
>>>>>>> + JBIG2Stream *jbig2Str = static_cast<JBIG2Stream *>(str);
>>>>>>> + Object* globalsStr = jbig2Str->getGlobalsStream();
>>>>>>> + char *globalsBuffer;
>>>>>>> + int globalsLength;
>>>>>>> +
>>>>>>> + // According to PDF spec, Globals should be stream (or nothing)
>>>>>>> +
>>>>>>> + if (globalsStr->isNull())
>>>>>>> + return gTrue;
>>>>>>> +
>>>>>>> + if (!globalsStr->isStream())
>>>>>>> + return gFalse;
>>>>>>> +
>>>>>>> + // See JBIG2Stream constructor for valid reference number
>>>>>>> + if (!jbig2Str->getGlobalsStreamRefNum())
>>>>>>> + return gFalse;
>>>>>>>
>>>>>>> The globalsStr->isStream() check should be sufficient. The reference
>>>>>>> number should always be valid of the global stream is valid.
>>>>>> Umm. Requesting "both of JBIG2Stream::globalsStream and Ref to
>>>>>> it must be always valid as far as GlobalsStream is used" is
>>>>>> intuitive solution, but public JBIG2Stream constructor has not
>>>>>> requested the Ref to GlobalsStream before in earlier version.
>>>>>>
>>>>>> If some developers use JBIG2Stream constructor with valid
>>>>>> GlobalsStream but without Ref to it (as the syntax before my
>>>>>> patch), there is the situation that JBIG2Stream::globalsStream
>>>>>> is valid but Ref to it is invalid (if I supply the default
>>>>>> value for the argument to pass Ref). How do we detect or prevent
>>>>>> such case?
>>>>>>
>>>>>> A) Supplying the default value (NULL pointer) in the argument
>>>>>> to pass the Ref, and JBIG2Stream returns an error when
>>>>>> globalsStream is valid stream but its Ref is not passed - and
>>>>>> prevent the case that globalsStream is valid but its Ref is
>>>>>> invalid.
>>>>>>
>>>>>> B) Not supplying the default value, and prevent the construction
>>>>>> without the Ref to the globalsStream.
>>>>>>
>>>>>> C) Add new boolean value in JBIG2Stream class, to indicate if
>>>>>> Ref to globalsStream is explicitly set.
>>>>>>
>>>>>> D) Initialize Ref with irregular value (e.g. 0,0 or 0,65535)
>>>>>> internally when Ref is not passed, and deal the case as Ref to
>>>>>> the globalsStream is not initialized (my last patch design)
>>>>>>
>>>>>> Which is the best? Or, no need to case such case?
>>>>> JBIG2Stream is not part of the public API so there is no need to
>>>>> preserve compatibility.
>>>>>
>>>>> Something else I just noticed - could you rename "st" to "status".
>>>>> CairoOutputdev.cc always uses the name "status" for cairo status so we
>>>>> should be consistent with this convention.
>>>>>
>>> _______________________________________________
>>> poppler mailing list
>>> poppler at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
-------------- next part --------------
A non-text attachment was scrubbed...
Name: poppler-cairo-jbig2_20150119b.diff.xz
Type: application/octet-stream
Size: 2608 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20150120/c0c31bde/attachment-0001.obj>
More information about the poppler
mailing list