
<!DOCTYPE html>

<html>

<head>

<title></title>

<style type="text/css">p.MsoNormal,p.MsoNoSpacing{margin:0}</style>

</head>

<body><div><br></div>

<div><br></div>

<div><br></div>

<div>On Tue, Dec 4, 2018, at 10:15, Shaleen Jain wrote:<br></div>

<blockquote type="cite"><div>Hi,<br></div>

<div><br></div>

<div>On Tue, 2018-12-04 at 09:03 +0100, Thomas Guillem wrote:<br></div>

<blockquote type="cite" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:2px;padding-left:1ex;"><div><br></div>

<div><br></div>

<div><br></div>

<div>On Mon, Dec 3, 2018, at 14:31, Shaleen Jain wrote:<br></div>

<blockquote type="cite" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:2px;padding-left:1ex;"><div>On Mon, 2018-12-03 at 12:10 +0100, Thomas Guillem wrote:<br></div>

<blockquote type="cite" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:2px;padding-left:1ex;"><pre>Hello,<br></pre><div><br></div>

<div><br></div>

<div><br></div>

<pre>Could you also refactor renderer module options, like "show-perf-warning", "audio-passthrough", "conversion-quality", and maybe some others.<br></pre><div><br></div>

<pre>You can check how it's done in modules/video_output/opengl/display.c, with add_glopts().<br></pre><div><br></div>

<div><br></div>

<pre>On Wed, Nov 28, 2018, at 16:44, Shaleen Jain wrote:<br></pre></blockquote><div><br></div>

<div>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<br></div>

<div>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).<br></div>

<div>So I'm not sure if it'll be wise to make them (ip and port) common for each renderer.<br></div>

</blockquote><div><br></div>

<div>You're right, it won't.<br></div>

<div>Each modules should have their own options name:<br></div>

<div>"chromecast-something", "upnp-renderer-something" and not a common "renderer-something". This will allow users to setup different options for chromecast and upnp.<br></div>

<div><br></div>

<div>That said, modules can have different options name but using common options via some includes.<br></div>

<div>For ip and port, you could specifiy the default as a MACRO parameter.<br></div>

</blockquote><div><br></div>

<div>I think that would unnecessarily complicate things. <br></div>

<div>ip and port and other options are options that would only be setup by another module (the module acting as the renderer_discovery) <br></div>

<div>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.<br></div>

</blockquote><div><br></div>

<div>OK.<br></div>

<div><br></div>

<blockquote type="cite"><div><br></div>

<div>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"<br></div>

</blockquote><div><br></div>

<div>The "conversion-quality" setting must be chosen according to the chromecast device performance and according to the quality of the network. Therefore, I don't like sharing the same option string that the upnp one (The device handling upnp can have better perfs that the chromecast, so you can choose higher perfs for the upnp case for example).<br></div>

<div><br></div>

<div>Same for "audio-passthrought".<br></div>

<div><br></div>

<div>"show-perf-warning" can be common to all modules.<br></div>

<div><br></div>

<div><br></div>

<blockquote type="cite"><div><br></div>

<div>Please take a look at this patch here: <a href="https://mailman.videolan.org/pipermail/vlc-devel/2018-December/122042.html">https://mailman.videolan.org/pipermail/vlc-devel/2018-December/122042.html</a><br></div>

<div>it implements the things I have described above.<br></div>

</blockquote><div><br></div>

<div>OK, I like that. But like I said I would have added a suffix parameter for "audio-passthrough" and "conversion-quality" (chromecast or upnp). <br></div>

<div><br></div>

<div>PS:You didn't added copyright headers and vlc suffixes to renderer functions like I said in the fist review.<br></div>

<div><br></div>

<blockquote type="cite"><div><br></div>

<blockquote type="cite" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:2px;padding-left:1ex;"><div><br></div>

<blockquote type="cite" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-color:rgb(114, 159, 207);border-left-style:solid;border-left-width:2px;padding-left:1ex;"><div><br></div>

<div><div><span></span><br></div>

<div><span></span><br></div>

<pre><span>-- </span><br></pre><div><span></span><br></div>

<div style="width:71ch;"><span>Regards,</span><br></div>

<div style="width:71ch;"><span>Shaleen Jain</span><br></div>

<div><span></span><br></div>

</div>

<div><u>_______________________________________________</u><br></div>

<div>vlc-devel mailing list<br></div>

<div>To unsubscribe or modify your subscription options:<br></div>

<div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div>

</blockquote><div><br></div>

<pre>_______________________________________________<br></pre><pre>vlc-devel mailing list<br></pre><pre>To unsubscribe or modify your subscription options:<br></pre><div><a href="https://mailman.videolan.org/listinfo/vlc-devel"></a><br></div>

<pre><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></pre><div><a href="https://mailman.videolan.org/listinfo/vlc-devel"></a><br></div>

</blockquote><div><div><span></span><br></div>

<pre><span>-- </span><br></pre><div style="width:71ch;"><span>Regards,</span><br></div>

<div style="width:71ch;"><span>Shaleen Jain</span><br></div>

<div><span></span><br></div>

</div>

<div><u>_______________________________________________</u><br></div>

<div>vlc-devel mailing list<br></div>

<div>To unsubscribe or modify your subscription options:<br></div>

<div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br></div>

</blockquote><div><br></div>

</body>

</html>