Audacious

This line seems wrong:
tuple_set_int(submit_tuple, FIELD_LENGTH, NULL, 240000);

Why should you artificially set the length to 4m00s?

For the different titles in the same stream, we do initially not know how long they are. So initially, the length is set to an undefined value:

tuple_get_int(submit_tuple, FIELD_LENGTH, NULL) <= 0

In general, we will not be able to determine the length of the title until the next title starts ( which is noticed by a change in the title or artist tag field ). So my first attempt was to keep the length at the initial value or assign it a very small value before we know its real length.

But then I observed that the length value that is submitted as now playing to last.fm seems to have an influence on how long the track is displayed as "now playing" on last.fm. This means that every track was only shown as "now playing" at last.fm for a few seconds and then disappeared again, which is clearly not as intended.

• Initially assume a length of four minutes.
• If the title is shorter than four minutes, then this is no problem. The title's real length is obtained from the amount of time that the title was playing at the moment when the next title starts. The old title information together with its length found out is then put into the list of tracks that were listened to completely. () The now playing information for the new title is then immediately submitted.
• The problematic case is when the title played is longer than four minutes. So after the currently approximated length is over (np_item->timeplayed > np_item->len), another now playing request (with the same length) is sent again and the length is set to the double of the value. This means after for 4 minutes another now playing of four minutes is sent. After 8 minutes another now playing of 8 minutes and so on. This was just an easy possiblility to not have to send a now playing request too often.

The crucial part is that the now playing request has no influence on the list of songs really listened to but only controls the now playing information displayed in the last.fm profile. So initially overapproximating is uncritical as the next now playing request for another title will overwrite the old one is needed. Initially underapproximating is also uncritical as we may simply approximate the doubled value as soon as the currently approximated value is too short.

In my tests, this leaded to an appropriate treatment of the now playing information in my last.fm profile.

• Titles so far not known to last.fm would be treated wrong
• If the title has a different length then the same title in the last.fm db (an alternate version or something), then it could possibly be shown as now playing for a too short period of time. But the intention of the now playing field is too show the title exactly as long it is really played.

I see now why you would submit a bogus length to last.fm, but changing the length in the tuple will change it in the Audacious playlist also. That's not what you intended, is it?

You are right, that is not what I intended. I just had a short look at the sources again and it seems I did not quite understand the meaning of aud_playlist_entry_get_tuple. I thought it would create an independent copy of the information in the playlist.

I did not realize that this could be the case as, at least with my version here, in the user interface there is no length displayed for streams at all.

I agree with you, that this is surely not the best way. Maybe one should use a plugin-local variable to do the accumulated changing of the title length in and replace the use of the tuple in all places by this?

On the other hand, if it is clear, that streams will most likely not starting to provide some meaningful length tag information, then user interfaces will also most likely not start to display that value and there will be nobody noticing the bogus value inside the length field. And before the hack, there was also a bogus value in that field, namely something <= 0.

It's clearly a matter of style, how to solve this. Both ways are not optimal and I wonder if it is worth rewriting from one to ther other.

I think, on the long run, the information in the playlist tuple of audacious should anyway become more aware of the possibility of stream entries. We should not have to use our own ishttp helper function. Maybe there should one day be an own entry sub type for stream entries and and one for file based entries. The length field would then only exist for file based entries and we could not even try to put a bogus value in there.

If you want to make a copy of the tuple data locally, there is tuple_copy(). As for expecting the user interface to not display a bogus value, I think that is a bad idea. We exclude 0 and -1 from being displayed because some plugins used to treat those as reserved values meaning "unknown length", but even that behavior should not be relied on.

If you want to make a second version of the patch, I will be happy to look it over.

This block of code:

    if (submit_tuple)
        tuple_unref (submit_tuple);
    {
        Tuple *temp_tuple = aud_playlist_entry_get_tuple (playlist, pos, FALSE);
        if (! temp_tuple)
            return;
        submit_tuple = tuple_copy (temp_tuple);
        tuple_unref (temp_tuple);
    }

Also, you set item->is_http_source to FALSE in create_item(), and then set it to its proper value after create_item() returns. It would be clearer to pass the proper value to create_item().

hi,

last.fm does not allow scrobbling http streams like shoutcast, they will revoke audacious's privileges to scrobble if we implement that.

how do you prevent scrobbling shoutcast streams?

Hi William,

in fact, I was not aware that some streams are not allowed to be scrobbled.

There is currently no solution to this problem.

I will first try to do a little web research on this topic. Could you please provide me your source of information?

The thing that surprises me a bit is that other players seem to also scrobble from streams. Maybe I will have to take a look if and how they treat this problem.

Michael Jacobs wrote:

In fact, the second point you mention is also related to this. You are right about the thing that temp_tuple should in general be set to null after unreferencing it. However, in this special case, I omitted it, as the scope of temp_tuple ends anyway after the unref and as the only purpose of the braces was to wrap the temporal variable and prohibit its usage at other positions in the method.

Read what I said again. temp_tuple is not the problem, it's submit_tuple that you need to set to null.

William Pitcock wrote:

last.fm does not allow scrobbling http streams like shoutcast, they will revoke audacious's privileges to scrobble if we implement that.

Are you sure this is still the case? I skimmed Last.fm's API documentation (http://www.last.fm/api/scrobbling) and their terms of service (http://www.last.fm/api/tos) and didn't see anything mentioned.