ogg dev - Apr 2008 - [PATCH] browser_plugin - kate support, build fixes, and misc

> I've tried to separate out the general bugfixes from the new
> implementation work in this patch:
Sorry, I should have done that, I do tend to fix stuff I find while
coding and neglect splitting up afterwards. Bad me.
> I've not yet committed this to trunk. It does sound pretty cool
> though. I don't see any problem with adding pango etc. support,
it's
> pretty much where we wanted to go for CMML handling anyway.
Ah, good. At the moment, it's still imlib2 only though. Not great
but will do for now. Cairo/Pango will be needed if/when I add my
rendering lib, but I'm just starting on it, so imlib2 it will be for the
near future.

If you have any comments/concerns about the Kate code, feel free
to comment. Especially on the Javascript API side (never done any
of this before).
> Let me know if this works for you or not; eg. it may be necessary to
> also put X_EXTRA_LIBS etc. in src/Makefile.am.
will try this.
> >   - bad pointer dereference fix (mouse callback)
>
> I couldn't find that particular hunk, could you please provide a patch
> which just fixes this?
Two hunks, the first one to initialize the mouse callback pointer
to NULL, and the second one to test for NULL before actually using it:

--- svn/annodex/browser_plugin/src/plugin.cpp   2008-04-04
15:14:28.000000000 +0100
+++ svn/annodex/browser_plugin-with-kate/src/plugin.cpp 2008-04-27
22:08:08.000000000 +0100
@@ -245,7 +244,8 @@
   mCmmlCallback(NULL),
   mAsyncCmmlCallback(NULL),
   mEndPlayCallback(NULL),
-  mPlaylistCallback(NULL)
+  mPlaylistCallback(NULL),
+  mMouseClickCallback(NULL)
 #if defined(XP_MACOSX)
   ,
   mOutputCleared(FALSE)
@@ -1037,7 +1175,9 @@

   if (mMouseButtonDown) {
     mMouseButtonDown = FALSE;
-    mMouseClickCallback->Call();
+    if (this->mMouseClickCallback != NULL) {
+      mMouseClickCallback->Call();
+    }
   }

   if (useSemaphore) {

> >   - avoid the input widget in test.html firing hotkey'd functions
>
> is this specific to the Kate changes? I noticed that it adds a focus
> tracking variable, but that seems to be used by some of the Kate UI
> also so I'm not sure if it's a general bugfix or Kate-specific.
Well, not really, more like only useful for the new kate test functions:
the keypress routine would be called for every keypress, but all
existing test routines would expect numbers in the input box, and no
numbers were hotkeys for any function, so nothing would get done.
Since I added a function (to see the font to use) that expects input
with other characters than numbers, some test functions would be
invoked.

However, looking at the patch, the input_focus variable isn't used
by the Kate UI. Were you talking about something else ?

Thanks for your time.

2008/4/29 ogg.k.ogg.k at googlemail.com <ogg.k.ogg.k at
googlemail.com>:
>  > >   - bad pointer dereference fix (mouse callback)
>  >
>  > I couldn't find that particular hunk, could you please provide a
patch
>  > which just fixes this?
>
>  Two hunks, the first one to initialize the mouse callback pointer
>  to NULL, and the second one to test for NULL before actually using it:
>
>  --- svn/annodex/browser_plugin/src/plugin.cpp   2008-04-04
>  15:14:28.000000000 +0100
>  +++ svn/annodex/browser_plugin-with-kate/src/plugin.cpp 2008-04-27
>  22:08:08.000000000 +0100
>  @@ -245,7 +244,8 @@
>    mCmmlCallback(NULL),
>    mAsyncCmmlCallback(NULL),
>    mEndPlayCallback(NULL),
>  -  mPlaylistCallback(NULL)
>  +  mPlaylistCallback(NULL),
>  +  mMouseClickCallback(NULL)
>   #if defined(XP_MACOSX)
>    ,
>    mOutputCleared(FALSE)
>  @@ -1037,7 +1175,9 @@
>
>    if (mMouseButtonDown) {
>      mMouseButtonDown = FALSE;
>  -    mMouseClickCallback->Call();
>  +    if (this->mMouseClickCallback != NULL) {
>  +      mMouseClickCallback->Call();
>  +    }
>    }
>
>    if (useSemaphore) {
>
thanks, merged in changeset:3573
>  > >   - avoid the input widget in test.html firing hotkey'd
functions
>  >
>  > is this specific to the Kate changes? I noticed that it adds a focus
>  > tracking variable, but that seems to be used by some of the Kate UI
>  > also so I'm not sure if it's a general bugfix or
Kate-specific.
>
>  Well, not really, more like only useful for the new kate test functions:
>  the keypress routine would be called for every keypress, but all
>  existing test routines would expect numbers in the input box, and no
>  numbers were hotkeys for any function, so nothing would get done.
>  Since I added a function (to see the font to use) that expects input
>  with other characters than numbers, some test functions would be
>  invoked.
>
>  However, looking at the patch, the input_focus variable isn't used
>  by the Kate UI. Were you talking about something else ?
nope, I was just unsure if it was independent of  the other kate stuff.

committed in changeset:3574

Conrad.

> > Let me know if this works for you or not; eg. it may be necessary to
> > also put X_EXTRA_LIBS etc. in src/Makefile.am.
>
> will try this.
It worked fine for me, with no need to add anything to Makefile.am.
> > >   - bad pointer dereference fix (mouse callback)
> >
> > I couldn't find that particular hunk, could you please provide a
patch
> > which just fixes this?
>
> Two hunks, the first one to initialize the mouse callback pointer
> to NULL, and the second one to test for NULL before actually using it:
That didn't compile though, sorry, the attached patch fixes that. I edited
the original patch by hand to remove the Kate stuff and it seems I ended
up with a stray comma.

Also, I'd entirely forgotten about that, but I need to set FIREFOX_CFLAGS
and FIREFOX_LIBS to (resp) "-I$(ffpath)/include" and
"-L$(ffpath)/lib" for
configure to not whine it can't find Firefox. I've got the Gecko SDK,
but
I can't see any pc files in there, so the configure.ac check's got me
puzzled. Since it all builds and run if I do that, I can only guess that
the reason is that I'm using a prebuilt version (I did hear that FF is a
real pain to build, and my dev box is still having FF 1 :)). I've got:
gecko-sdk-i686-pc-linux-gnu-1.8.0.4.tar.bz2
If you decide you don't want to support building against prebuilt SDKs,
then that's fine by me, just wanted to mention it.

