[vlc-devel] [RFC v2] audioscrobbler: submit the currently playing track

Hugo Beauzée-Luyssen hugo at beauzee.fr
Wed Nov 4 13:55:37 CET 2015


Hi,

On 10/30/2015 01:11 PM, Jonas Lundqvist wrote:
> This is an implementation of the "Now Playing" feature that will show
> the currently playing song on last.fm.
> ---
>   modules/misc/audioscrobbler.c | 176 +++++++++++++++++++++++++-----------------
>   1 file changed, 104 insertions(+), 72 deletions(-)
>
> diff --git a/modules/misc/audioscrobbler.c b/modules/misc/audioscrobbler.c
> index bbbf69a..b3da6d8 100644
> --- a/modules/misc/audioscrobbler.c
> +++ b/modules/misc/audioscrobbler.c
> @@ -22,11 +22,10 @@
>    * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301, USA.
>    *****************************************************************************/
>
> -/* audioscrobbler protocol version: 1.2
> - * http://www.audioscrobbler.net/development/protocol/
> +/* Last.fm Submissions protocol version: 1.2
> + * http://www.last.fm/api/submissions
>    *
> - * TODO:    "Now Playing" feature (not mandatory)
> - *          Update to new API? http://www.lastfm.fr/api
> + * TODO:    Update to new API? http://www.last.fm/api/scrobbling
>    */

This should be in a separate patch

>   /*****************************************************************************
>    * Preamble
> @@ -88,11 +87,8 @@ struct intf_sys_t
>       vlc_url_t               p_submit_url;       /**< where to submit data   */
>
>       /* submission of playing song */
> -#if 0 //NOT USED
> -    char                    *psz_nowp_host;     /**< where to submit data   */
> -    int                     i_nowp_port;        /**< port to which submit   */
> -    char                    *psz_nowp_file;     /**< file to which submit   */
> -#endif

Removal of dead code should be in another patch as well.

> +    vlc_url_t               p_nowp_url;         /**< where to submit data   */
> +
>       char                    psz_auth_token[33]; /**< Authentication token */
>
>       /* data about song currently playing */
> @@ -101,7 +97,7 @@ struct intf_sys_t
>       mtime_t                 time_pause;         /**< time when vlc paused   */
>       mtime_t                 time_total_pauses;  /**< total time in pause    */
>
> -    bool                    b_submit;           /**< do we have to submit ? */
> +    bool                    b_submit_nowp;      /**< do we have to submit ? */
>
>       bool                    b_meta_read;        /**< if we read the song's
>                                                    * metadata already         */
> @@ -196,8 +192,8 @@ static void ReadMetaData(intf_thread_t *p_this, input_thread_t *p_input)
>           goto end;
>       }
>
> -    /* Now we have read the mandatory meta data, so we can submit that info */
> -    p_sys->b_submit = true;
> +    /* We have the mandatory meta data and can submit the now playing song */
> +    p_sys->b_submit_nowp = true;
>
>       ALLOC_ITEM_META(p_sys->p_current_song.psz_b, Album);
>       if (!p_sys->p_current_song.psz_b)
> @@ -216,8 +212,11 @@ static void ReadMetaData(intf_thread_t *p_this, input_thread_t *p_input)
>
>       msg_Dbg(p_this, "Meta data registered");
>
> +    vlc_cond_signal(&p_sys->wait);
> +
>   end:
>       vlc_mutex_unlock(&p_sys->lock);
> +

Extra whitespace change

