[vlc-devel] [PATCH 0/4] Add DLNA/UPNP support

[Hugo Beauzée-Luyssen]

Hi,

Sorry it took quite a bit to look at your patches.

Overall, the patchset looks good, and it mostly will come down to a few leaks/style issues and a bit of refactoring.
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.
>From this set, only patch 2 and 3 seem to apply, but I guess just a rebase on your end will solve this issue.

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.
Please use static_cast when casting from void* to an actual type
I'd suggest you use a control/http port that isn't the one used by the chromecast module

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

I'll review & test more after you send some rebased patches!

Thanks,

On Tue, Aug 14, 2018, at 2:13 PM, Shaleen Jain via vlc-devel wrote:
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> Email had 1 attachment:
> + [PATCH 0/4] Add DLNA/UPNP support
>   3k (message/rfc822)


-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr