exislow / tidal-dl-ng

TIDAL Media Downloader Next Generation! Up to HiRes Lossless / TIDAL MAX 24-bit, 192 kHz.
GNU Affero General Public License v3.0
345 stars 33 forks source link

[Feature] Option to circumvent MQA and optionally downgrade to LOSSLESS #86

Closed crankedguy closed 5 months ago

crankedguy commented 5 months ago

Current Situation

Hi,

now that HI_RES_LOSSLESS is available for "all" I wanted to ask for a feature. As no MQA file has to ever enter my library (I do not want to start and neither am interested on a discussion around MQA, it is just bad) an option for HI_RES_LOSSLESS would be great that if it would deliver an MQA file instead of a high-res FLAC to "downgrade" to lossless. I am aware that another request would have to be made for this to the API with just LOSSLESS, but I could happily live with that.

Is this something you would consider?

Many thanks!

Suggestion / Feature Request

Option in the config file like "downgrade on MQA": true/false

NoDowt commented 5 months ago

I think there was another request for showing quality options available. That might somewhat negate the need for this one also if you can see MQA rather than hires FLAC.

crankedguy commented 5 months ago

Yes that would also be an option, although I am used to command line use and a simple if with a second request automatically would be great though. Of course I would be happy with any solution as long as I do not have to sort it out myself afterwards.

The whole m4a thing with high-res-lossless is strange enough for me, but I completely accept the standpoint of the dev here, as this is also easily "fixable" with a simple ffmpeg copy in batch, as the whole FLAC is already in the m4a, no need to reencode or anything, which would have been a bummer...

crankedguy commented 5 months ago

I have to write a wrapper around anyways as I saw I am quite unhappy with the tags because there seem to be mp4 tags used although the stream contained in the m4a IS a FLAC. And as far as I saw I can only see if this is an MQA in the manifest if there is an encryption_key, and this is not something I can do post download, so it would have to be done here in the downloader.

Another thing absolutely crucial : Why is there the isrc missing in these m4a files/tags? They are used for flac, for mp3, but not for mp4 tags. This is all a bit insane, as said I accept the devs standpoint, BUT : A perfectly encoded flac (which it is without further alteration) encapsulated in a m4a container with mp4 tags is for sure the strangest and most stupid (sorry TIDAL) thing I have ever seen

Example album with no ISRC in these downloaded files : https://tidal.com/browse/album/350296357 It has ISRCs NLHD82400001 upwards IRL

The code also shows clearly that there is no ISRC written in this case :

def set_mp4(self):
        self.m.tags["\xa9nam"] = self.title
        self.m.tags["\xa9alb"] = self.album
        self.m.tags["aART"] = self.albumartist
        self.m.tags["\xa9ART"] = self.artists
        self.m.tags["cprt"] = self.copy_right
        self.m.tags["trkn"] = [[self.tracknumber, self.totaltrack]]
        self.m.tags["disk"] = [[self.discnumber, self.totaldisc]]
        # self.m.tags['\xa9gen'] = self.genre
        self.m.tags["\xa9day"] = self.date
        self.m.tags["\xa9wrt"] = ", ".join(self.composer) if self.composer else ""
        self.m.tags["\xa9lyr"] = self.lyrics

I know there is no standardized ISRC tag in mp4 but please use a freeform tag at least so someone could work with these files ongoing in a professional manner

crankedguy commented 5 months ago

Regarding the downgrading I tried a quick and dirty solution in the main loop just for proof of concept and this seems to work great with temporary setting session.audio_quality to "LOSSLESS" and then back to "HIGH_RES_LOSSLESS", depending on stream_manifest.encryption_type != "NONE"

crankedguy commented 5 months ago

I just upgraded to 0.7.6 mentioned here https://github.com/exislow/tidal-dl-ng/issues/58#issuecomment-2058906627 and used the is_Mqa and is_Hires implementations instead of having to extract the stream manifest for this and it seems to work so far, so good

However, one has to be careful as to how this is implemented and checked in tidalapi, because a track can be actually all 3, MQA, HIRES_LOSSLESS, and LOSSLESS, so if you check on is_Mqa there is a good possibility that the file is also HIRES_LOSSLESS and the MQA isn't delivered at all. This is something where one either has to trust Tidal that they always deliver the HIRES_LOSSLESS or one would have to check in the end again against the downloaded and extracted manifest. I hope one can trust Tidal here to what they promised.

exislow commented 5 months ago

Thank you for your proposal and sharing your testing knowledge. But, first of all, please try to avoid this passive aggressiveness. It doesn't help at all. It is my project, which I advance for free in my leisure time. If you like to help, feel free to get me pull request and get them reviewed / merged.

