-
-
Notifications
You must be signed in to change notification settings - Fork 413
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
Add enum for video and audio codecs. #321
Conversation
add enum to make it easier for developers to access codecs.
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.
So this seems fine in principle, but I do have a couple of comments/concerns
-
How is the list of codecs generated? How can we easily keep it updated?
-
Not all the codecs will be available on the target system, and I'd prefer folks call
FFmpeg.codecs()
instead to figure out what's available. So maybe if these enums are commented to explain that these may not be available, and the caller should cross check withcodecs()
or run the risk of their command failing. -
I wonder if this should be a list of
Codec
objects, instead of Strings.
Ok cool. Thanks for the responses, so
@Euklios do you have any opinions on the above. |
@bramp Thanks for the responses 😁 Would document be okay as below? If you like it, I'll commit , and this is the python code for the above results. import subprocess
import re
CODECS_REGEX = re.compile("^ ([.D][.E][VASD][.I][.L][.S]) (\S{2,})\s+(.*)$")
def removeLeadingNumbers(text):
index = 0
while index < len(text) and text[index].isdigit():
index += 1
return text[index:]
def writeCodec(m,codec):
document = "\t"+"/**"+ m.group(3).rstrip() +"*/\n"
enumCode = "\t" +removeLeadingNumbers(m.group(2).replace(".","_")).upper() +'("'+ m.group(2) +'"),' +'\n'
codec.write(document)
codec.write(enumCode)
output = subprocess.check_output("ffmpeg -codecs", shell=True).decode('utf-8')
print(output)
output = output.split("\n")
videoCodec = open("VideoCodec.java", 'w')
audioCodec = open("AudioCodec.java", 'w')
videoCodec.write(
"""package net.bramp.ffmpeg.builder;
public enum VideoCodec {
""")
audioCodec.write(
"""package net.bramp.ffmpeg.builder;
public enum AudioCodec {
""")
for item in output:
m = CODECS_REGEX.match(item)
if not m : continue
lst = item.split()
if 'A' in m.group(1):
writeCodec(m,audioCodec)
if 'V' in m.group(1):
writeCodec(m,videoCodec)
videoCodec.write("""
;
final String codec;
VideoCodec(String codec) {
this.codec = codec;
}
@Override
public String toString() {
return this.codec;
}
}
""")
audioCodec.write("""
;
final String codec;
AudioCodec(String codec) {
this.codec = codec;
}
@Override
public String toString() {
return this.codec;
}
}
""")
videoCodec.close()
audioCodec.close() Thank you for reading 😋 |
Thanks, that documentation looks good. I also think a comment on the enum itself saying to use FFmpeg.codec() for a accurate list for your deployment. As for the python I think it should be checked in, under /tools/generate_codec_enum.py or similar. Then we can update the README to mention it. |
@bramp I committed document and enum generator python code for enum. Thank you. |
Oh, that is a good point. So this maybe should be more like the AUDIO_SAMPLE_* constants. They are not complete, but are handy for folks use. So yes, I think a |
So that we're on the same page: My thought was something like class VideoCodecs {
public static final ____ HEVC = new ____ Keeping it out of FFmpeg is a good idea. |
ah yes, keeping it out of |
The Build fails now with this error:
This seems a little strict, but I suspect it want's you to change & to & |
@@ -0,0 +1,69 @@ | |||
import subprocess |
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.
A quick comment at the top saying how to run this tool, and a pointer to the file it generates, would be useful.
Imagine a year from now, someone coming in code, having to run this.
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 didn't know there would be a problem with & at javadoc . 😅 I modified it in codec files and generator.
And I added a description at the top of the generator.
Thank you.
@bramp
Hi. Thank you for using your library well.
I want to add Enum to make it easier to identify and add codecs in IDE.
like
What do you think?
Please reply.