Skip to content

add bitrate information to the QualityDetailsBtn.vue screen#1754

Open
Bonusbartus wants to merge 3 commits into
music-assistant:mainfrom
Bonusbartus:add_bitrate_to_qualitydetails
Open

add bitrate information to the QualityDetailsBtn.vue screen#1754
Bonusbartus wants to merge 3 commits into
music-assistant:mainfrom
Bonusbartus:add_bitrate_to_qualitydetails

Conversation

@Bonusbartus

Copy link
Copy Markdown
Contributor

add bitrate information to the QualityDetailsBtn.vue screen, but only…when input is encoded with a lossy codec
information is inserted between samplerate and bitdepth.

@OzGav

OzGav commented May 6, 2026

Copy link
Copy Markdown
Contributor

As I think I mentioned previously, bit_rate on AudioFormat is not guaranteed to be set — it’s provider-dependent, not calculated upstream.

Tying the kbps row purely to inputQualityTier (GOOD/LOW) is unsafe — a Deezer or YouTube Music stream (for example) classified in those tiers will still have bit_rate as None/0, and .toFixed(0) will then render 0 kbps or throw an error.

Also why did you choose to put bitrate in the middle? Sample rate and bit depth typically go together.

@Bonusbartus

Copy link
Copy Markdown
Contributor Author

As I think I mentioned previously, bit_rate on AudioFormat is not guaranteed to be set — it’s provider-dependent, not calculated upstream.

Tying the kbps row purely to inputQualityTier (GOOD/LOW) is unsafe — a Deezer or YouTube Music stream (for example) classified in those tiers will still have bit_rate as None/0, and .toFixed(0) will then render 0 kbps or throw an error.

You are right, I lookad at this piece of code:

else if (sd.audio_format.bit_rate >= 256) {
    return QualityTier.GOOD;
  } else {
    return QualityTier.LOW;

and somehow assumed figured because of the first check that the bit_rate had to be set to something after all, and that the fall through was going to QualityTier.Unknown..
I think with this in mind, I'll change the logic to a separate function generating a boolean to keep the template v-if readable.
otherwise I'd have to add two tests in v-if line.
What do you think?

Also why did you choose to put bitrate in the middle? Sample rate and bit depth typically go together.
That choice is more or less random. I figured for lossy codecs, the bitrate is more important/indicative of quality than the bit-depth. As bit-depth as far as I know will (almost?) always be 16 bit for mp3 and ogg etc.. Hence put it in front.
Also the values will be more or less from largest value to smallest. If you prefer another order I'd be happy to change it.
let me know what you think!

@OzGav

OzGav commented May 7, 2026

Copy link
Copy Markdown
Contributor

Personally I think it simple enough to keep inline

Also I would put bitrate on the end so the line reads consistently whether the bitrate is available or not (and as I mentioned sample rate and bit depth normally go together)

…samplerate kHz / bit_depth bits / bit_rate kbps with: / bit_rate kbps only shown when using a lossy codec and bit_rate information is available
@Bonusbartus

Copy link
Copy Markdown
Contributor Author

Personally I think it simple enough to keep inline

Also I would put bitrate on the end so the line reads consistently whether the bitrate is available or not (and as I mentioned sample rate and bit depth normally go together)

Done, I kept the checks inline, it only needed an additional AND (&& streamDetails.audio_format.bit_rate) which is the same check also performed in the more detailed pop-up window you can click/hover.
I also moved the bit-rate to the end of the string, as you suggested.

@OzGav

OzGav commented May 11, 2026

Copy link
Copy Markdown
Contributor

Just had a play and I wonder if people are going to ask why we show the bit rate on input but not output?

@Bonusbartus

Copy link
Copy Markdown
Contributor Author

Just had a play and I wonder if people are going to ask why we show the bit rate on input but not output?

That is a good question:
So for me that is the same as why I only added it for the lossy codecs: for lossless codecs I don't really care, as it doesn't tell you anything about the audio quality (for lossless only bit-depth and sample rate are important). In these cases the bitrate could be of interest while debugging maybe? Which is could be useful keep in the pop-up screen with more information.

As for the output side: My expectation was that the quality here should mostly be equal to or better than the input, and format is mostly chose by the output device or user directly?
if it makes sense to add the bit-rate here too, I have no issue adding it.

@OzGav

OzGav commented May 11, 2026

Copy link
Copy Markdown
Contributor

I was looking at it and thinking why do we have it on input and not output that is all. I thought you could use the same template so it shows at the same time as the input? Not 100% sure though so I will wait for others to have a look as well.

@Bonusbartus

Copy link
Copy Markdown
Contributor Author

yes, it will be very easy to add :) Let me know what others think of this.

@marcelveldt

Copy link
Copy Markdown
Member

Hmmm, not sure I'm following this. We already show bitrate today in the quality details (when hovering the [i]).
Why would this need to be in a dedicated row. If it is really that important to show it, I would show it in the same row as where we now show sample rate and bit depth and you can even argue that we just need to replace that row with just codec + bitrate as samplerate+bitdepth do not make any sense for lossy codecs.

Do note that I recently updated the backend so that we now dynamically read the actual bitrate from ffmpeg when its reading the input stream becaus eteh info provided by (streaming) providers was a bit hit and miss.

Technically we also have the bitrate for the output stream but we need a bit more plumbing to expose this to the frontend. I would say we leave that out of scope now

@Bonusbartus

Bonusbartus commented May 24, 2026

Copy link
Copy Markdown
Contributor Author

Hi @marcelveldt

If it is really that important to show it, I would show it in the same row as where we now show sample rate and bit depth

This is exactly what I added. Bitrate next to the samplerate and bitdepth, on the same line. The first iteration added it inbetween, but @OzGav suggested to keep the order of items as is.

and you can even argue that we just need to replace that row with just codec + bitrate as samplerate+bitdepth do not make any sense for lossy codecs.

I agree, hence the discussion here. I didn’t want to mess up the current scheme though and agree with @OzGav that keeping the order the same everywhere makes sense

Do note that I recently updated the backend so that we now dynamically read the actual bitrate from ffmpeg when its reading the input stream becaus eteh info provided by (streaming) providers was a bit hit and miss.

Technically we also have the bitrate for the output stream but we need a bit more plumbing to expose this to the frontend. I would say we leave that out of scope now

Yes, I thought about this as well. But as the user has more control over the output protocol and quality I figured it would not be as important.
To me the bitrate for lossy codecs was missing as it provides a very quick view of the quality you are currently listening to. Especially with local files that might need an upgrade to a higher quality version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants