Conversation
| { | ||
| arguments | ||
| .Add($"-b:a:{lastAudioStreamIndex++}") | ||
| .Add(Math.Round(audioStreamInfo.Bitrate.KiloBitsPerSecond) + "K"); |
There was a problem hiding this comment.
Perhaps it's worth noting I didn't investigate whether audioStreamInfo.Bitrate.KiloBitsPerSecond is used elsewhere. It seems it's still useful, at least, to implementations using YoutubeExplode, right? So perhaps no further refactoring is necessary.
There was a problem hiding this comment.
Are you asking on the account of whether this property should be kept or removed? In that case, yes, it should be kept since it's part of YoutubeExplode's public API.
|
file Hdo56B1YxT4.opus ffmpeg -y -threads 3 -i "Hdo56B1YxT4.opus" -vn -ar 48000 -b:a 128k "Hdo56B1YxT4.mp3" ffmpeg -y -threads 3 -i "Hdo56B1YxT4.opus" -vn -ar 48000 -q:a 128k "Hdo56B1YxT4.mp3" Of which, -b:a is closer to opus than -q:a for the same processing. Of course, with -b:a the size is larger but the quality is better. |
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes MP3 audio degradation issues (issue #926) by changing the FFmpeg encoding approach from bitrate-matching to quality-based encoding. The change prioritizes audio quality over file size, resulting in better output quality at the cost of approximately 66% larger file sizes.
Key Changes:
- Replaced FFmpeg's
-b:a(bitrate) parameter with-q:a 0(quality) parameter for MP3 encoding - Consolidated multi-line argument building into a single line
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks 🙏🏻 |
|
@avengerx I thought about it after merging, but do you think we could just do |
I wouldn't mind either way. Perhaps doing it in one shot would be better readability-wise, but I'd say it's more a matter of taste than anything. I can't tell a strong disadvantage either way. Well aside of your spent energy hitting the keyboard keys to make the new commit and pull request. :) Well, in short, I don't know! :P It might just work and save that loop! |
This closes #926 by using
-q:a:<stream> 0instead of-b:a:<stream> <input_stream_bitrate>Notice a "drawback" of this approach is, output mp3 files will have considerably higher size. For the "not-audiophile" people and large libraries, this might mean a considerable impact.
The expected average increase is of 2/3rs (66%), but it depends both on the input quality and input codec. Opus in particular should always mean bigger files as it usually packs better quality in smaller bitrates in comparison to lame/mp3.
Ideally YoutubeExplode should accept quality control on the MP3 output, but that's really falling into a "new feature" category and is probably not the focus anymore. Having the best quality possible leaves room for manually reducing the quality after the download if size matters over quality.
p.s.: it was not my decision to join the lines, Visual Studio done it on its own will. Shall we bend to IA's will this time? :)