<html><head></head><body><div>Hi,</div><div><br></div><div>On Tue, 2018-08-28 at 13:30 +0200, Hugo Beauzée-Luyssen wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>Hi,</div><div><br></div><div>Sorry it took quite a bit to look at your patches.</div><div><br></div><div>Overall, the patchset looks good, and it mostly will come down to a few leaks/style issues and a bit of refactoring.</div><div>That being said, I'm not sure where the issue lies down, but the patch is an attached email for me, which makes it quite hard to review inline.</div><div>From this set, only patch 2 and 3 seem to apply, but I guess just a rebase on your end will solve this issue.</div><div><br></div><div>Regarding patch 4, a lot of function seem to to the exact same thing, with the exception of how many times UpnpAddToAction gets called. It would be a better idea to have the common part as a separated function, and pass the various parameters as a vector/va_args/something similar. Also, you seem to always leek what getServiceURL returns.</div><div>Please use static_cast when casting from void* to an actual type</div></blockquote><div>Ok</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>I'd suggest you use a control/http port that isn't the one used by the chromecast module</div></blockquote><div>Ok</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><br></div><div>You can probably simplify quite a bit by using C++11 unique/shared_ptr to manage the lifetime of your objects in patches 3 & 4</div><div><br></div><div>I'll review & test more after you send some rebased patches!</div></blockquote><div><br></div><div>Thanks, I'll send my revised and improved patches shortly .</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><br></div><div>Thanks,</div><div><br></div><div>On Tue, Aug 14, 2018, at 2:13 PM, Shaleen Jain via vlc-devel wrote:</div><div><br></div><div>_______________________________________________</div><div>vlc-devel mailing list</div><div>To unsubscribe or modify your subscription options:</div><div><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></div><div>Email had 1 attachment:</div><div>+ [PATCH 0/4] Add DLNA/UPNP support</div><div> 3k (message/rfc822)</div><div><br></div><div><br></div><div><br></div><div><br></div></blockquote><div><span><pre>-- <br></pre><div>Regards,</div><div>Shaleen Jain</div></span></div></body></html>