[vlc-devel] [PATCH] lua: Expose media tracks through lua bindings

[Hugo Beauzée-Luyssen]

On 01/01/2017 02:49 AM, Pierre Ynard wrote:
> Happy New Year,

Happy new year indeed!

>
> Can you explain what the purpose of and use cases for this new API are,
> how it is not redundant with vlclua_input_item_info(), etc...? Also
> please patch the API documentation too.

The way I understand it, vlclua_input_item_info is related to a media 
meta info, I guess I see it as a libvlc_media_get_meta equivalent

I'll update the documentation in the next version of this patch

>
>> +    for ( int i = 0; i < p_item->i_es; ++i )
>> +    {
>
>> +        lua_rawseti( L, -2, i );
>> +    }
>
> I think lua table are indexed starting from 1 instead of 0?...
>

«Since you can index a table with any value, you can start the indices 
of an array with any number that pleases you. However, it is customary 
in Lua to start arrays with one (and not with zero, as in C) and the 
standard libraries stick to this convention.»

So apparently it is, thanks!

> What's the reason for the choice of exposed fields? Limited
> implementation, avoiding bloat...? Does the ES ID really need to be
> exposed to lua scripts?
>
>> +void luaopen_input_constants( lua_State *L )
>> +{
>> +    lua_newtable(L);
>> +    lua_pushinteger( L, VIDEO_ES );
>> +    lua_setfield( L, -2, "video" );
>> +    lua_pushinteger( L, AUDIO_ES );
>> +    lua_setfield( L, -2, "audio" );
>> +    lua_pushinteger( L, SPU_ES );
>> +    lua_setfield( L, -2, "spu" );
>> +    lua_pushinteger( L, UNKNOWN_ES );
>> +    lua_setfield( L, -2, "unknown" );
>> +    lua_setfield( L, -2, "tracks" );
>> +}
>
> That looks over-complicated, bloats the variable namespace, and the
> lua code I can imagine to use it looks kinda awkward. I think it would
> probably be better to expose the type as a string field in the track
> table, or maybe split the tracks into groups indexed by type, or
> something else.

I thought of splitting the values within different tables, but that felt 
a bit overkill to me. I agree that pushing a few constants looks ugly.
The lua code doesn't feel weird IMHO (if tracks.type == 
vlc.tracks.video), but that's a matter of taste I suppose. I'm fine 
changing it.

So to summarize, you'd like to have the type as a string, and preferably 
a video/audio sub table when appropriate?

Thanks for your review,

Regards,