Bitrate filter does not work as expected

[defaultxr]

Nicotine+ version: 3.2.2
Operating System/Distribution: Arch Linux

Describe the bug

The "Bitrate" results filter does not seem to work as expected:

• I would expect that typing <320 and pressing enter should hide all results with bitrates of 320 or above. But 320kbps mp3s are still visible.
• Typing >192 and pressing enter should hide all results with bitrates of 192 or below. They are still visible though.
• Typing >192 <320 and pressing enter should show only results with bitrates above 192 and below 320. Again, this is not what happens.

It seems that Nicotine+ is interpreting > as >= and < as <=, since <319 seems to hide 320kbps results as expected; >193 also seems to hide 192kbps.

However, then I would expect a query like >193 <319 to hide 320kbps and 192kbps results. Instead, I still see them. Is it possible to filter to a specific range?

Maybe I am misunderstanding how this filter works; if so, a more informative tooltip may be helpful. I could've sworn previous versions had a better tooltip but in 3.2.2 it just says "Bitrate" when I hover my mouse over the input field.

Either way, the bitrate field has always felt a bit buggy.

Expected behavior

• > and < operate as greater then/less than, rather than greater than or equals/less than or equals.
• Multiple (space-separated) entries in the Bitrate field should be AND-ed together like they are in the other filters.
• A better tooltip with bitrate filtering syntax and examples of bitrate filtering.

Steps to reproduce the bug

1. Do a search.
2. Open "Result filters".
3. Try any of the above examples.

Additional context

Let me know if you need any other information.

[slook]

The bitrate filter has already been revised in 3.3.0dev1, see #1892.

Is it possible to filter to a specific range?

Multiple (space-separated) entries in the Bitrate field should be AND-ed together like they are in the other filters.

Yes, multiple conditions are possible to specify a range now with the | (OR) operator syntax in the latest build.

Please can you review the behaviour in the latest unstable build of Nicotine+ and we can go from there.

---

Changing from Bug to Enhancement, since it is debatable as to what the expected behaviour of the < and > operators should be, and any improvement would require the addition of more operators for the filters or the ability to select the default operator.

> and < operate as greater then/less than, rather than greater than or equals/less than or equals.

Whilst this would make sense for the integer values of Bitrate, it would not work for the Size and Duration filters because applying the same logic there (and indeed the same logic is actually used for all three filters) would wrongly exclude files that are infact only a decimal fraction more/less than the input specified.

[defaultxr]

Yes, multiple conditions are possible to specify a range now with the | (OR) operator syntax in the latest build.

Hmm. But "AND" is not supported for bitrates? I tried with & but it seems to exclude all results. For example, even though there are plenty of 256kbps files in the results, a bitrate filter like >192&<320 doesn't show them (or any results, in fact).

Please can you review the behaviour in the latest unstable build of Nicotine+ and we can go from there.

Sure. I tried the flatpak linked in TESTING.md and am seeing the same behavior. <320 still shows files with 320kbps bitrates, etc.

Whilst this would make sense for the integer values of Bitrate, it would not work for the Size and Duration filters because applying the same logic there (and indeed the same logic is actually used for all three filters) would wrongly exclude files that are infact only a decimal fraction more/less than the input specified.

So you're saying that an input like <3MB or similar would exclude files that are 3.01MB, for example? What's wrong with that? I think most people would say that's correct. I would think if a user wants files less than 3.01MB they would input something like <3.01MB instead.

[slook]

In the context of the filter functions AND is OR so the seperator used is the | (pipe) character, since a single file object cannot be both 192Kbps and 320Kbps at the same time.

Admittedly more characters other than | would make specifying multiple conditions much more intuitive as you have highlighted, and really it is reasonable for an end user to consider the filters apply to a group of result objects rather than a single object, so & should be considered as a valid operator regardless of the actual operation done in the filtering function.

For the sizes I made it so that values which are converted from Bytes into MiB are approximated to with 0.125 of a unit, because otherwise getting or hiding any results with the = (equal) or ! (not-equal) operators was practically impossible unless the exact raw Bytes size was known and entered.

For the duration I suppose one would consider the example <3:01 to not show files that are 3:01.9, because milliseconds are not displayed in the interface, nor supported in the filters, but >3:01 would be confusing not to see files that were say 3:01.9 long, even though it is technically longer it still shows as 3:01 in the results list.

It seems a reason why < and > are not mathmatically correct is that only 1 character is allowed for the operator symbol, but I suppose 2 character Python style operators <= and >= could be supported aswell without too much trouble.

[slook]

@defaultxr Are you able to clone this development branch for testing? If possible, your feedback on this would be most welcome before PR #2238 is considered for merging into master.

git clone https://github.com/slook/nicotine-plus.git --branch filter-operator-sep --depth 1
cd nicotine-plus
./nicotine

[defaultxr]

Sure. Testing as per your instructions and the bitrate filter definitely looks like it's working how I expected it to. Makes it a lot easier to find V0 mp3 in search results and such. I would be very pleased if this behavior became the new standard behavior in the master branch.