>   }
>
>   /*****************************************************************************
> @@ -229,7 +228,9 @@ static void AddToQueue (intf_thread_t *p_this)
>       intf_sys_t                  *p_sys = p_this->p_sys;
>
>       vlc_mutex_lock(&p_sys->lock);
> -    if (!p_sys->b_submit)
> +
> +    /* Check that we have the mandatory meta data */
> +    if (!p_sys->p_current_song.psz_t || !p_sys->p_current_song.psz_a)
>           goto end;
>
>       /* wait for the user to listen enough before submitting */
> @@ -297,7 +298,6 @@ static void AddToQueue (intf_thread_t *p_this)
>
>   end:
>       DeleteSong(&p_sys->p_current_song);
> -    p_sys->b_submit = false;
>       vlc_mutex_unlock(&p_sys->lock);
>   }
>
> @@ -360,7 +360,6 @@ static int ItemChange(vlc_object_t *p_this, const char *psz_var,
>       VLC_UNUSED(oldval);
>
>       p_sys->b_meta_read      = false;
> -    p_sys->b_submit         = false;
>
>       if (p_sys->p_input != NULL)
>       {
> @@ -389,6 +388,7 @@ static int ItemChange(vlc_object_t *p_this, const char *psz_var,
>       p_sys->p_input = vlc_object_hold(p_input);
>       var_AddCallback(p_input, "intf-event", PlayingChange, p_intf);
>
> +

extra whitespace

>       if (input_item_IsPreparsed(p_item))
>           ReadMetaData(p_intf, p_input);
>       /* if the input item was not preparsed, we'll do it in PlayingChange()
> @@ -409,6 +409,7 @@ static int Open(vlc_object_t *p_this)
>           return VLC_ENOMEM;
>
>       p_intf->p_sys = p_sys;
> +    p_sys->b_submit_nowp = false;
>
>       vlc_mutex_init(&p_sys->lock);
>       vlc_cond_init(&p_sys->wait);
> @@ -449,10 +450,7 @@ static void Close(vlc_object_t *p_this)
>       for (i = 0; i < p_sys->i_songs; i++)
>           DeleteSong(&p_sys->p_queue[i]);
>       vlc_UrlClean(&p_sys->p_submit_url);
> -#if 0 //NOT USED
> -    free(p_sys->psz_nowp_host);
> -    free(p_sys->psz_nowp_file);
> -#endif

ditto

> +    vlc_UrlClean(&p_sys->p_nowp_url);
>       vlc_cond_destroy(&p_sys->wait);
>       vlc_mutex_destroy(&p_sys->lock);
>       free(p_sys);
> @@ -616,23 +614,13 @@ static int Handshake(intf_thread_t *p_this)
>
>       /* We need to read the nowplaying url */
>       p_buffer_pos += 7; /* we skip "http://" */
> -#if 0 //NOT USED
>       psz_url = strndup(p_buffer_pos, strcspn(p_buffer_pos, "\n"));
>       if (!psz_url)
>           goto oom;
>
> -    switch(ParseURL(psz_url, &p_sys->psz_nowp_host,
> -                &p_sys->psz_nowp_file, &p_sys->i_nowp_port))
> -    {
> -        case VLC_ENOMEM:
> -            goto oom;
> -        case VLC_EGENERIC:
> -            goto proto;
> -        case VLC_SUCCESS:
> -        default:
> -            break;
> -    }
> -#endif
> +    vlc_UrlParse(&p_sys->p_nowp_url, psz_url);
> +    free(psz_url);
> +
>       p_buffer_pos = strstr(p_buffer_pos, "http://");
>       if (!p_buffer_pos || strlen(p_buffer_pos) == 7)
>           goto proto;
> @@ -683,6 +671,7 @@ static void *Run(void *data)
>       uint8_t                 p_buffer[1024];
>       int                     canc = vlc_savecancel();
>       bool                    b_handshaked = false;
> +    bool                    b_nowp_submission_ongoing = false;
>
>       /* data about audioscrobbler session */
>       mtime_t                 next_exchange = 0; /**< when can we send data  */
> @@ -699,7 +688,7 @@ static void *Run(void *data)
>           vlc_mutex_lock(&p_sys->lock);
>           mutex_cleanup_push(&p_sys->lock);
>
> -        while (p_sys->i_songs == 0)
> +        while (p_sys->i_songs == 0 && p_sys->b_submit_nowp == false)
>               vlc_cond_wait(&p_sys->wait, &p_sys->lock);
>
>           vlc_cleanup_pop();
> @@ -748,57 +737,91 @@ static void *Run(void *data)
>           }
>
>           msg_Dbg(p_intf, "Going to submit some data...");
> +
>           char *psz_submit;
> +        vlc_url_t *url;
> +        char *psz_submit_song, *psz_submit_tmp;
> +
>           if (asprintf(&psz_submit, "s=%s", p_sys->psz_auth_token) == -1)
>               break;
>
>           /* forge the HTTP POST request */
>           vlc_mutex_lock(&p_sys->lock);
> -        audioscrobbler_song_t *p_song;
> -        for (int i_song = 0 ; i_song < p_sys->i_songs ; i_song++)
> +
> +        if (p_sys->b_submit_nowp)
>           {
> -            char *psz_submit_song, *psz_submit_tmp;
> -            p_song = &p_sys->p_queue[i_song];
> +            b_nowp_submission_ongoing = true;
> +            url = &p_sys->p_nowp_url;
>               if (asprintf(&psz_submit_song,
> -                    "&a%%5B%d%%5D=%s"
> -                    "&t%%5B%d%%5D=%s"
> -                    "&i%%5B%d%%5D=%u"
> -                    "&o%%5B%d%%5D=P"
> -                    "&r%%5B%d%%5D="
> -                    "&l%%5B%d%%5D=%d"
> -                    "&b%%5B%d%%5D=%s"
> -                    "&n%%5B%d%%5D=%s"
> -                    "&m%%5B%d%%5D=%s",
> -                    i_song, p_song->psz_a,
> -                    i_song, p_song->psz_t,
> -                    i_song, (unsigned)p_song->date, /* HACK: %ju (uintmax_t) unsupported on Windows */
> -                    i_song,
> -                    i_song,
> -                    i_song, p_song->i_l,
> -                    i_song, p_song->psz_b,
> -                    i_song, p_song->psz_n,
> -                    i_song, p_song->psz_m
> -           ) == -1)
> +                "&a=%s"
> +                "&t=%s"
> +                "&b=%s"
> +                "&l=%d"
> +                "&n=%s"
> +                "&m=%s",
> +                p_sys->p_current_song.psz_a,
> +                p_sys->p_current_song.psz_t,
> +                p_sys->p_current_song.psz_b,
> +                p_sys->p_current_song.i_l,
> +                p_sys->p_current_song.psz_n,
> +                p_sys->p_current_song.psz_m
> +                ) == -1)
>               {   /* Out of memory */
>                   vlc_mutex_unlock(&p_sys->lock);
>                   goto out;
>               }
> -            psz_submit_tmp = psz_submit;
> -            if (asprintf(&psz_submit, "%s%s",
> -                    psz_submit_tmp, psz_submit_song) == -1)
> -            {   /* Out of memory */
> -                free(psz_submit_tmp);
> -                free(psz_submit_song);
> -                vlc_mutex_unlock(&p_sys->lock);
> -                goto out;
> +
> +        }
> +        else
> +        {
> +            url = &p_sys->p_submit_url;
> +            audioscrobbler_song_t *p_song;
> +            for (int i_song = 0 ; i_song < p_sys->i_songs ; i_song++)
> +            {
> +                p_song = &p_sys->p_queue[i_song];
> +                if (asprintf(&psz_submit_song,
> +                        "&a%%5B%d%%5D=%s"
> +                        "&t%%5B%d%%5D=%s"
> +                        "&i%%5B%d%%5D=%u"
> +                        "&o%%5B%d%%5D=P"
> +                        "&r%%5B%d%%5D="
> +                        "&l%%5B%d%%5D=%d"
> +                        "&b%%5B%d%%5D=%s"
> +                        "&n%%5B%d%%5D=%s"
> +                        "&m%%5B%d%%5D=%s",
> +                        i_song, p_song->psz_a,
> +                        i_song, p_song->psz_t,
> +                        i_song, (unsigned)p_song->date, /* HACK: %ju (uintmax_t) unsupported on Windows */
> +                        i_song,
> +                        i_song,
> +                        i_song, p_song->i_l,
> +                        i_song, p_song->psz_b,
> +                        i_song, p_song->psz_n,
> +                        i_song, p_song->psz_m
> +                       ) == -1)
> +                {   /* Out of memory */
> +                        vlc_mutex_unlock(&p_sys->lock);
> +                        goto out;
> +                }
>               }
> -            free(psz_submit_song);
> +        }
> +
> +        psz_submit_tmp = psz_submit;
> +        if (asprintf(&psz_submit, "%s%s",
> +                    psz_submit_tmp, psz_submit_song) == -1)
> +        {   /* Out of memory */
>               free(psz_submit_tmp);
> +            free(psz_submit_song);
> +            vlc_mutex_unlock(&p_sys->lock);
> +            goto out;
>           }

You could free those unconditionally, and then check for asprintf error

> +        free(psz_submit_song);
> +        free(psz_submit_tmp);
> +
>           vlc_mutex_unlock(&p_sys->lock);
>
> -        int i_post_socket = net_ConnectTCP(p_intf, p_sys->p_submit_url.psz_host,
> -                                        p_sys->p_submit_url.i_port);
> +        int i_post_socket = net_ConnectTCP(p_intf, url->psz_host,
> +                                        url->i_port);
>

You didn't check for vlc_UrlParse success in Handshake, so host/url 
could be NULL here.

>           if (i_post_socket == -1)
>           {
> @@ -821,8 +844,8 @@ static void *Run(void *data)
>               "\r\n"
>               "%s\r\n"
>               "\r\n",
> -            p_sys->p_submit_url.psz_path, strlen(psz_submit),
> -            p_sys->p_submit_url.psz_host, psz_submit
> +            url->psz_path, strlen(psz_submit),
> +            url->psz_host, psz_submit
>          );
>
>           free(psz_submit);
> @@ -868,9 +891,18 @@ static void *Run(void *data)
>
>           if (strstr((char *) p_buffer, "OK"))
>           {
> -            for (int i = 0; i < p_sys->i_songs; i++)
> -                DeleteSong(&p_sys->p_queue[i]);
> -            p_sys->i_songs = 0;
> +            if (b_nowp_submission_ongoing)
> +            {
> +                b_nowp_submission_ongoing = false;
> +                p_sys->b_submit_nowp = false;
> +            }
> +            else
> +            {
> +                for (int i = 0; i < p_sys->i_songs; i++)
> +                    DeleteSong(&p_sys->p_queue[i]);
> +                p_sys->i_songs = 0;
> +            }
> +
>               i_interval = 0;
>               next_exchange = 0;
>               msg_Dbg(p_intf, "Submission successful!");
>

Regards,



More information about the vlc-devel mailing list