[Poppler-bugs] [Bug 47282] Added media rendition support for Qt4

[bugzilla-daemon at freedesktop.org]

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

--- Comment #8 from Guillermo A. Amaral <g at maral.me> 2012-03-13 17:39:25 UTC ---
(In reply to comment #6)

I would like to point out that I tried to stick as close as possible to current
implementations, though I did also add a few of my usual blunders to keep
things fresh. Enjoy!

> What does the loop in ScreenAnnotation::ScreenAnnotation( const QDomNode & node
> ) do?
> 
> What's the point in creating an empty screen elment in ScreenAnnotation::store?

  *Removed* (appeared in MovieAnnotation, left it in as a place holder)

>  \since 0.10 needs to be  \since 0.20

  *Replaced* (woops)

> Please pass the MediaRendition to LinkRenditionPrivate

  *Passed* (though it appeared as assignment in other existing implementations)

> poppler-media.h: qt interface to poppler in poppler-media.cc

  *Fixed*

> MediaRendition has no documentation at all

  *Documented*

> MediaRendition misses some const in the getters (isValid, contentType, etc)

  Sadly the original (wrapped) class didn't declare it's functions as constant,
and I was a bit reluctant to change the original rendition class. Anyway, it's
been done.

  *Constant-ed*

> MediaRenditionPrivate *d_ptr; is duplicating what Q_DECLARE_PRIVATE(
> MediaRendition ) does?

  Again, noticed that some already existing implementation where using
Q_DECLARE_PRIVATE in order to use Q_D in member functions.
  As you know, d_ptr is not declared by the macro, so I had to add it manually.

#define Q_DECLARE_PRIVATE(Class) \                                              
inline Class##Private* d_func() { return reinterpret_cast<Class##Private
*>(qGetPtrHelper(d_ptr)); } \
inline const Class##Private* d_func() const { return reinterpret_cast<const
Class##Private *>(qGetPtrHelper(d_ptr)); } \
friend class Class##Private;

#define Q_D(Class) Class##Private * const d = d_func()

  *Answered*

> Please use UnicodeParsedString instead of QString::fromLatin1

  *Replaced* (remnant from MovieAnnotation's current implementation)

> Please d-pointify ScreenObject

  *PIMPL'd*

> poppler-screen.cc says poppler-sound.cc: qt interface to poppler

  *Replaced*

> What's the use of passing AnnotScreen *ann to ScreenObject::ScreenObject

  *Removed* (sorry, left over from a previous attempt)

> What's the use of ScreenObject itself if only holds a LinkRendition ?

  For consistency, anyway the class is (was) no more heavy than a private
class, not to mention it might come in handy later.

  * Consistency*

> Q_DISABLE_COPY to StreamDevice ?

  *Added*

> StreamDevice seems like it could to more to implement QIODevice functions like
> reset, etc, i understand you decided to say its isSequential to simplify the
> job, maybe you should rename StreamDevice to StreamSequentialDevice?

  Well, not quite, I first implemented it as a full blown non-sequential
stream, but then I noticed that the Stream object getting sent to the wrapper
was a FilterStream, so seeking was out of the question.

  *Renamed* (also added close for kicks)
StreamSequentialDevice

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.