Can someone explain this C++ bit?

mdooligan · 07-07-2015, 03:41 PM

I'm hacking alsaplayer, changing the interface, adding features, etc. Yesterday it was working, today it's not, and I'm trying to figure out what I did to break it it. *shrug* This is how you learn.

I've been programming C as a hobby for quite a number of years now, but only dabbled in C++.

It no longer loads playlists, or anything for that matter.

I tracked it down to this bit:

Code:

Playlist.cpp:

void Playlist::Register (playlist_interface * pl_if)
{
    printf ("%s-1: %p\n", __FUNCTION__, pl_if);
    LockInterfaces ();
    cinterfaces.insert (pl_if); 
    UnlockInterfaces ();
    if (queue.size ()) {
        printf ("%s-1: Qsz\n", __FUNCTION__);
        LockInterfaces ();
        pl_if->cbinsert (pl_if->data, queue, 0); // <-- this bit
        UnlockInterfaces ();
    }
    printf ("%s-1: 5\n", __FUNCTION__);
    LockInterfaces ();
    pl_if->cbsetcurrent (pl_if->data, curritem);
    printf ("%s-1: 6\n", __FUNCTION__);
    UnlockInterfaces ();
}
void Playlist::Register (PlaylistInterface * pl_if)
{
    printf ("%s-2: %p\n", __FUNCTION__, pl_if);
    LockInterfaces ();
    interfaces.insert (pl_if);
    // Tell the interfaces about the current state
    pl_if->CbClear ();
    if (queue.size ()) {
        pl_if->CbInsert (queue, 0);
    } 
    pl_if->CbSetCurrent (curritem);
    UnlockInterfaces ();
}

The printf's are mine. That's how I tracked it down fairly fast.

Why is there 2 nearly identical routines?

By the way, I never modified this file before I started trying to find this problem. I was hacking away in interface/gtk2 when it broke.

queue.size() always returns 0 now, and cbinsert() never gets called. OK, fair enough. Let's have a look at queue.size()... I cannot find it. queue takes me to

Code:

Playlist.h:

class Playlist
{
    (...)
    AlsaNode *our_node; // Node
    CorePlayer *coreplayer; // Core player - set this

    std::vector<PlayItem> queue;    // List of files to play
    //                    ^^^^^

    unsigned curritem;              // Position of next file to play
    std::set<PlaylistInterface *> interfaces;  // Things to tell when things change
    std::set<playlist_interface *> cinterfaces; // C version
    (...)
    std::vector<PlayItem>& GetQueue() { return queue; }
};

OK. What's a PlayItem?

Code:

Playlist.h

class PlayItem
{
    private:
        bool parsed;
        bool eof;
    public:
        PlayItem(std::string filename_new) {
            filename = filename_new;
            playtime = 0;
            parsed = false;
            marked_to_keep_curritem = 0;
            UnsetEof();
        }
        bool Parsed() { return parsed; }
        void SetEof() { eof = true; }
        void UnsetEof() { eof = false; }
        bool Eof() { return eof; }
        void SetParsed() { parsed = true; }
        std::string filename;
        std::string title;
        std::string artist;
        std::string album;
        std::string genre;
        std::string year;
        std::string track;
        std::string comment;
        int playtime;
        bool marked_to_keep_curritem;       // Don't use it if you don't what is it!
};

Here it is, but where's queue.size() that always returns 0 when it shouldn't.

Can someone with a better working knowledge of C++ explain this to me, or point me to a document that will?

Perhaps if I understand this, it will take me closer to understanding C++ in general.

Thank you in advance.

Peace and cheer.

smallpond · 07-07-2015, 03:59 PM

It is quite common in C++ to overload a function name with different arguments so that it can be called from different class methods.

Code:

void Playlist::Register (playlist_interface * pl_if)
void Playlist::Register (PlaylistInterface * pl_if)

Its usually a good idea to keep a copy of the original code. Make and test small changes at a time so you can know where the breakage occurred.

size is a method of the std::vector class.
http://www.cplusplus.com/reference/vector/vector/

suicidaleggroll · 07-07-2015, 04:00 PM

"size" isn't a member of PlayItem, it's part of std::vector:

Code:

std::vector<PlayItem> queue

http://www.cplusplus.com/reference/vector/vector/size/

edit: too slow

johnsfine · 07-07-2015, 04:29 PM

If you posted the changes you made to the original code, likely one of the C++ experts here would spot the bug you introduced. But here are some of the basics that might help you understand it yourself:

You have implied there exists at least one object of type Playlist. Inside each object of type Playlist, there is a member named queue, which is a std::vector of PlayItem's.

Unless forced otherwise (in the construction of queue inside the constructor of Playlist) that vector would be initially empty (so its size is correctly zero).

The code seems to depend on that size not being zero, and I think you said that is not part of your change, so you assume it is not incorrect to depend on the size being nonzero, so it is incorrect that the size is still zero when that part of the code is reached. So before you broke it, some part of the code inserted at least one PlayItem into the queue member of that Playlist. So the next step is find the place in the original code where that happened and figure out why, in your version, it does not.

sundialsvcs · 07-07-2015, 06:02 PM

Another thing that's useful to use, when you are "hacking anything," is: git

Git is a "server-less" version control system. You could cd to the directory where you are hacking the source code, run git init, and, presto! now it's "a repository." (You should start with the unmodified code.)

Do a git branch to describe your brainchild, then start making changes and as you go along, git commit. Exact details of every change are immediately recorded, and you can utterly-reliably go back to any of them. (And, furthermore, since "git always goes forward," you can even un-do "going back."

Nope, you'll never git push that repo anywhere else. But, now you have a way to avoid shooting yourself in the foot. (Or, if you prefer, "an absolutely reliable way to un-shoot it.") If you had done this, not only could you now see exactly what you did, but you also could have un-done it.

NevemTeve · 07-07-2015, 11:59 PM

Note: add fflush(stdout) after every printf: without that your debug messages might get delayed

mdooligan · 07-08-2015, 02:15 PM

Wow. Thank you for the very informative replies.

Those links to cplusplus.com are great. I'm already poking around there checking out the std::stuff. Huge library.

re git: I have git installed and I played with it a bit. I should use it more often. Thanks for reminding me.

re fflush(): I use it when needed. Here the sequence is what's important and I just need to see that the points are being reached. I have certainly done what you say in situations where the timing is critical.

re backups: yeah, I always have backups. I never hack the originals. This subtree is called alsaplayer-0.99.81-hack2

hack7 is usually where it starts getting good, and I pretty much know the code in & out.

So std::vector is a data type, like int or char, except that it's fancier and comes with a bunch of handy utilities. Kinda like declaring a GList *list and then using g_list_length(list), but you can do list.length() instead. So when I see those std::things they are from trusted external library. That means queue.size() is *completely* separate from the program and cannot be part of the issue. Hmm. I can see the playlist loading, but they aren't getting into the queue somehow. This helps me greatly.

This puzzles me:

Code:

Playlist .h:

    std::set<PlaylistInterface *> interfaces;  // Things to tell when things change
    std::set<playlist_interface *> cinterfaces; // C version

What? Why 2 versions? How is it decided which to use? Shouldn't these be in a union or something? Maybe this is issue, because interfaces is a bunch of callbacks that are obviously not getting called.

In alsaplayer here, we have hybrid. Some files are .c, and some are .cpp. It's good opportunity to learn something, and I certainly need to learn C++ better. I don't really care if my hacks turn into something great, I'm just trying to learn by doing (and breaking) things.

Thanks again for the replies. Now I'm not looking in the wrong place.