[Poppler-bugs] [Bug 56843] [PATCH] Feature : extract media in html format

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=56843

--- Comment #7 from Albert Astals Cid <tsdgeos at terra.es> ---
First of all thanks for the contribution, don't let the comments make you think
your contribution is unwanted, it's just that we try to get the better code
possible for all the features.

The patches still seem a big mixed
https://bugs.freedesktop.org/attachment.cgi?id=70245 and
https://bugs.freedesktop.org/attachment.cgi?id=70246 both seem to have the same
first "patch" inside.

Anyway, could you explain in text what the patch does? I can read the code but
i'd like something longer than the first comment, something like, it saves this
there and bla bla.

Also it would be cool if you could send the patches with the least number of
whitespace changes, it takes a while to have a look at 
-    HtmlImage(GooString *_fName, GfxState *state)
+  HtmlImage(GooString *_fName, GfxState *state)
or
-      Object obj1;
-      // valid 'F' key -> external file
-      kind = soundExternal;
-      if (getFileSpecNameForPlatform (&tmp, &obj1)) {
-        fileName = obj1.getString()->copy();
-        obj1.free();
-      }
+        Object obj1;
+        // valid 'F' key -> external file
+        kind = soundExternal;
+        if (getFileSpecNameForPlatform (&tmp, &obj1)) {
+          fileName = obj1.getString()->copy();
+          obj1.free();
+        }
and realize the only thing that has changed is the leading space, so it helps
making the review faster if these kind of change is not present

Also can you explain the code change in poppler/FileSpec.cc?

In AnnotRichMediaInstance:
 * you have the name variable that is never used
 * type and filespec are not set to NULL if the objects are not present
 * You want to strdup or create a goostring from type since once the Object
runs out of references you'll have a pointer pointing to free'd memory
 * Please make subtype be an enum and not an int

In AnnotRichMediaConfiguration:
 * You never free the instances list
 * Same thing for the subtype enum thing
 * Same thing for the strduping of char* you get from a getName

Please fix these mistakes and post the new patches and i'll continue with the
review.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20121119/656a9014/attachment.html>