HydrogenAudio

Hosted Forums => foobar2000 => Development - (fb2k) => Topic started by: danZ on 2003-06-02 18:20:48

Title: ICC
Post by: danZ on 2003-06-02 18:20:48
I like to start a discussion of implementing some inter component communication framework.  If foobar already allows this then please enlighten me.

Otherwise, what I'd be interested in is a way to utilize other installed components.

For example,

CuriOus_George as a nice one foo_playlistfind I believe it is called.

Aero just released a first pass of foo_bookmark.

I'd like to be able to use both of these directly from foo_wsgui (even though I seem to unknowingly crash foo_bookbark  ) via some programming interface.

1.  For playlist find I'd like to be able to catch keystrokes myself then call "search" with the string.  Playlist find would do its thing.
2.  For bookmark I'd like to be able to add a bookmark directly from foo_wsgui.

So, the point is I'd like to have a way that all of these components could work together.

The only way I can think to do that now is with some hotkeys assigned to the actions then passing those to foobar when pressed in wsgui.  Even that only really works for case 2.  Since I'm doing a front end engine something more robust would be useful.

Ideas anyone?
Title: ICC
Post by: Curi0us_George on 2003-06-02 18:53:04
Well, 3rd party plugin creators can create services and register them.  Other plugins can then enumerate these services.  foo_history does just that.

P.S.  There's no way to "tap into" the playlist searching, though, so far as I know.
Title: ICC
Post by: danZ on 2003-06-02 19:02:53
Quote
Well, 3rd party plugin creators can create services and register them.  Other plugins can then enumerate these services.  foo_history does just that.

P.S.  There's no way to "tap into" the playlist searching, though, so far as I know.

I wanted to tap in to your searching  - is foo_playlistfind a service?

How does one enumerate the services available and the methods they support?
Title: ICC
Post by: Curi0us_George on 2003-06-02 19:18:55
I believe the appropriate method is service_factory_base::create_enum().  I don't have the SDK handy.  But basically, you can enumerate anything.  You can enumerate the contextmenu items.  You can enumerate the components menu items, etc.  To tap into anything more than the SDK provides (e.g. if I created some nifty searching plugin with great service capabilities), you would need some sort of API (e.g. I would need to provide an API for my DLL, and register the appropriate service).

How exactly did you want to tap into foo_playlistfind?  You should be able to pop up the window (by manually calling the components menu item), I believe, but I'm not sure if that's what you're wanting.
Title: ICC
Post by: danZ on 2003-06-02 19:28:37
Quote
I believe the appropriate method is service_factory_base::create_enum().  I don't have the SDK handy.  But basically, you can enumerate anything.  You can enumerate the contextmenu items.  You can enumerate the components menu items, etc.  To tap into anything more than the SDK provides (e.g. if I created some nifty searching plugin with great service capabilities), you would need some sort of API (e.g. I would need to provide an API for my DLL, and register the appropriate service).

How exactly did you want to tap into foo_playlistfind?  You should be able to pop up the window (by manually calling the components menu item), I believe, but I'm not sure if that's what you're wanting.

Thanks for the info.

For foo_playlistfind I'd might like to provide an input box in wsgui and just pass the search string to foo_playlistfind.  ie. bypass your edit box window but still get the same outcome (the first match is highlighted in the playlist).  Your plugin is a good candidate for a service since your UI isn't really needed wrt to foo_wsgui, foo_ufts, or even your own foo_minibar could use it as a "service"

Same idea with a book mark plugin.  I might want to provide a button on wsgui that adds the current song to the bookmarks without using the foo_bookmark UI.
Title: ICC
Post by: Curi0us_George on 2003-06-02 19:34:55
I'm supposed to be churning out a new version of foo_minibar soon.  (I was supposed to put out a new version this past weekend, but I didn't do any work on it.)  I'll see about adding a proper service, so you can just pass your own search string, rather than using my input box.
Title: ICC
Post by: danZ on 2003-06-02 19:44:37
Quote
I'm supposed to be churning out a new version of foo_minibar soon.  (I was supposed to put out a new version this past weekend, but I didn't do any work on it.)  I'll see about adding a proper service, so you can just pass your own search string, rather than using my input box.

That would be cool as I think foo_playlistfind is a good compliment to the built in search.

Oh, speaking of that another suggestion for foo_playlistfind.

