[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 #8 from Allemand Corentin <corentin.allemand at aquafadas.com> ---




(In reply to comment #7)
> 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.


Changes in this patch are designed to transform PDF to HTML but keeping the
structure of the PDF to find it in the HTML.
In the end, I have all gathered in the same patch


> 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?

I made ​​the change in the file "FileSpec.cc" because of all the test files I
used the old method to read the filename does not work.
Looking at the struture of PDF, I noticed that the tag 'F' was still present.


> In AnnotRichMediaInstance:
>  * you have the name variable that is never used

Clean

>  * type and filespec are not set to NULL if the objects are not present

Change Done

>  * 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

Change Done

>  * Please make subtype be an enum and not an int

Change Done

> 
> In AnnotRichMediaConfiguration:
>  * You never free the instances list

Change Done

>  * Same thing for the subtype enum thing

Change Done

>  * Same thing for the strduping of char* you get from a getName

Change Done

> 
> 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/20121120/c86ac6a1/attachment.html>