[poppler] poppler Digest, Vol 73, Issue 27
srinivas adicherla
srinivas.adicherla at gmail.com
Wed Mar 23 00:30:32 PDT 2011
Hi Carlos,
Thank you for your valuable suggestions. I made the changes
according to that. I attached a new patch with this.
- I have some doubt when setting a 'Rendition' action, we have two options
one is to set 'OP' other one is 'JS'. I am asking the user to pass the
Javascript and set it as a stream here.
- One more is i wrote a new function 'get_file_name_from_path' is it fine?
what is the good place to add this function.
Please give suggestions again.
Thanks
--
A Srinivas
On Mon, Mar 21, 2011 at 12:30 AM, <poppler-request at lists.freedesktop.org>wrote:
> Send poppler mailing list submissions to
> poppler at lists.freedesktop.org
>
> To subscribe or unsubscribe via the World Wide Web, visit
> http://lists.freedesktop.org/mailman/listinfo/poppler
> or, via email, send a message with subject or body 'help' to
> poppler-request at lists.freedesktop.org
>
> You can reach the person managing the list at
> poppler-owner at lists.freedesktop.org
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of poppler digest..."
>
>
> Today's Topics:
>
> 1. Re: [PATCH] Simplify the GCC compiler flags/warnings
> (Albert Astals Cid)
> 2. Re: glib/poppler-annot.cc glib/poppler-annot.h
> poppler/Annot.cc poppler/Annot.h (Carlos Garcia Campos)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Sun, 20 Mar 2011 13:59:24 +0000
> From: Albert Astals Cid <aacid at kde.org>
> Subject: Re: [poppler] [PATCH] Simplify the GCC compiler
> flags/warnings
> To: poppler at lists.freedesktop.org
> Message-ID: <201103201359.24744.aacid at kde.org>
> Content-Type: Text/Plain; charset="iso-8859-1"
>
> A Diumenge, 13 de mar? de 2011, Albert Astals Cid va escriure:
> > A Diumenge, 13 de mar? de 2011, Pino Toscano va escriure:
> > > Alle domenica 13 marzo 2011, Albert Astals Cid ha scritto:
> > > > A Dissabte, 12 de mar? de 2011, Pino Toscano va escriure:
> > > > > Alle sabato 12 marzo 2011, Hib Eris ha scritto:
> > > > > > Two more remarks:
> > > > > >
> > > > > > +if test "x$GCC" == xyes; then
> > > > > > + extra_cxxflags="-Wall -Wno-write-strings -Woverloaded-virtual
> > > > > > \ + -Wnon-virtual-dtor -Wcast-align
> > > > > > -fno-exceptions \ + -fno-check-new -fno-common"
> > > > > > + case "$enable_compile_warnings" in
> > > > > > + yes)
> > > > > > + extra_cxxflags="$extra_cxxflags \
> > > > > > + -D_XOPEN_SOURCE=600 -D_BSD_SOURCE \
> > > > > > + -W -Wno-long-long -Wundef -Wconversion
> > > > > > -Wpointer-arith \ + -Wwrite-strings
> > > > > > -Wformat-security \
> > > > > > + -Wmissing-format-attribute"
> > > > > > + ;;
> > > > > > + esac
> > > > > > + CXXFLAGS="$extra_cxxflags $CXXFLAGS"
> > > > > > +fi
> > > > > >
> > > > > >
> > > > > > 1. In the 'yes' case, you end up with both -Wno-write-strings
> > > > > > and -Wwrite-strings, it is at least not obvious which one the
> > > > > > compiler will pick.
> > > > >
> > > > > The last of course.
> > > >
> > > > I disagree with that. It will result in people sending patches to
> > > > move all char * to const char* which adds nothing but noise to the
> > > > git history.
> > >
> > > -Write-strings is in the new "yes", which is the equivalent of the
> > > current "kde", so nothing changes for the default configuration.
> >
> > True, but the current "kde" is something noone uses, if you name it
> "enable
> > warnings" everyone will think it is our default "warnings" setting.
>
> Since it seems noone else cares i'd suggest you go my "simple" way with
> only
> one set of warnings, if we ever need more we can reintroduce the other
> "yes"
> warnings.
>
> Albert
>
> >
> > Albert
> >
> > > > I'd just scrap the "extra warnings" setting and by default enable the
> > > > ones we think make sense and that's it.
> > >
> > > That may be another idea -- other opinions?
> >
> > _______________________________________________
> > poppler mailing list
> > poppler at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/poppler
>
>
> ------------------------------
>
> Message: 2
> Date: Sun, 20 Mar 2011 16:16:26 +0100
> From: Carlos Garcia Campos <carlosgc at gnome.org>
> Subject: Re: [poppler] glib/poppler-annot.cc glib/poppler-annot.h
> poppler/Annot.cc poppler/Annot.h
> To: aacid <aacid at kemper.freedesktop.org>
> Cc: poppler <poppler at lists.freedesktop.org>
> Message-ID: <1300629799-sup-5028 at charizard>
> Content-Type: text/plain; charset="utf8"
>
> Excerpts from aacid's message of jue mar 17 21:10:41 +0100 2011:
> > glib/poppler-annot.cc | 33 ++++
> > glib/poppler-annot.h | 5
> > poppler/Annot.cc | 377
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> > poppler/Annot.h | 2
> > 4 files changed, 417 insertions(+)
> >
> > New commits:
> > commit 66575c990f379871e4b796befc899de178332670
> > Author: Srinivas Adicherla <srinivas.adicherla at gmail.com>
> > Date: Thu Mar 17 20:14:05 2011 +0000
> >
> > Patch for embedding videos in to the pdf
> >
> > diff --git a/glib/poppler-annot.cc b/glib/poppler-annot.cc
> > index 62a0879..c714288 100644
> > --- a/glib/poppler-annot.cc
> > +++ b/glib/poppler-annot.cc
> > @@ -332,6 +332,39 @@ _poppler_annot_screen_new (Annot *annot)
> > return poppler_annot;
> > }
>
> I'm sorry I didn't find the time to review previous patches and I was
> on vacation when Albert asked for approval before commiting it. There
> are some problems with the patch, I'll comment inline.
>
> > +/**
> > + * poppler_annot_screen_new:
> > + * @doc: a #PopplerDocument
> > + * @rect: a #PopplerRectangle
> > + * @video_file: pass the path of the video to be embed
> > + * @mimetype: pass the mimetype of the video to put in the Media Clip
> Dictionary
> > + * @img_file: pass png/jpeg image file for poster
> > + *
> > + * Creates a new Screen annotation that will be
> > + * located on @rect when added to a page. See
> > + * poppler_page_add_annot()
> > + *
> > + * Return value: A newly created #PopplerAnnotScreen annotation
> > + *
> > + * Since: 0.18
> > + */
> > +PopplerAnnot *
> > +poppler_annot_screen_new (PopplerDocument *doc,
> > + PopplerRectangle *rect,
> > + const char* video_file,
> > + const char* mimetype,
> > + const char* img_file)
>
> A screen annotation can be used to trigger any action, not only
> rendition actions. This should be just
>
> PopplerAnnot *
> poppler_annot_screen_new (PopplerDocument *doc, PopplerRectangle
> *rect);
>
> And another method to set the action
> poppler_annot_screen_set_action(), since the activation action is not
> required in a screen annotation.
>
> > +{
> > + AnnotScreen *annot;
> > + PDFRectangle pdf_rect(rect->x1, rect->y1,
> > + rect->x2, rect->y2);
> > +
> > + annot = new AnnotScreen (doc->doc->getXRef(), &pdf_rect,
> doc->doc->getCatalog());
> > + if (annot->setAction(video_file, mimetype, img_file))
> > + return _poppler_annot_screen_new ((Annot*)annot);
> > + else
> > + return NULL;
>
> This function should never return NULL, since the action is optional
> there could be a screen annot without an action.
>
> > +
> > +GBool AnnotScreen::setAction(const char *video_file, const char
> *mimetype, const char *img_file) {
>
> There's a AnnotScreen::getAction() method that returns the LinkAction
> associated with the annotation, but this method doesn't update the
> current annotation action. Since the action might be of any type, I
> think this should be
>
> void AnnotScreen::setAction(LinkAction *action)
>
> if the annotation already had an action it will be replaced by the new
> one and the annot dictionary will be updated using Annot::update()
>
> > + if (!video_file) {
> > + error(-1, "Need to pass the video file");
> > + return gFalse;
> > + }
> > +
> > + FILE *fs;
> > + unsigned int size = 0;
> > + if (!(fs = fopen(video_file, "rb"))) {
> > + error(-1, "Couldn't open video file '%s'", video_file);
> > + return gFalse;
> > + }
> > + fseek(fs, 0, SEEK_END);
> > + size = ftell(fs);
> > +
> > + // Extract the video name from the file uri
> > + const char *video_name = strrchr(video_file, '/');
> > + if (video_name) {
> > + video_name++;
> > + } else {
> > + video_name = video_file;
> > + }
> > +
> > + Object obj1, obj2, obj3, obj4;
> > +
> > + annotObj.dictSet("T", obj1.initString(new GooString(video_name))); //
> title
>
> This should be another method AnnotScreen::setTitle() instead of
> always using the vodeo name. Since this is optional it's not a problem
> if the entry is not present unless the user explicitely calls setTitle()
>
> > + annotObj.dictSet("Contents", obj2.initString(new
> GooString(video_name))); // alternate text to be dispalyed for the
> > annotation
>
> The same here, we already have Annot::setContents(), use it to make sure
> the contents are correctly encoded and that the contents variable is
> correctly updated.
>
> > + annotObj.dictSet("F", obj3.initInt(4)); // No Zoom
>
> 4 is the bit position within the flag word, not the value itself. The
> spec doesn't say
>
> > + Object formXObj;
> > + formXObj.initDict(xref);
> > +
> > + formXObj.dictSet("Type", obj1.initName("XObject"));
> > + formXObj.dictSet("Subtype", obj2.initName("Form"));
> > +
> > + Object bbox;
> > + bbox.initArray(xref);
> > + bbox.arrayAdd(obj1.initReal(0.0000));
> > + bbox.arrayAdd(obj2.initReal(0.0000));
> > + bbox.arrayAdd(obj3.initReal(1.0000));
> > + bbox.arrayAdd(obj4.initReal(1.0000));
> > + formXObj.dictSet("BBox", &bbox);
>
> why is BBox 0, 0, 1, 1, shouldn't you use the annotation rectangle?
> something like
>
> bbox[0] = bbox[1] = 0;
> bbox[2] = rect->x2 - rect->x1;
> bbox[3] = rect->y2 - rect->y1;
>
> not that we already have Annot::createForm() that might help here
> too.
>
> > + Ref imgRef;
> > + Object imgXObj;
> > + imgXObj.initDict(xref);
> > + imgXObj.dictSet("Type", obj1.initName("XObject"));
> > + imgXObj.dictSet("Subtype", obj2.initName("Image"));
> > +
> > + if (img_file) { //show the poster, else do not show anything
> > + FILE *imgfp;
> > + if (!(imgfp = fopen(img_file, "rb"))) {
> > + error(-1, "Couldn't open file: %s", img_file);
> > + return gFalse;
> > + }
> > + unsigned char h[10];
> > + fread(h, 1, 10, imgfp);
> > + fseek(imgfp, 0, SEEK_SET);
> > +
> > + MemStream *imgStream = NULL;
> > + // Load the stream from the png file
> > + if (h[0] == 0x89 && h[1] == 0x50 && h[2] == 0x4E && h[3] == 0x47) {
> > + imgStream = load_from_png (imgfp, &imgXObj);
> > + // Load the stream from the jpeg file
> > + } else if(h[0] == 0xFF && h[1] == 0xD8 && h[6] == 0x4A && h[7] ==
> 0x46 && h[8] == 0x49 && h[9] == 0x46) {
> > + imgStream = load_from_jpeg (imgfp, &imgXObj);
> > + } else {
> > + error(-1, "Image format cannot be supported, only png/jpeg\n");
> > + return gFalse;
> > + }
> > +
> > + if (!imgStream) {
> > + obj1.free();
> > + obj2.free();
> > + obj3.free();
> > + obj4.free();
> > + bbox.free();
> > + imgXObj.free();
> > + return gFalse;
>
> the AP entry is optional, you can have a screen annotation with a
> rendition action associated that doesn't have an appearance stream, so
> I don't think we should return here.
>
> > + }
> > +
> > + obj1.initStream(imgStream);
> > + imgRef = xref->addIndirectObject(&obj1);
> > +
> > + obj2.initDict(xref); // Image XObject
> > + obj2.dictSet("Im1", obj3.initRef(imgRef.num, imgRef.gen));
> > + obj4.initDict(xref);
> > + obj4.dictSet("XObject", &obj2);
> > +
> > + formXObj.dictSet("Resources", &obj4);
> > +
> > + GooString *newString = new GooString();
> > + newString->append("/Im1 Do");
> > + formXObj.dictSet("Length", obj1.initInt(newString->getLength()));
> > +
> > + MemStream *fstream = new
> MemStream(copyString(newString->getCString()), 0, newString->getLength(),
> &formXObj);
> > + fstream->setNeedFree(gTrue);
> > + delete(newString);
> > +
> > + obj1.initStream(fstream);
> > + Ref appRef = xref->addIndirectObject(&obj1);
> > +
> > + obj2.initDict(xref);
> > + obj2.dictSet("N", obj3.initRef(appRef.num, appRef.gen));
> > + obj2.dictSet("R", obj3.initRef(appRef.num, appRef.gen));
>
> The spec says:
>
> "The AP entry refers to an appearance dictionary (see Table 168) whose
> normal appearance provides the
> visual appearance for a screen annotation that shall be used for
> printing and default display when a media
> clip is not being played. If AP is not present, the screen annotation
> shall not have a default visual
> appearance and shall not be printed."
>
> it talks about the normal appearance, I don't think we need to set
> the rollover appearance too.
>
> > + annotObj.dictSet("AP", &obj2);
>
> I think this should be moved to a generic method
> Annot::setAppearanceFromImage(const char *imageFile);
> or something like that.
>
> > + formXObj.initDict(xref);
> > + formXObj.dictSet("Type", obj3.initName("XObject"));
> > + formXObj.dictSet("Subtype", obj4.initName("Form"));
> > +
> > + bbox.initArray(xref);
> > + bbox.arrayAdd(obj1.initReal(0.0000));
> > + bbox.arrayAdd(obj2.initReal(0.0000));
> > + bbox.arrayAdd(obj3.initReal(1.0000));
> > + bbox.arrayAdd(obj4.initReal(1.0000));
> > + formXObj.dictSet("BBox", &bbox);
>
> Why 0, 0, 1, 1 here again?
>
> > + obj2.initDict(xref);
> > + obj2.dictSet("Im1", obj3.initRef(imgRef.num, imgRef.gen));
> > + obj1.initDict(xref);
> > + obj1.dictSet("XObject", &obj2);
> > + formXObj.dictSet("Resources", &obj1);
> > +
> > + newString = new GooString();
> > + newString->append("/Im1 Do");
> > + formXObj.dictSet("Length", obj1.initInt(newString->getLength()));
> > +
> > + fstream = new MemStream(copyString(newString->getCString()), 0,
> newString->getLength(), &formXObj);
> > + fstream->setNeedFree(gTrue);
> > + delete(newString);
> > +
> > + obj1.initStream(fstream);
> > + appRef = xref->addIndirectObject(&obj1);
> > +
> > + obj2.initDict(xref); // MK dictionary
> > + obj2.dictSet("I", obj3.initRef(appRef.num, appRef.gen));
> > + annotObj.dictSet("MK", &obj2);
>
> There's a AnnotScreen::getAppearCharacs() method that should refelct
> this change too
>
> > + }
> > +
> > + // Rendition Action to be add
> > + Object actionDict;
> > + actionDict.initDict(xref);
> > + actionDict.dictSet("Type", obj1.initName("Action"));
> > + actionDict.dictSet("S", obj2.initName("Rendition"));
> > + actionDict.dictSet("OP", obj3.initInt(0));
>
> OP = 0 means: play the rendition specified by R, associating it with
> the annotation specified by AN, which makes AN a required field. AN
> should be an indirect reference to the screen annotation.
>
> I think we could add a new constructor to LinkRendition() that takes
> the required fields of a rendition action like we currenlty do with
> annotations. Then this method would simply take the action object and
> update the A entry with the indirect reference to the action.
>
> > + // Media Rendition
> > + Object MRendition;
> > + MRendition.initDict(xref);
> > + MRendition.dictSet("S", obj1.initName("MR"));
> > + MRendition.dictSet("N", obj2.initString(new GooString(video_name)));
>
> Same here, we could have a new constructor for MediaRendition that
> creates the new media rendition object.
>
> > + // Media Clip Dictionary
> > + Object MClipDict;
> > + MClipDict.initDict(xref);
> > + MClipDict.dictSet("S", obj1.initName("MCD"));
> > + MClipDict.dictSet("N", obj2.initString(new GooString(video_name)));
> > +
> > + if (mimetype) {
> > + MClipDict.dictSet("CT", obj3.initString(new GooString(mimetype)));
> > + }
> > +
> > + obj4.initString(new GooString("TEMPACCESS"));
> > + obj1.initDict(xref);
> > + obj1.dictSet("TF", &obj4);
> > + MClipDict.dictSet("P", &obj1);
> > +
> > + // File Specification Dictionary
> > + Object fsDict;
> > + fsDict.initDict(xref);
> > + fsDict.dictSet("Type", obj1.initName("Filespec"));
> > + fsDict.dictSet("F", obj2.initName((char*)video_name));
> > + fsDict.dictSet("UF", obj3.initName((char*)video_name));
>
> The UF entry should be encoded using PDFDocEncoding or UTF-16BE with a
> leading byte-order marker (as defined in 7.9.2.2, "Text String Type").
>
> > +
> > + obj3.initDict(xref); // file stream eictionary
> > + obj3.dictSet("DL", obj1.initInt(size));
> > + obj3.dictSet("Length", obj2.initInt(size));
> > +
> > + FileStream *stream = new FileStream(fs, 0, 0, size, &obj3);
> > + obj1.initStream(stream);
> > +
> > + Ref newRef;
> > + newRef = xref->addIndirectObject(&obj1);
> > + obj2.initRef(newRef.num, newRef.gen);
> > + obj1.initDict(xref);
> > + obj1.dictSet("F", &obj2);
> > + fsDict.dictSet("EF", &obj1);
>
> This code could be useful for other annots, like FileAttachment
> annotations, so I think it could be moved to a protected method in
> Annot, something like
>
> void Annot::createFilespec(const char *filename);
>
> > + newRef = xref->addIndirectObject(&fsDict);
> > + obj1.initRef(newRef.num, newRef.gen);
> > + MClipDict.dictSet("D", &obj1);
> > +
> > + newRef = xref->addIndirectObject(&MClipDict);
> > + obj1.initRef(newRef.num, newRef.gen);
> > + MRendition.dictSet("C", &obj1);
> > +
> > + newRef = xref->addIndirectObject(&MRendition);
> > + obj1.initRef(newRef.num, newRef.gen);
> > + actionDict.dictSet("R", &obj1);
> > +
> > + newRef = xref->addIndirectObject(&actionDict);
> > + obj1.initRef(newRef.num, newRef.gen);
> > + annotObj.dictSet("A", &obj1);
> > +
> > + return gTrue;
> > +}
> > +
>
> Thank you very much for the patch, and sorry again for not reviewing
> it before.
>
> Regards,
> --
> Carlos Garcia Campos
> PGP key: http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x523E6462
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: signature.asc
> Type: application/pgp-signature
> Size: 198 bytes
> Desc: not available
> URL: <
> http://lists.freedesktop.org/archives/poppler/attachments/20110320/5c61136e/attachment-0001.pgp
> >
>
> ------------------------------
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>
> End of poppler Digest, Vol 73, Issue 27
> ***************************************
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20110323/806602e4/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: srinivas_video_embedd.patch
Type: text/x-patch
Size: 18100 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20110323/806602e4/attachment-0001.bin>
More information about the poppler
mailing list