I read a post by someone asking you to make it only search on return key instead of per character.

Another option that I've used before that I think works well (and would prob. how I will implement it once you make a service  ) is to reset a timer on each key press and delay the search until the timer expires.  Fast keying keeps resetting the timer so that you don't search on the first letter typed unless you want to (ie. type a single char and nothingn else for say 250 ms or something).  At the same time you don't have to wait for a carriage return either.  This works well for backspacing too and it makes the search a little more responsive since you weed out unneeded intermediate searches.

Just an idea.
Title: ICC
Post by: Curi0us_George on 2003-06-02 19:48:41
Not a bad idea.  It could work for something like this.  (It wouldn't work very well for something like foo_shizzle, though.)
Title: ICC
Post by: foosion on 2003-06-03 22:01:13
Quote
Not a bad idea.  It could work for something like this.  (It wouldn't work very well for something like foo_shizzle, though.)

Are you refering to making the interface more responsive? Well, searching in the background surely helps - like in foo_dbsearch, though the code that synchronizes results with the gui is a bit ... messy right now. Reimplementing this with a proper model-view-controller architecture will surely help...

Given a specific plugin, providing new services is imho the best way to let other plugins use its features. Of course the interface needs to be public (e.g. available in an include file) as well as the guid (which is located in a .cpp file and must be linked into any plugins that want to use it). I suggest to use a common directory to put these files into. I already did this for foo_history (the directory is called "3rd_party" there), and foo_dbsearch will have this as well some time in the future. I plan to make the search core with all it's features available and - to some degree - the gui. Unfortunately I don't have much time at the moment to work on fb2k plugins
Title: ICC
Post by: Curi0us_George on 2003-06-04 04:07:16
That's what I plan to do (with the service interface).

I never implemented background searching in foo_shizzle because I was getting deadlocks.  I think I know now where the deadlocks were occuring, but I also lost a huge chunk of the foo_shizzle code, so I just haven't felt like messing with it.  For a plugin like playlistfind, though, searching in the background is just overkill and I refuse to mess with it.
Title: ICC
Post by: danZ on 2003-06-04 16:47:19
Can someone post a code example of getting/using a 3rd party component service (like foo_history) from another component?

EDIT

And what happens if your component is "linked" to to 3rd party service and that component is gone or doesn't load?  Also, is there a mechanism for making sure the linked interface matches the actual service interface (like if new versions come out).  Sorta like how COM does it in that you are guaranteed a certain interface regardless of versions, etc.

/EDIT
Title: ICC
Post by: Peter on 2003-06-04 17:01:11
Interface types are identified by GUIDs (see: static GUID get_class_guid() everywhere). If a third party developer publishes their custom interfaces, its their own responsibility not to change those interfaces between different versions of the component (and if they need to be changed, changing the GUID is recommended way to prevent people from getting wrong interface pointers). If particular interface isn't present, attempt to enumerate it will not return any objects.
There is code enumerating/using services all over SDK (eg. sending messages to console, opening files, opening inputs, etc), just open your eyes and read it instead of asking.
Title: ICC
Post by: danZ on 2003-06-04 23:29:17
Quote
Interface types are identified by GUIDs (see: static GUID get_class_guid() everywhere). If a third party developer publishes their custom interfaces, its their own responsibility not to change those interfaces between different versions of the component (and if they need to be changed, changing the GUID is recommended way to prevent people from getting wrong interface pointers). If particular interface isn't present, attempt to enumerate it will not return any objects.
There is code enumerating/using services all over SDK (eg. sending messages to console, opening files, opening inputs, etc), just open your eyes and read it instead of asking.

Yeah, I've seen that I just wondered if the procedure was any different for a third party component that isn't part of the SDK  - I'll figure it out.

Sorry to bother you with my questions.
Title: ICC
Post by: Peter on 2003-06-05 00:32:57
Well, if third parties provide their own service types, it's a good habit to provide some kind helper code for enumerating them too.
Otherwise, standard service_enum_t methods work everywhere (no reasons why they wouldn't), it's just that enumerating job can be simplified with some services - eg. enumerate only those object that match certain criteria, or simple get() function returning pointer to a single static instance of the object (like with metadb).
Title: ICC
Post by: Hero.Hua on 2003-06-05 02:25:11
Yeah, helpful!
Title: ICC
Post by: Curi0us_George on 2003-06-05 02:33:56
danZ, foo_playlistfind has a very simple interface now.  It should get fleshed out soon, but it can do basic playlist searching now.

There's an extremely trivial example in the "examples" folder.

http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

P.S. The stuff you need to interact with the plugin is in the Interfaces folder. 
Title: ICC
Post by: danZ on 2003-06-05 04:16:03
Quote
danZ, foo_playlistfind has a very simple interface now.  It should get fleshed out soon, but it can do basic playlist searching now.

There's an extremely trivial example in the "examples" folder.

http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

P.S. The stuff you need to interact with the plugin is in the Interfaces folder. 

Cool, I'll check it out.  Thanks.

The playlistfind-src.zip doesn't have an Interfaces folder.  It did have an Examples folder so I have a good idea how the service will work.  I better work on some kind of keyboard input field for foo_wsgui now that I don't have my keyboard handling fecked up.
Title: ICC
Post by: foosion on 2003-06-06 07:14:26
foo_history does contain an example how to use the history service - the GUI. Just have a look at foo_history.cpp, where it is implemented. history_i.cpp contains the implementation for the history service; you actually shouldn't need to worry about it.
Title: ICC
Post by: danZ on 2003-06-06 16:58:14
Quote
foo_history does contain an example how to use the history service - the GUI. Just have a look at foo_history.cpp, where it is implemented. history_i.cpp contains the implementation for the history service; you actually shouldn't need to worry about it.

So I would inlcude foo_history.h in my build and then simply enum the history service and use it?
Title: ICC
Post by: danZ on 2003-06-06 16:59:13
Quote
The playlistfind-src.zip doesn't have an Interfaces folder. It did have an Examples folder so I have a good idea how the service will work


Did you leave out some stuff from the .zip file?
Title: ICC
Post by: Curi0us_George on 2003-06-06 17:21:07
Hmm.  Apparently I did leave it out of the folder.  Hrmf.  I'll get a new bersion up pronto.  I'm fixing some bugs in foo_playlistfind right now.
Title: ICC
Post by: Curi0us_George on 2003-06-06 17:28:54
They should be in the zip now.

http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)
Title: ICC
Post by: danZ on 2003-06-06 19:24:26
Quote
They should be in the zip now.

http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

Thanks, I'll try it out - sounds like a lot of new features in there as well.  I better grab the latest binary too.
Title: ICC
Post by: Curi0us_George on 2003-06-06 19:31:33
Don't do too much work with it, though.  I'll be putting out a new version with some new interface features sometime today. 
Title: ICC
Post by: danZ on 2003-06-06 20:32:46
Weird problem.

I add the playlist interface to my build.

I'm calling the search, getting what look like correct results but the playlist isn't updating.  Basically, I can't set the focus item from my component.

I tried setting the focus from the iniquit object and that worked so I suspect some thread issue or something.

I noticed you had a

extern playlist_oper * g_oper = NULL;

which you set/release in the initquit.  Did you have a problem like what I describe?

I tried replicating the same thing

a global called from my message loop thread but it still didn't work.

my relevant code is just

      int findPos = global_playlist_find::get()->search(m_Hot[Title1].GetText(),g_play_oper->get_focus());
      g_play_oper->set_focus_sel(findPos);


findPos seems to be correct (the search is working).

EDIT

Otherwise, very simple to add and use and I think this compoent service sharing could be very powerful and beneficial overall.  Especially for a gui front end like wsgui.  Thanks for creating the interface.

/EDIT

Ideas?
Title: ICC
Post by: danZ on 2003-06-06 20:38:58
Quote
Don't do too much work with it, though.  I'll be putting out a new version with some new interface features sometime today. 

I hope that includes things like

play_focused

and some access to the things you are doing with your keyboard shortcut support, etc.
Title: ICC
Post by: danZ on 2003-06-06 21:05:11
The playlist_oper "gets" seem to work from my component but not the "sets".
Title: ICC
Post by: Curi0us_George on 2003-06-06 21:12:49
That's really odd that you're unable to set the focused item.  What exactly is it doing?  It is defocusing the currently selected, or just taking no action at all?  But I can't control what the playlist_oper does.  I'm only caching a reference to speed up searches.

Things like play_focused are done through the play_control and playlist_oper classes, so I won't be duplicating that functionality.

Most of the new functionality I'm adding is to allow searching of lists other than the playlist, and to have more control over exactly how they are searched.  I've also added code to show and close the search window, but there's no code to control the window.  It's just like pressing the hotkeys.
Title: ICC
Post by: Curi0us_George on 2003-06-06 21:20:43
0.6.b11.011

Quite a few changes to the interface.  One thing to especially note is that start_index now is included in the searches.

http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)
Title: ICC
Post by: danZ on 2003-06-06 21:30:37
Quote
That's really odd that you're unable to set the focused item.  What exactly is it doing?  It is defocusing the currently selected, or just taking no action at all?  But I can't control what the playlist_oper does.  I'm only caching a reference to speed up searches.

Things like play_focused are done through the play_control and playlist_oper classes, so I won't be duplicating that functionality.

Most of the new functionality I'm adding is to allow searching of lists other than the playlist, and to have more control over exactly how they are searched.  I've also added code to show and close the search window, but there's no code to control the window.  It's just like pressing the hotkeys.

It simply does nothing.  The focus isn't changed at all.

I'll keep messing around with it.
Title: ICC
Post by: danZ on 2003-06-06 22:25:58
Quote
0.6.b11.011

Quite a few changes to the interface.  One thing to especially note is that start_index now is included in the searches.

http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

playlist_find.cpp is not needed any longer?

If so you should remove from the interfaces folder.

Thanks.
Title: ICC
Post by: Curi0us_George on 2003-06-06 22:40:25
Damn you.    I was going to tell you that you must have unzipped over the old one so it's still there, but it was my fault.  I forgot to remove the old one from the zip. 

P.S.  Gone now.  No other changes, though.  You can just delete it from the folder.
Title: ICC
Post by: danZ on 2003-06-06 22:54:56
I included the new playlist_find.h and made no other changes but now I alwasy get -1 for the search result.

Still just calling

      int findPos = global_playlist_find::get()->search(m_Hot[Title1].GetText(),g_play_oper->get_focus());
      g_play_oper->set_focus_sel(findPos);

is there any other setup or did the search() semantics change?
Title: ICC
Post by: Curi0us_George on 2003-06-06 23:51:22
The search semantics actually did change a little (start_index is now included in the search, whereas it was previously not included), but that doesn't explain why you're getting -1 as the result every time.  By sending the selected item, it should (likely) just be returning it every time.

int findPos = global_playlist_find::get()->search(m_Hot[Title1].GetText()); // you can drop the g_play_oper->get_focus()
g_play_oper->set_focus_sel(findPos);

edit:
Can you post an example of what m_Hot[Title1].GetText() returns, along with your playlist formatting string, your playlistfind string, and any other data you think might be important.  (If you've got "match only beginning" enabled, etc.)
Title: ICC
Post by: danZ on 2003-06-07 04:39:52
Quote
The search semantics actually did change a little (start_index is now included in the search, whereas it was previously not included), but that doesn't explain why you're getting -1 as the result every time.  By sending the selected item, it should (likely) just be returning it every time.

int findPos = global_playlist_find::get()->search(m_Hot[Title1].GetText()); // you can drop the g_play_oper->get_focus()
g_play_oper->set_focus_sel(findPos);

edit:
Can you post an example of what m_Hot[Title1].GetText() returns, along with your playlist formatting string, your playlistfind string, and any other data you think might be important.  (If you've got "match only beginning" enabled, etc.)

I got it working again.  Here's how if you want to try to see what happened.

I delete playlistfind, ran foobar which cleaned out the config, reinstalled.

I think that just coping this most recent version over the one I had from this morning or yesterday, whenever it was caused some config problems.

EDIT

Actually, I can't get playlist_find to work reliably after I close foobar and restart.  Removing it, running foobar, the reinstalling seems to clear it up for the first run.  This is just via the supplied UI not from my component calling via the service ptr.

Hum, I can't reproduce that reliably either.  I have seen it not work from my code and from the default UI.  But its working now...
I should call it a night while I'm ahead.

/EDIT

now, still can't set the focused item.

I was wondering, would you consider from a design standpoint and as a test to include the focus setting in playlist find.

So something like

search(const char*, int index, bool wrap, bool updatefocus);

I'm wondering if this would work for me.  I have everythign in place to use playlist find except that I can't figure out why the focus stuff won't work from my message loop. (I still think foobar is locking me out somehow).

Also question.

You say don't derive from your playlist class yet the interface declares the methods as virtual and the class is preceded by NOVTABLE.

Just wondering about that one.
Title: ICC
Post by: Curi0us_George on 2003-06-07 07:53:43
I'm not really understanding the problems you're having.  I'm not having any trouble getting playlistfind to work after restarting foobar.  :\

Also, if your code can't set the currently focused item as a result of threading issues, mine won't be able to either, because it'll be getting called from your thread.  It will run under your thread, not the main thread.

So search(const char*, int index, bool wrap, bool updatefocus); would be functionally identical to search(const char*, int index, bool wrap); followed by a call to playlist_oper::get()->set_focus_sel().

If you can work your code problems down into a small sample, I might be able to take a look at it.  I don't know if I'll be able to tell you anything, though.

As for not deriving from the interface, that message is for you ("you" meaning all other developers).  If you derive from the interface, you will cause problems.  There should be exactly one derivation of the interface, and my plugin provides it.
Title: ICC
Post by: Curi0us_George on 2003-06-07 08:07:59
Here's the source of some of your problems:

from play_control.h:
//important: you should call api commands declared in this header ONLY from main thread !!

I've got no clue how you can work around that, but then, I don't know the internal structure of wsgui, so I've got no clue why you're even using separate threads.
Title: ICC
Post by: danZ on 2003-06-07 15:22:09
Quote
Here's the source of some of your problems:

from play_control.h:
//important: you should call api commands declared in this header ONLY from main thread !!

I've got no clue how you can work around that, but then, I don't know the internal structure of wsgui, so I've got no clue why you're even using separate threads.

Well, that explains a lot.  This is my first attempt at using WTL (Windows Template Library) and I was under the mistaken impression that I needed something like this to drive my window.

Code: [Select]
    MSG msg;
    while (sRunning && GetMessage ( &msg, NULL, 0, 0 ) > 0 )
    {
 TranslateMessage ( &msg );
 DispatchMessage ( &msg );
    }


The only way to do that was with a thread and I was creating a thread to run that.

The above is not needed at all - I didn't realize how the message loop worked in general I guess.  My window still gets messages without the above and this happens in the context of the "main thread".

So, I removed the threaded message pump and voila I can set the focused item

Now I'm still having problems with playlist_find not working but I can focus on that now and try to get you some better information.

One thing I noticed is when it stops working I get access violations in foobar every time I type a key in the playlist_find edit box.

Quote
First-chance exception in foobar2000.exe: 0xC0000005: Access Violation.


I also noticed I get
Quote
metadb_handle leaks, 22 objects


when I close foobar if I have run playlist_find but not if I haven't run playlist_find.  I have 22 items in my playlist.

This might be related to

Code: [Select]
    bool do_matching(const char * str, int playlist_index) {
 static string8 out;
 out.reset();
 if (search_list)
     search_list->get_item(playlist_index)->handle_format_title(out,get_search_format(),NULL);
 else
     g_oper->get_item(playlist_index)->handle_format_title(out,get_search_format(),NULL);


but I'm not sure (ie. get_item returns a ref'ed handle? ).
Title: ICC
Post by: danZ on 2003-06-07 18:04:15
OK, some progress but no explanations.  Everything seems to work but only in a release build.  If I run my component in the debugger playlist_find does not work.

In the release build it seems fine and working well  You can type a search directly into wsgui, it will pop up the foobar main window and set the focused item to the current playlist_find search hit.

I'll have to try another service and see if I have the same experience.
Title: ICC
Post by: Curi0us_George on 2003-06-07 21:58:28
I'll look into it some more.
Title: ICC
Post by: Curi0us_George on 2003-06-07 22:58:16
Hmm.  When I build playlistfind as a debug, I get some problems.  I'm trying to figure out what's going on.
Title: ICC
Post by: danZ on 2003-06-07 23:27:28
Quote
Hmm.  When I build playlistfind as a debug, I get some problems.  I'm trying to figure out what's going on.

If it would help I can post my working version of wsgui that support playlist find and you can debug playlist find when wsgui is generating the calls into the service.
Title: ICC
Post by: danZ on 2003-06-07 23:51:29
BTW

Quote
As for not deriving from the interface, that message is for you ("you" meaning all other developers). If you derive from the interface, you will cause problems. There should be exactly one derivation of the interface, and my plugin provides it.


I understand that I was just wondering why the methods were declared as virtual.  I'm just making sure I understand how the service set up works in case I ever want to make one.
Title: ICC
Post by: Curi0us_George on 2003-06-07 23:58:10
Right now I don't need you to post your code.  I've got some other odd things I need to track down first.  Maybe later, though.

The interface methods must be declared as virtual, or it wouldn't be possible to override them in a subclass.
Title: ICC
Post by: danZ on 2003-06-08 00:15:22
Quote
Right now I don't need you to post your code.  I've got some other odd things I need to track down first.  Maybe later, though.

The interface methods must be declared as virtual, or it wouldn't be possible to override them in a subclass.

Oh, OK, I didn't see this part

class global_playlist_find_impl : public global_playlist_find

global_playlist_find is an abstract interface.  I missed that.
Title: ICC
Post by: Curi0us_George on 2003-06-08 00:22:13
Playlist Find 0.6.b11.012

compiled: 2003.06.07

Quote
[0.6.b11.012]
Fixed:   Potential memory leak
Added:   Pass keystrokes to foobar toggle
Changed:
  Reorganized config slightly.  Disables invalid options properly.
Changed:
  Now clears the current selection if a matching entry is not found.
Fixed:   Wierd bug with the "global" search format.


http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

danZ, try it now.  I had a couple of wierd bugs sneak in.  I'm not sure if your issues will be fixed or not, though.
Title: ICC
Post by: danZ on 2003-06-08 00:36:59
Quote
Playlist Find 0.6.b11.012

compiled: 2003.06.07

Quote
[0.6.b11.012]
Fixed:   Potential memory leak
Added:   Pass keystrokes to foobar toggle
Changed:
  Reorganized config slightly.  Disables invalid options properly.
Changed:
  Now clears the current selection if a matching entry is not found.
Fixed:   Wierd bug with the "global" search format.


http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

danZ, try it now.  I had a couple of wierd bugs sneak in.  I'm not sure if your issues will be fixed or not, though.

Works!  Debug and Release builds.

Care to share what was up?  No crashes when I do search_backwards either which I had in the last build.

Cool, thanks.
Title: ICC
Post by: Curi0us_George on 2003-06-08 00:44:14
Well, I'll give you a hint.

this (the original, broken code):
Code: [Select]
playlist_find_impl()
{
    search_list = NULL;
    char * search_format = NULL;
    srch_type = SEARCH_DEFAULT;
}


is definitely not the same as this (the new, not so messed-up code):
Code: [Select]
playlist_find_impl()
{
    search_list = NULL;
    search_format = NULL;
    srch_type = SEARCH_DEFAULT;
}


I'm honestly not sure why it was working for me.
Title: ICC
Post by: danZ on 2003-06-08 00:46:10
One thing - might be how I'm calling pfind but I think I did it the same as foo_playlistfind

If I type 2 chars past the last match.

So

b - match
be - match
bec - no match
beck - no match

then backspace to

bec

I get a crash when it calls search_backwards

Happens every time but I can't repro on your edit box.

The callstack is in foo_playlistfind.dll when the exception occurs.

Code: [Select]
      int findPos = -1;

     if (wasBack)
         findPos = global_playlist_find::get()->search_backwards(m_KeyFocus->GetText());
     else
         findPos = global_playlist_find::get()->search(m_KeyFocus->GetText());

     playlist_oper::get()->set_focus_sel(findPos);


wasBack is true when I'm processing VK_BACK key.

ideas?

EDIT

m_KeyFocus->GetText() just returns a const char* ptr to "bec"

and I if I only type one char past the last match then backspace no crash.


/EDIT
Title: ICC
Post by: Curi0us_George on 2003-06-08 00:53:04
Well, the obvious question is whether "m_KeyFocus->GetText()" could be returning NULL.

Assuming that's not the case, then the problem is apparently in search_backwards().  I'll take a look at it. 

I should mention, though, that search_backwards() probably isn't doing what you are expecting.  Search backwards searches from bottom to top, meaning that it should be finding the last entry when you are calling it, rather than the previously found entry.
Title: ICC
Post by: danZ on 2003-06-08 00:55:39
Quote
Well, the obvious question is whether "m_KeyFocus->GetText()" could be returning NULL.

Assuming that's not the case, then the problem is apparently in search_backwards().  I'll take a look at it. 

I should mention, though, that search_backwards() probably isn't doing what you are expecting.  Search backwards searches from bottom to top, meaning that it should be finding the last entry when you are calling it, rather than the previously found entry.

Yeah, I just looked at your code again and you don't use search backwards for vk_back and yeah, it won't do what I was thinking.

Still, the text ptr. is def. not null, I am stepping it in the debugger and see "bec" being passed.

EDIT

Yeah, changed my code to not call search_backwards and just call search again when the the string is "backspaced" and no crashes.  Works the same as your edit box.

OK, doesn't work the same (my logic is still not right for an incremental find) but no crashes....

/EDIT
Title: ICC
Post by: Curi0us_George on 2003-06-08 01:19:40
Playlist Find 0.6.b11.013

compiled: 2003.06.07

Quote
[0.6.b11.013]
Fixed:   Bug in interface (wrapping on search_backwards).
Fixed:   Possible (?) crash when searching backwards when no item
  selected.


http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

The searching backwards bug should be fixed.  I couldn't duplicate the crash, but I could see where the bug was.

This should perform an incremental search:
while (USER_WANTS_TO_SEARCH)
{
playlist_oper::get()->set_focus_sel(global_playlist_find::get()->search("YOUR SEARCH STRING HERE"));
}
Title: ICC
Post by: danZ on 2003-06-08 01:31:42
Quote
Playlist Find 0.6.b11.013

compiled: 2003.06.07

Quote
[0.6.b11.013]
Fixed:   Bug in interface (wrapping on search_backwards).
Fixed:   Possible (?) crash when searching backwards when no item
  selected.


http://gelaed.com/resources/cplusplus/foo_...laylistfind.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind.zip)
http://gelaed.com/resources/cplusplus/foo_...istfind-src.zip (http://gelaed.com/resources/cplusplus/foo_playlistfind-src.zip)

The searching backwards bug should be fixed.  I couldn't duplicate the crash, but I could see where the bug was.

This should perform an incremental search:
while (USER_WANTS_TO_SEARCH)
{
playlist_oper::get()->set_focus_sel(global_playlist_find::get()->search("YOUR SEARCH STRING HERE"));
}

Yeah, fixed the crash.

I think search_back is what I want for my algorithm

as the user types in chars I keep calling

search(str)

as soon as they backspace I want to find the previous match and it appears search_back does that.

anyway, seems solid.

Thanks.
Title: ICC
Post by: Curi0us_George on 2003-06-08 02:14:30
For something like that, you should just store the last index found.
Title: ICC
Post by: foosion on 2003-06-08 08:11:30
Quote
Quote
foo_history does contain an example how to use the history service - the GUI. Just have a look at foo_history.cpp, where it is implemented. history_i.cpp contains the implementation for the history service; you actually shouldn't need to worry about it.

So I would inlcude foo_history.h in my build and then simply enum the history service and use it?

Almost. Include foo_history.h and add foo_history.cpp to your project (it contains the guid stuff). You don't need to enumerate yourself, history::get() does this for you. You should check for a null pointer in case the foo_history plugin is not present.
Sorry for the late answer, I haven't been reading the forum for some days.
Title: ICC
Post by: danZ on 2003-06-09 14:49:53
Quote
Quote
Quote
foo_history does contain an example how to use the history service - the GUI. Just have a look at foo_history.cpp, where it is implemented. history_i.cpp contains the implementation for the history service; you actually shouldn't need to worry about it.

So I would inlcude foo_history.h in my build and then simply enum the history service and use it?

Almost. Include foo_history.h and add foo_history.cpp to your project (it contains the guid stuff). You don't need to enumerate yourself, history::get() does this for you. You should check for a null pointer in case the foo_history plugin is not present.
Sorry for the late answer, I haven't been reading the forum for some days.

No problem on the late reply.  This is also how Curi0us_George did the playlistfind interface (using a global "get()" function) which after fleshing out some problems on both sides is working very well now.  I'm doing a similar NULL pointer check for playlistfind and logging a console::error if the look attempts to use playlistfind w/o the service being available.

I'll check out foo_history next and foo_bookmark when available
Title: ICC
Post by: danZ on 2003-06-09 15:06:51
Playlistfind should prob. check the bounds of the start_index parameter because you can still crash playlistfind with bad parameters.

I think the problem is associated with the wrapping code which ends up calling do_matching which crashes if the start index is out of bounds (ie > the get_count() )

Code: [Select]
  // if we get to this point, we didn't find an item that
  // matched after the current one, so we start over from the top
  if (wrap)
  {
   count = start_index;
   for (int i = 0; i < count; i++)
   {
    if (do_matching(str, i)) return i;
   }
  }


Code: [Select]
    bool do_matching(const char * str, int playlist_index) {
 static string8 out;
 out.reset();
 metadb_handle * item;
 item = get_item(playlist_index);
 item->handle_format_title(out,get_search_format(),NULL);
 item->handle_release();
Title: ICC
Post by: Curi0us_George on 2003-06-09 17:56:09
Well, I'm not checking parameters within the playlist_find class, because I think that's the responsibility of the interface user.  However, I am doing some basic bounds checking in the global_playlist_find class.

Code: [Select]
    int search(const char * str, int start_index, bool wrap)
    {
 if (start_index == -1)
 {
     start_index = g_oper->get_focus() + 1;
     if (start_index > g_oper->get_count())
     {
   start_index = g_oper->get_count();
     }
 }
 return glob_find->search(str,start_index,wrap);
    }


Have you actually crashed it with a normal input parameter?  (i.e. -24 dousn't count as "normal")
Title: ICC
Post by: danZ on 2003-06-09 18:05:07
Quote
Well, I'm not checking parameters within the playlist_find class, because I think that's the responsibility of the interface user.  However, I am doing some basic bounds checking in the global_playlist_find class.

Code: [Select]
    int search(const char * str, int start_index, bool wrap)
    {
 if (start_index == -1)
 {
     start_index = g_oper->get_focus() + 1;
     if (start_index > g_oper->get_count())
     {
   start_index = g_oper->get_count();
     }
 }
 return glob_find->search(str,start_index,wrap);
    }


Have you actually crashed it with a normal input parameter?  (i.e. -24 dousn't count as "normal")

No, it works fine as long as you send an index that is in range.  I had an error and sent a bad value which, yes, was my fault.  I'm just saying a simple check would prevent the crash (maybe playlistfind could log an error instead).
Title: ICC
Post by: Curi0us_George on 2003-06-09 18:07:11
I generally avoid doing much checking, because then everything ends up getting checked on both sides.
Title: ICC
Post by: danZ on 2003-06-09 18:42:12
Quote
I generally avoid doing much checking, because then everything ends up getting checked on both sides.

well, like I said, it was a problem on my side so it isn't a big deal I just think returning -1 instead of a crash would be more helpful for someone else who might want to use the foo_playlistfind service.  If I send a bad URL to foo_inet_read I'd expect it to return an error, not crash foobar.

You can also crash it (same bug really) by clearing the playlist in the middle of a playlist find.  While overall I agree with you about double checking, etc.  I think that this particular case would be best served for all potential clients if you just add a simple bounds check.  At the same time its not worth arguing over.

EDIT

Also, if I send out of bounds values to the playlist_oper service they are ignored or error value is returned.

/EDIT
Title: ICC
Post by: Curi0us_George on 2003-06-09 19:50:11
I might add some checking to the actual search later.  It should really be looking for NULL metadb_handles anyway.
Title: ICC
Post by: danZ on 2003-06-09 20:02:49
Quote
I might add some checking to the actual search later.  It should really be looking for NULL metadb_handles anyway.

Thanks.
Title: ICC
Post by: danZ on 2003-06-11 16:08:38
Curi0us_George.

When you get a chance to work on the playlistfind component again could you add a get_search_format() accessor to the global interface.  I want to be able to get that so I can format text for display in my component based on the current searching format.

Thanks.
Title: ICC
Post by: Curi0us_George on 2003-06-11 17:47:44
Sure.
Title: ICC
Post by: danZ on 2003-07-04 16:31:20
Quote
Curi0us_George.

When you get a chance to work on the playlistfind component again could you add a get_search_format() accessor to the global interface.  I want to be able to get that so I can format text for display in my component based on the current searching format.

Thanks.

I haven't looked at 0.7 SDK but I heard some mention of public config variables so perhaps this is a non-issue now???
Title: ICC
Post by: Curi0us_George on 2003-07-04 19:45:06
I won't be making that a public variable. 

But the latest version I uploaded already has an accessor method.  I changed the GUID to prevent problems, but it also has a method to retrieve the version number, so I shouldn't need to change the GUID again.