As you have seen in the issues an upgrade to tidalapi >= 0.7.6 is crucial for further feature implementation. I will let you know, as soon as the update is done. You can use this updated code base for pull request. Or just wait until I find some time to think about a proper solution for your proposal. Thanks and keep up with other feature requests and bug reports :-)

crankedguy commented 5 months ago

@exislow Thanks for your reply. I think this might be a misunderstanding in parts. This passive aggressiveness is/was there, yes, not against you though. But I truly apologize of course if you took it personally! It was not meant this way. This goes more in the direction on what Tidal does, because it is just stupid, I'll stand by that. And I also stand by that understanding you not messing around with it. As said, as the FLAC lies perfectly in there everyting is fine, re-encoding would have been not so fine actually.

Yea I have done an update to 0.7.6 here in my local copy and it seems to work so far, but I did not do very extensive tests, I am sadly lacking time for that right now.

Honestly, there is a lot to do anyways for me in terms of tagging etc,, as I have very special demands for my library which will for sure not be something you can implement globally. E.g. I am using grouping tag for isrc now in mp4 tags. I am not sure if you would want such things in the official code, I wouldn't :). But it is the only solution to either misuse a tag, or just use a free form field, it does not matter as I delete this container, just as an example. I can also do a quick and dirty implement for the downgrade if there is no big demand for that, no worries.

I sadly have to tell you that I stopped contributing to open source projects code-wise quite some time ago. My main job is fixing other companies' botched software, which is a highly paid job luckily, but I am trying to stay clear as far as I can from things in my freetime. I mostly apply dirty and specialized fixes that work for me and do not put an extended amount of time in it. I loved to do it back in the days, but people are more often than not hard to satisfy and I am also sick of the often nonsensical discussions that come up time and again. Of course I am not talking about this project or you, I don't even know you, and I just stumbled upon the project luckily! Sometimes I am for sure missing out on great discussions!

One might think or say this is unsocial, but this is just my standpoint and I am honest about it. If I can be of any other help than writing clean code that is for everyone, like you said with bug reports etc. I am here for it if I find anything. It is a great base for further work here and a nice project!

exislow commented 5 months ago

Regarding ISRC, see https://github.com/exislow/tidal-dl-ng/commit/50737c46a3bd2a85d05e8bce58f486fde20ffe41

crankedguy commented 5 months ago

I know it might sound arrogant but did you test this? I ask because I implemented this exactly like you here, and it did not show up at all in the file as a tag. Supposing as ISRC does not exist in mp4 spec I misused a tag that is in the spec and only then the ISRC showed up.

exislow commented 5 months ago

mediainfo shows the tag correctly.

crankedguy commented 5 months ago

ok many thanks for the info, ffprobe did definitely not. for me it does not matter I used just an unused field of the spec then. I have to use a personalized version anyways, so for me you do not have to implement this request here if you see no demand for it or no one wants to have it. I am simply instantiating the media in the "download" function already and then check against is_mqa and is_hires, then downgrade accordingly. but as the download in my tests was unusually slow (I do not know why) I am thinking of skipping this whole container mess Tidal has incorporated here altogether and stay with 16/44.1 and the very fast downloader I have, taking hires only from qobuz as i am/was used to. not sure yet thanks for getting back so quick!

exislow commented 5 months ago

Implemented with https://github.com/exislow/tidal-dl-ng/commit/b07c3374b24073ba3953797be9d5ecd13af3a482

crankedguy commented 5 months ago

Implemented with https://github.com/exislow/tidal-dl-ng/commit/b07c3374b24073ba3953797be9d5ecd13af3a482

very nice solution, thanks that you implemented it anyways!

do you know why the mpd stream download is so slow? because I find it unusually slow and this does not seem to come from your code. it's so much slower than bts downloads...

exislow commented 5 months ago

I do not know why MPD is slow but I guess, it is because it is a stream. Stream usually buffer not everything at once but from time to time and it does not necessarily to have full bandwidth. I assume that TIDAL just limits the bandwidth on their side, due to this facts,

crankedguy commented 5 months ago

Yea I thought of something in that direction, too. I mean in the end it is not thought to be downloaded if it is streamed anyways, so this is not exactly something one can file a complaint about, same with the encapsulation thing :) But bigger downloads one can forget like this, or run it in the background/overnight etc.

motlaghreyhan commented 5 months ago

Line 216 in download.py looks to be a typo. Causes the app to crash, once I fix the variable to self.settings.data.downgrade_on_hi_res everything works

exislow commented 5 months ago

Some more fixes had been necessary sadly, but checkout this release: https://github.com/exislow/tidal-dl-ng/releases/tag/v0.13.1