Re: [Musicpd-dev-team] UPnP db plugin
You know what: you are right. The UPnP plugin is 4000 lines of bloat and your nitpicking about a few hundred lines of commodity code inside it has just saved you from the whole packet. A few months ago, I put up with your flat out lies about the Proxy DB plugin (no denying possible, just see email messages and commit log), and I've answered calmly to your abuse from last night, because I like MPD and felt like contributing. But I have more pleasant things to do. Please delete my repository from the musicpd site, Best regards. jf Max Kellermann writes: On 2013/12/18 10:02, j...@dockes.org wrote: +static const string rootid(0); Using std::string here is useless bloat, isn't it? There's much more of this useless bloat in your code. I do not think that this is useless bloat, except if you consider that std::string is useless bloat in general. As most of the code is based on std::string, defining the constant as such means that we do not incur a char*-to-std::string conversion each time the constant is used (to construct another string). When would this conversion happen? Use nullptr instead of 0. 0 is obscure. 0 is not obscure, as it was the standard up to really recently (NULL usage was discouraged by Stroustrup himself, you can easily google for this). It WAS the standard. MPD was not designed for this outdated standard. It was rewritten to C++11. When MPD was C99, it used NULL - Stroustrup's opinion does not matter here, because he was talking about C++. Now MPD uses C++11, and it shall use nullptr - Stroustrup's opinion still does not matter, because it's obsolete. I am all for converting to nullptr, but saying that 0 is obscure is a bit of an exaggeration. It obscures the nature of the variable. Using 0 implies that it's an integer, but it's not. It was obscure even when it was the standard. This old standard is obsolete for a good reason. I can change the declaration order if you really insist, but you should be aware that many people (e.g. Google standards) quite logically recommend to put the public declarations (the interface) first, and the private stuff (including the data members) last because this improves readability. There are arguments both for both sides. It boilds down to personal opinion. There are no logical super-arguments for any. I criticized your coding style because it does not follow what the rest of MPD does. Google standards are irrelevant here. The variables are not passed as std::string but as const refs to std::string, so, no overhead. Const or not const does not matter for the question of whether it's overhead, does it? It is overhead because a simple string literal must be converted to std::string each time the function is called - and that implies a heap allocation and deallocation. I actually don't understand your remark. Using std::string for everything except when constrained by an external interface is just good c++ practise. Maybe in your opinion. It's not in my opinion. MPD uses C++ syntax because it is easier to write than C. I converted MPD from C99 to C++ when C++11 came, which allowed me to eliminate most of the runtime overhead. Only then was C++ acceptable for me. Now you're adding all the C++ bloat that I feared, why I have been disliking C++ for so many years. Using a writable ref for returning the result avoids copying a possibly expensive object. This is also just good practise and I think that it's not clear to you because you are not used to it: non-const refs are for returning data, else they would be const... You may want to read https://en.wikipedia.org/wiki/Return_value_optimization What is reallyOpen()? The method name is obscure. I have to delay initialization of libupnp until after mpd daemonizes, else the lib does not work. I don't know if the fork does it or some descriptor manipulation. So it's a kludge to work around a MPD initialization order problem. This should be well-documented in a code comment so it can be eliminated once the MPD core has been improved. And you should write a ticket to request this improvement. + if (m_root) + return true; What does that check mean? As reallyOpen() has to be called for every visit call this is checking if we need to actually do something or we're already done. Setting m_root is the last thing the method does when really working. I could put the test at the call locations, but I find it cleaner this way. Again, there should be a code comment explaining this kludge. + FormatDebug(db_domain, UpnpDatabase::reallyOpen\n); Do we need those debug messages? What value do they add? Well, they're used for debugging actually. I could remove them, but having
Re: [Musicpd-dev-team] UPnP db plugin
Max Kellermann writes: I see many bug fix commit, which fix bugs introduced earlier by yourself. This is confusing to read, and makes the repository hard to bisect. Please fold those into the commit that introduced the bug. Ok, then, I've squashed everything into one commit, as the history was all bug fixes over the initial code add. jf -- Rapidly troubleshoot problems before they affect your business. Most IT organizations don't have a clear picture of how application performance affects their revenue. With AppDynamics, you get 100% visibility into your Java,.NET, PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro! http://pubads.g.doubleclick.net/gampad/clk?id=84349831iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Using MPD to play music from an UPnP Media Server
Max Kellermann writes: On 2013/11/02 16:11, j...@dockes.org wrote: Hi, I have uploaded the code for a UPnP Database plugin to git://git.musicpd.org/medoc/mpd.git Your first commit UpNP database plugin: build and configuration refers to files which don't exist (yet). All commits should be able to build and should work, or git bisect fails, which is annoying when looking for a bug. I wonder why you chose to split the commits this way. I wanted to separate the commit that modified existing mpd files from the commits that added new code. Sorry about not thinking about the bisect issue, but except for the configuration files themselves the commit only activates code if --enable-upnp is used. I wonder if you will actually want to keep this commit history anyway. Maybe the best approach will be to wait for a reasonably stable version (to be decided), decide if you want this at all, add the code in a single modification, and then switch to regular maintenance/modification commits. I think that it is a little early to test for regressions in this code, and that early commit histories only add noise, because it's very difficult to be orderly when exploring new stuff. jf -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Using MPD to play music from an UPnP Media Server
Max Kellermann writes: On 2013/11/02 16:11, j...@dockes.org wrote: Hi, I have uploaded the code for a UPnP Database plugin to git://git.musicpd.org/medoc/mpd.git One more thing, I had a very quick look at the first commit that adds code: you added a copy of libexpat. Why that? Why not link this library, like everybody else does? I am certainly not adding a copy of libexpat ! I have integrated and modified one file from libexpatmm (a c++ wrapper for libexpat), because this is very simple code, and I did not judge that it was worth the trouble to introduce another dependancy for something almost trivial. I have really no objection in principle to depending on libexpatmm instead, only I'm not completely sure that this is packaged on every platform. All this can be changed, this is a relatively big and complex piece of code, and I certainly don't expect to get it right the first time... With the commits I just added tonight, things mostly work with gmpc and minidlna, with exceptions which will be difficult to implement over UpNP (things based on VisitUniqueTags()). The main thing that I find still missing is add directory, I'm not too sure how to do this right. And the stats, I need to do the stats too :) jf -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Using MPD to play music from an UPnP Media Server
Max Kellermann writes: On 2013/11/04 19:02, j...@dockes.org wrote: I wanted to separate the commit that modified existing mpd files from the commits that added new code. I don't follow this argument. Adding a new source code file and registering it in Makefile.am belongs together, that's an operation that cannot be splitted. Only when you require modifications to other parts of MPD as preparation for your feature, then this must be a separate commit. For example, when you need one more parameter/method in an existing API declaration or function prototype, or when you add another generic utility library. Point taken, I should not have done the Makefile.am modification in this commit, but in the one which added the files. I wonder if you will actually want to keep this commit history anyway. Maybe the best approach will be to wait for a reasonably stable version (to be decided), decide if you want this at all, add the code in a single modification, and then switch to regular maintenance/modification commits. Tell me when you feel it should be merged, and then I'll review the branch you cleaned up. It's somewhat cumbersome to review this branch in the current form. Hint: learn about stgit. Ok. This said, at this point in the evolution of the still very young plugin: - I think that it is more useful to look at the code than at the commits. I am mostly using git as a backup tool for now. In really new code like this, it is not so interesting to look at the history, it's too chaotic. I've already dumped a good part of it. - If nobody is interested in actually trying out the function (as seems to be the case), the merge won't be a concern :) jf -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Using MPD to play music from an UPnP Media Server
Max Kellermann writes: On 2013/11/04 19:33, j...@dockes.org wrote: - If nobody is interested in actually trying out the function (as seems to be the case), the merge won't be a concern :) I can't - I have no UPnP hardware. Me neither. Therefore, I have exactly zero personal interest in the feature ;-) I'm much more interested in the MPD proxy db, which I use already. On the other hand, I've also just spent a fair amount of time persuading a piece of code to properly process Hindi text (उसको सत्य नहीं कह सकते हैं), in which I can't even count the letters, so I gather that personal interest is not always the motivation for writing software :) jf -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] Using MPD to play music from an UPnP Media Server
Hi, I have uploaded the code for a UPnP Database plugin to git://git.musicpd.org/medoc/mpd.git The plugin enables MPD to browse and play music from an UPnP Media Server. As far as I can see, things mostly work properly using MiniDLNA as an UPnP server and phpMp as the MPD client. If anybody sees any usefulness in this feature, I'd be very interested by feedback, particularly using other UPnP servers and MPD clients. There is a page of instructions for pulling the code and setting things up: http://www.lesbonscomptes.com/pages/mpd-upnp.html Cheers, jf -- Android is increasing in popularity, but the open development platform that developers love is also attractive to malware creators. Download this white paper to learn more about secure code signing practices that can help keep Android apps secure. http://pubads.g.doubleclick.net/gampad/clk?id=65839951iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] ProxyDatabase plugin
Hi, I have fleshed out the code inside ProxyDatabasePlugin.cpp so that it now works. I have only lightly tested this, on a basic setup, and I don't no a lot about mpd, so I can't be sure that everything is working as it should. One visible problem is a performance issue for the search operation. Because it is not forwarded to the remote server but done by walking the tree, the performance is not good (because of the many trips to the server necessary for exploring each directory). For example on my configuration, a search which takes 0.4 S on the local server will take 6 S when using the proxy. The list operation which is executed by calling the VisitUniqueTags() method in the Database plugin, and which delegates the work to the server, does not suffer from the same issue, and it would seem possible to have the same kind of approach with search (using mpd_search_db_songs() instead of mpd_search_db_tags()), but this needs a non-local change (to the plugin interface). I also had to open up the SongFilter interface a bit (for implementing the constaints inside VisitUniqueTags(). I think that this plugin could be quite useful in a configuration where there is a central music store and several independant players: only the central server has to update(). As as mentionned a couple of months ago, I would like to implement another equivalent plugin to talk to an UPNP Media Server, I'm just coming back to it, and exploring the ProxyDatabase one was a good way to see how things worked on the mpd side. A patch against the current master branch is attached, I hope that you will find it useful. Best regards, jf mpd-proxydb.diff Description: Binary data -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register http://pubads.g.doubleclick.net/gampad/clk?id=60134071iu=/4140/ostg.clktrk___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Musicpd as a media server client
Max Kellermann writes: On 2013/07/22 11:26, j...@dockes.org wrote: I wonder if someone has begun working on this or is planning to in the near future ? Otherwise I'd be quite willing to give it a try. afaik, nobody is working on that. I'm not, because I don't even know what DLNA is. DLNA is an additional compatibility specification over UPnP. The latter defines a standard way for devices (Media Server, Media Player, many other pieces of home electronics) to communicate over a network. Mostly, all communication-capable media devices except the Apple ones use UPnP. The Apple devices use DAAP instead, it's mostly the same function, but implemented differently. The idea is that multiple mpd instances on a network, could share a common music store (UPnP MediaServer), which maintains the catalog (tag scanning etc...) and presents the catalog and data through UPnP/DLNA. This would probably be much better than each instance doing its scanning across a file-sharing protocol (NFS or SMB). There are multiple implementations of DLNA Media Servers, open source or not. Many NAS boxes include a DLNA server. So the work here is just to implement the catalog retrieving and data access parts to let MPD use them. There are several open source libs which could provide the base layer (UPnP is a complicated beast). I was wondering if somebody had maybe started something because there are already multiple entries in the issues db on the subject. But ok, good then, I'll give it a try. Don't expect much progress in August though :) Cheers, jf -- See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] Musicpd as a media server client
Cody Marx Bailey writes: Why not just send it curl strings? There's a CURL input plugin that will suck the playlist/files off a simple HTTP server. You lost me here. How does this solve the catalog/directory part ? -- See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] Musicpd as a media server client
Hello, It has become quite popular to implement audio players by running mpd on small devices (e.g.: Raspberry PI). These devices often have little to no local storage and access the audio data through the network. Currently, this has to be done using file sharing protocols (SMB or NFS), which has multiple disadvantages (redundant scanning of tags, possible access control issues). Enabling mpd to function as a MediaRenderer would be very valuable, as all players could share the directory and data access services of the MediaServers on the network. Issue 1924 in the musicpd bug tracker proposes to integrate mpd in the DLNA framework: http://bugs.musicpd.org/view.php?id=1924 I wonder if someone has begun working on this or is planning to in the near future ? Otherwise I'd be quite willing to give it a try. As far as I can see, the tasks involved are: - Selecting the most appropriate upnp library package - Implementing the directory part. I guess that the experimental Proxy Database plugin could be the base for this ? - Implementing an input plugin. Please let me know if my working on this makes sense. Cheers, J.F. Dockes -- See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831iu=/4140/ostg.clktrk ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