Next, the audio code doesn't build for me, same thing about old software.
My /usr/include/alsa/version.h says "1.0.9". Various functions are
being
undefined. I can supply the list if needed, but you may want to decide to
not support old versions.

Last, the building of the static lib fails, as it tries to pull liboggz,
liboggplay, etc, out of /usr/local/lib (well, @LIBDIR@) rather than from
wherever pkg-config says they are.

I'd omitted these chunks from my previous patch (esp. the ALSA one, which
is down to my own peculiar configuration), but that last one (which I do
not know how to fix) is likely to be annoying, as I suppose a distribution
of the plugin as a binary is going to include the static one, rather than
the shared one (just guessing here).

If you know how to fix it, feel free to tell me if there's a patch you
want me to test here.

Cheers
-------------- next part --------------
A non-text attachment was scrubbed...
Name: plugin.cpp.comma.diff
Type: text/x-patch
Size: 324 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/ogg-dev/attachments/20080430/1fc1d081/attachment.bin

2008/4/30 ogg.k.ogg.k at googlemail.com <ogg.k.ogg.k at
googlemail.com>:
> > > Let me know if this works for you or not; eg. it may be necessary
to
>  > > also put X_EXTRA_LIBS etc. in src/Makefile.am.
>  >
>  > will try this.
>
>  It worked fine for me, with no need to add anything to Makefile.am.
>
good
>  > > >   - bad pointer dereference fix (mouse callback)
>  > >
>  > > I couldn't find that particular hunk, could you please
provide a patch
>  > > which just fixes this?
>  >
>  > Two hunks, the first one to initialize the mouse callback pointer
>  > to NULL, and the second one to test for NULL before actually using
it:
>
>  That didn't compile though, sorry, the attached patch fixes that. I
edited
>  the original patch by hand to remove the Kate stuff and it seems I ended
>  up with a stray comma.
sorry, that was my fault with the merge; I fixed it a few hours later in r3576
>  Also, I'd entirely forgotten about that, but I need to set
FIREFOX_CFLAGS
>  and FIREFOX_LIBS to (resp) "-I$(ffpath)/include" and
"-L$(ffpath)/lib" for
>  configure to not whine it can't find Firefox. I've got the Gecko
SDK, but
>  I can't see any pc files in there, so the configure.ac check's got
me
>  puzzled. Since it all builds and run if I do that, I can only guess that
>  the reason is that I'm using a prebuilt version (I did hear that FF is
a
>  real pain to build, and my dev box is still having FF 1 :)). I've got:
>  gecko-sdk-i686-pc-linux-gnu-1.8.0.4.tar.bz2
>  If you decide you don't want to support building against prebuilt
SDKs,
>  then that's fine by me, just wanted to mention it.
hmm, perhaps we should add some configure arguments like
--with-firefox-cflags= and --with-firefox-libs
>  Next, the audio code doesn't build for me, same thing about old
software.
>  My /usr/include/alsa/version.h says "1.0.9". Various functions
are being
>  undefined. I can supply the list if needed, but you may want to decide to
>  not support old versions.
>
>  Last, the building of the static lib fails, as it tries to pull liboggz,
>  liboggplay, etc, out of /usr/local/lib (well, @LIBDIR@) rather than from
>  wherever pkg-config says they are.
ok, what architecture are you on? that's been reported on x86_64 but
may occur elsewhere
>  I'd omitted these chunks from my previous patch (esp. the ALSA one,
which
>  is down to my own peculiar configuration), but that last one (which I do
>  not know how to fix) is likely to be annoying, as I suppose a distribution
>  of the plugin as a binary is going to include the static one, rather than
>  the shared one (just guessing here).
>
>  If you know how to fix it, feel free to tell me if there's a patch you
>  want me to test here.
will do, but we don't have a patch for that yet :-)

cheers,

Conrad.