-
Notifications
You must be signed in to change notification settings - Fork 45
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
Check audio codec on uploaded videos #1414
Conversation
Your Testserver will be ready at https://1414.test.live.mm.rbg.tum.de in a few minutes. Logins
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!! I added a couple ideas on your code :)
worker/worker/ffmegtools.go
Outdated
probe, err := probe(file) | ||
if err != nil { | ||
return "", err | ||
} | ||
return gjson.Get(probe, "streams.0.codec_name").String(), nil | ||
codecNumber := gjson.Get(probe, "streams.#").Int() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a more descriptive name would be nice, something like
codecNumber := gjson.Get(probe, "streams.#").Int() | |
nStreams := gjson.Get(probe, "streams.#").Int() |
worker/worker/ffmegtools.go
Outdated
videoIndex = int(gjson.Get(probe, fmt.Sprintf("streams.%d.index", i)).Int()) | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead we could just return here
videoIndex = int(gjson.Get(probe, fmt.Sprintf("streams.%d.index", i)).Int()) | |
break | |
return gjson.Get(probe, fmt.Sprintf("streams.%d.codec_name", i)).String(), nil |
worker/worker/ffmegtools.go
Outdated
return "", errors.New("no video stream found") | ||
} | ||
|
||
func getAudioCodec(file string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could combine these methods into one, something like this:
func getAudioCodec(file string) (string, error) { | |
// getCodec returns the codec of the file, codecType=video returns the video codec, codecType=audio the audio codec. | |
func getCodec(file string, codecType string) (string, error) { |
worker/worker/ffmegtools.go
Outdated
if codecType == "video" { | ||
for i := 0; i < int(nStreams); i++ { | ||
if gjson.Get(probe, fmt.Sprintf("streams.%d.codec_type", i)).String() == "video" { | ||
return gjson.Get(probe, fmt.Sprintf("streams.%d.codec_name", i)).String(), nil | ||
} | ||
} | ||
return "", errors.New("no video stream found") | ||
} | ||
if codecType == "audio" { | ||
for i := 0; i < int(nStreams); i++ { | ||
if gjson.Get(probe, fmt.Sprintf("streams.%d.codec_type", i)).String() == "audio" { | ||
return gjson.Get(probe, fmt.Sprintf("streams.%d.codec_name", i)).String(), nil | ||
} | ||
} | ||
return "", errors.New("no audio stream found") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should do the exact same, no?
if codecType == "video" { | |
for i := 0; i < int(nStreams); i++ { | |
if gjson.Get(probe, fmt.Sprintf("streams.%d.codec_type", i)).String() == "video" { | |
return gjson.Get(probe, fmt.Sprintf("streams.%d.codec_name", i)).String(), nil | |
} | |
} | |
return "", errors.New("no video stream found") | |
} | |
if codecType == "audio" { | |
for i := 0; i < int(nStreams); i++ { | |
if gjson.Get(probe, fmt.Sprintf("streams.%d.codec_type", i)).String() == "audio" { | |
return gjson.Get(probe, fmt.Sprintf("streams.%d.codec_name", i)).String(), nil | |
} | |
} | |
return "", errors.New("no audio stream found") | |
for i := 0; i < int(nStreams); i++ { | |
if gjson.Get(probe, fmt.Sprintf("streams.%d.codec_type", i)).String() == codecType { | |
return gjson.Get(probe, fmt.Sprintf("streams.%d.codec_name", i)).String(), nil | |
} | |
} | |
return "", fmt.Errorf("no %s stream found", streamType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah you are right, this is much more compact than my version. I will push this change before today's meeting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Niceee 🔥 Thanks!!
Motivation and Context
closes #1336
Description
Audio codec is now also checked during filavigate to a Livestreame upload and if it is not in aac format it will be transcoded.
Steps for Testing
Screenshots