[vlc-devel] [PATCH 1/3] chromecast: refactor out encoder option functions

[Shaleen Jain]

Hi,
On Tue, 2018-12-04 at 09:03 +0100, Thomas Guillem wrote:
> 
> 
> 
> 
> On Mon, Dec 3, 2018, at 14:31, Shaleen Jain wrote:
> 
> > On Mon, 2018-12-03 at 12:10 +0100, Thomas Guillem wrote:
> > 
> > > Hello,
> > > 
> > > 
> > > Could you also refactor renderer module options, like "show-perf-warning", "audio-
> > > passthrough", "conversion-quality", and maybe some others.
> > > You can check how it's done in modules/video_output/opengl/display.c, with
> > > add_glopts().
> > > 
> > > On Wed, Nov 28, 2018, at 16:44, Shaleen Jain wrote:
> > 
> > Do you mean to use a define for the common options? besides the three that you have
> > already mentioned "ip" and "port" are the common
> > 
> > options as they are required by vlc_renderer_item_new but they have different defaults
> > for chromecast and dlna each (for e.g chromecast has port 8009 and dlna has port 7070).
> > 
> > So I'm not sure if it'll be wise to make them (ip and port) common for each renderer.
> > 
> 
> You're right, it won't.
> 
> Each modules should have their own options name:
> 
> "chromecast-something", "upnp-renderer-something" and not a common "renderer-something".
> This will allow users to setup different options for chromecast and upnp.
> 
> 
> 
> That said, modules can have different options name but using common options via some
> includes.
> 
> For ip and port, you could specifiy the default as a MACRO parameter.

I think that would unnecessarily complicate things. ip and port and other options are
options that would only be setup by another module (the module acting as the
renderer_discovery) and no user will change it manually so it would be fine to let the sout
module add defines for them with the default and the module specific prefix.
But for the common options that would be exposed to the user and influenced by the platform
like "show-perf-warning", "audio-passthrough", "conversion-quality", I have them as a
single prefix ("sout-renderer-") and the marco "add_renderer_opts"
Please take a look at this patch here: 
https://mailman.videolan.org/pipermail/vlc-devel/2018-December/122042.html
it implements the things I have described above.
> 
> > 
> > 
> > -- 
> > Regards,
> > 
> > Shaleen Jain
> > 
> > 
> > 
> > 
> > _______________________________________________
> > 
> > vlc-devel mailing list
> > 
> > To unsubscribe or modify your subscription options:
> > 
> > https://mailman.videolan.org/listinfo/vlc-devel
> > 
> 
> 
> 
> _______________________________________________vlc-devel mailing listTo unsubscribe or
> modify your subscription options:https://mailman.videolan.org/listinfo/vlc-devel
-- 
Regards,
Shaleen Jain
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20181204/2dcbbbd0/attachment.html>