Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the bs1770gain defaults + true peak #1

Closed
WhitePeter opened this issue Aug 13, 2016 · 16 comments
Closed

Use the bs1770gain defaults + true peak #1

WhitePeter opened this issue Aug 13, 2016 · 16 comments

Comments

@WhitePeter
Copy link
Collaborator

WhitePeter commented Aug 13, 2016

Hi there,

First of all, thanks for this neat project! I just checked out your script. Still reading...

But I found one thing which I'd call over-descriptive. You ask bs1770gain to calculate the calculation method for "integrated" loudness and output EBU R 128 compliant values. Both are the set defaults of bs1770gain, see bs1770gain -h for reference.
Also, using sample peak (-p) is deprecated, if I understand the EBU document correctly. True peak is what they describe and it is supported by bs1770gain (-t).

Since I am very new to github and haven't set up my own repo yet, I append a patch which I think should make this EBU R 128 compliant. I also think the range could be calculated, who knows what it's worth, but why not, while we're at it. ;)

--- a/mkvrg
+++ b/mkvrg
@@ -116,8 +116,8 @@ find "$@" -type f -size "$MINSIZE" -iregex ".*\.\(mk[av]\|mk3d\)$" -print0 | whi
         ((trackindex++))

         echo "INFO: Running bs1770gain, this can take a while. (track $track on file '$file')"
-        RGINFO=$(bs1770gain --audio "$track" --ebu -ip "$file" | tee /dev/stderr | tr "\r\n" " " | grep -Poi "[-\d.]+\s+LU.+?\[ALBUM")
-        TRACKGAIN=$(echo "$RGINFO" | grep -Poi "[-\d.]+\s+LU\s+sample" | cut -d\  -f1)
+        RGINFO=$(bs1770gain --audio "$track" -t "$file" | tee /dev/stderr | tr "\r\n" " " | grep -Poi "[-\d.]+\s+LU.+?\[ALBUM")
+        TRACKGAIN=$(echo "$RGINFO" | grep -Poi "[-\d.]+\s+LU\s+true" | cut -d\  -f1)
         TRACKPEAK=$(echo "$RGINFO" | grep -Poi "[-\d.]+\s+\[ALBUM" | cut -d\  -f1)
         if [[ $TRACKGAIN == "" ]] || [[ $TRACKPEAK == "" ]]; then
             echo -e "\e[92mNOTICE: Problem finding replaygain info from bs1770gain for track $track on file '$file'.\e[0m"

Final edit (hopefully): Get rid off options that are default anyway, and unlikely to change. And use true peak calculation instead of sample peak.

@kevinlekiller
Copy link
Owner

Alright, not too aware of the specs, that was something I was wondering about, thanks.

@kevinlekiller
Copy link
Owner

I think the issue with not setting --ebu is if bsgain1770 changes the default to something else in the future, the -23db hardcoded in the xml won't be good anymore, unless we parse the value every time the script starts.

@kevinlekiller
Copy link
Owner

"I also think the range could be calculated, who knows what it's worth, but why not, while we're at it. ;)"

At first I was parsing the range, but I looked at the mpv source and only noticed peak / track was used so I removed it thinking it was useless.

But I agree with you, I think the more info the better.

@WhitePeter
Copy link
Collaborator Author

OK, I can see what you mean. Thought about that too, but I think that is unlikely. Anyway, I think keeping --ebu is actually more robust, as you say.

@kevinlekiller
Copy link
Owner

I could parse the default out of this this: bs1770gain --help

Which I think is a better idea, I agree with you, sticking with defaults is better if we can.

@WhitePeter
Copy link
Collaborator Author

But then you'd have to "know" the underlying reference loudness. I can see bloat there. ;)

@WhitePeter
Copy link
Collaborator Author

Oh, I see, the reference is also stated in the help.

@WhitePeter
Copy link
Collaborator Author

BTW, do you think this could be done in, say, python? bs1770gain is also provided for Windows. Obviously, running bash scripts there is quite a hassle. Not that I would care too much, I don't use that platform. Just a thought.

@kevinlekiller
Copy link
Owner

Yeah, although I've not written any python in a few years, so I'd have to catch up.

Then maybe it can be threaded also.

@WhitePeter
Copy link
Collaborator Author

WhitePeter commented Aug 13, 2016

I, myself know very little about python programming, although I did some coding at Checkio. Maybe this is the right size project to get my feet really wet. Thinking... :)

Edit: I think I need a break, my English is getting worse rather quickly.

@kevinlekiller
Copy link
Owner

Oh yeah, small projects are great to start.

Jetbrains has a free community version of their IDE for python (pycharm), if you're looking for good IDE.

@WhitePeter
Copy link
Collaborator Author

Thanks, I'll check it out.

kevinlekiller pushed a commit that referenced this issue Aug 13, 2016
Get reference loudness instead of hardcoding it, also allows replaygain implementation to be picked automatically by bs1770gain.
Use truepeak instead of samplepeak.
Calculate range.
Copy track info to album info (so mpv can parse the replaygain tags).
@kevinlekiller
Copy link
Owner

Peter just released bs1770gain v0.4.11 with the multi track fixes.

@WhitePeter
Copy link
Collaborator Author

WhitePeter commented Aug 13, 2016

Oh, is it a release already? Just found the beta2 on sourceforge. Thanks for the info and congrats on the credit. :)

@kevinlekiller
Copy link
Owner

wm4 also pushed to mpv to make album tags optional, I will remove the copying of album tags, since it doesn't make sense to have them now.

kevinlekiller referenced this issue Aug 13, 2016
mpv now can read track tags without album tags.
@WhitePeter
Copy link
Collaborator Author

On that note, this issue can be closed. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants