[poppler] glib/poppler-annot.cc glib/poppler-annot.h poppler/Annot.cc poppler/Annot.h
Carlos Garcia Campos
carlosgc at gnome.org
Sun Mar 20 08:16:26 PDT 2011
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.pgp>
More information about the poppler
mailing list