[vlc-devel] [PATCH] soundcloud.lua: Rewrite to support playlist and user tracks

[Pierre Ynard]

> +function parse_json(url)
> +    vlc.msg.dbg("Trying to parse JSON from " .. url)
> +    local json = require ("dkjson")

Cargo cult? The script would unconditionally use dkjson so there's no
reason to load it this way, besides it's not even proper lazy
initialization, it seems it would be re-loaded each time it's used.

> +-- Parse function.
> +function parse()
> +    local client_id = "fDoItMDbsbZz8dY16ZzARCZmzgHBPotA"
> +    local response = parse_json(vlc.access .. "://api.soundcloud.com/resolve?url=" .. vlc.access .. "://" .. vlc.path .. "&_status_code_map[302]=200&_status_format=json&client_id=" .. client_id)

No URL encoding of vlc.access .. "://" .. vlc.path ? This looks wrong.

> +    local data = parse_json(response.location)

response is used without checking if parse_json() returned an error.

Also, this uses two API calls instead of one, making load time slower
and increasing the possibility of error. I'm pretty sure it's
unnecessary, and anyway the main use case is tracks so that's an
unacceptable drawback.

> +    local playlist = {}
> +    if not data.kind then

data is used without checking if parse_json() returned an error.

> +    elseif data.kind == "user" then
> +        local tracks = parse_json(vlc.access .. "://api.soundcloud.com/users/" .. data.id .. "/tracks" .. "?client_id=" .. client_id)

That's three API calls now for users, but only one seems really
necessary. So no. And the return value needs to be checked for errors.

> -        local api = vlc.stream( vlc.access.."://api.soundcloud.com/i1/tracks/"..track.."/streams?client_id="..client_id.."&app_version="..app_version..( secret and "&secret_token="..secret or "" ) )

I don't see any code for private track secret, so is support gone?

> -    if not path then
> -        vlc.msg.err( "Couldn't extract soundcloud audio URL, please check for updates to this script" )
> -        return { }
> -    end

So kind of error message needs to be kept in case APIs change and
the script breaks. There just doesn't seem to be any error handling
anywhere, and unsupported pages are accepted but then silently ignored.

-- 
Pierre Ynard
"Une âme dans un corps, c'est comme un dessin sur une feuille de papier."