[slook]

Thanks @defaultxr that's good to hear.

I have further refined the syntax to enable any amount of spaces, ampersands or pipes can be entered between multiple filter conditions without breaking the field or creating duplicate history entries with different ||| spacing as it was doing before. Also, now one space after an operator can be entered before the digit without it being orphaned, allowing for a much more flexible syntax.

To test, update the development branch with:

cd nicotine-plus
git pull
./nicotine

[defaultxr]

That seems to work as well; < 320 > 192 works as expected. I also tried < 320 | > 192 and similar. Though I'm not sure if it's expected behavior or not but | doesn't seem to work as an or operator like I was expecting; for example, > 320 | < 192 doesn't show any results, even though there are plenty of files with > 320 bitrate in the non-filtered results, as well as plenty that are < 192. If | isn't meant to be an or then that's fine with me, I would still be happy with the behavior as it is in the filter-operator-sep branch right now as finding V0 mp3s more easily was my primary use-case (and I can't really imagine why someone would want to search "greater than 320 or less than 192" in reality).

[slook]

| doesn't seem to work as an OR operator

No, that's right. This behaviour has not changed since the numeric filters have had ability to concatonate multiple conditions together, the < and > have not been able to exclude a range, only to include a range.

I can't really imagine why someone would want to search "greater than 320 or less than 192" in reality

Agreed, while there might be a valid reason for excluding a range in some edge case scenario, it would require somebody much smarter than myself to work on this, and I don't think the extra complexity that would be needed is warranted, because the ! (not) operator does provide a satisfactory method of excluding particular values.

[defaultxr]

Makes sense. Then I would say the current behavior on this branch definitely seems good to me.

[slook]

Multiple (space-separated) entries in the Bitrate field should be AND-ed together like they are in the other filters.

@defaultxr The development branch is just updated again, perhaps it will be the final testing of the space seperators before merging it into master.

git clone https://github.com/slook/nicotine-plus.git --branch filter-operator-sep --depth 1
cd nicotine-plus
./nicotine

[defaultxr]

Tested, and it looks good to me! 👍 Looking forward to the official release with it :)

[slook]

@defaultxr After review discussion in #2238 it is changed slightly again, update with git pull to test.

• Changed: Exclude non-audio files (with zero duration or bitrate) when using digit based filter (unless exact match of zero)

What this means is that if you filtered for example <=128 it would inlcude also non-audio files with a bitrate of 0, which didn't feel right.

Whereas now, only files that are 1 Kbps or more are shown when using the filter. Hopefully this doesn't break anything.

[defaultxr]

Pulled and tested, and the bitrate filter still seems to work as expected. I also tested filtering by length and it looks like that is working for me as well.

[slook]

What do you think of the factory presets in the combobox?

"filterbr": ("!0", "128 <=256", "192 <=320", "=320", ">320"),

Maybe the last one should be >=320 instead of >320 not sure.

[mathiascode]

My intention was to have a filter for lossless files, hence >320.

[slook]

Indeed, and that is a good idea, however if one considers anything that is 320Kbps or above to be a minimum standard, then there is no preset for that in the proposed list of factory Bitrate presets.

[mathiascode]

I would assume people either want lossless or not, depending on if they care about quality or disk space. For a high quality filter containing both lossy and lossless files, >=320 isn't that good today, when you have newer audio formats with equivalent quality at a lower bitrate.

[slook]

That is true, however on the other hand since recent versions of LAME encoding, when used for electronic genres which are popular with our users such as house and techno, then 320Kbps MP3 files are considered to be indistinguishable from lossless, even on high end sound systems.

The fact remains that many older models in the CDJ range are still in use and they only support either MP3 or WAV, not FLAC.

I am concerned that if any users have >320 set as a default filter, then they will suddenly see less results than was previously expected as a result of recent changes to the math used in the bitrate filter.

Suggest:
"filterbr": ("!0", "128 <=256", "192 <=320", "320 <=768", ">768"),

An obviously different digit, such as >512 or >768 for example, would avoid any doubt if the value 320 is inclusive or exclusive.

[defaultxr]

What do you think of the factory presets in the combobox?

If you're asking me, I think having one for mp3 V0 files would be nice. I usually search >192 <320 for that. 192 <=320 isn't quite the same since it finds a lot more CBR files than >192 <320 does, as 192kbps is a pretty common CBR bitrate. 256kbps is a relatively common CBR bitrate as well, but as far as I'm aware, >192 <320 is the best way to locate V0 mp3s.

Otherwise, I pulled again and checked and everything still seems to be working fine for me.

[slook]

"filterbr": ("!0", "128 <=192", ">192 <320", "=320", ">320"),

[mathiascode]

@slook This works for me.

I am concerned that if any users have >320 set as a default filter, then they will suddenly see less results than was previously expected as a result of recent changes to the math used in the bitrate filter.

I don't think this will be a large issue, considering 320 was a preset before. We are probably breaking behavior in some other cases anyway, and we'll have to mention this in the release notes and bump the version accordingly.