bramp / ffmpeg-cli-wrapper

Java wrapper around the FFmpeg command line tool
BSD 2-Clause "Simplified" License
1.69k stars 413 forks source link

Add enum for video and audio codecs. #321

Closed van1164 closed 6 months ago

van1164 commented 6 months ago

@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

new FFmpegOutputBuilder().setFilename("output.mp4").setVideoCodec(VideoCodec.H264.codec)

What do you think?

Please reply.

van1164 commented 6 months ago

Thank you for the reply.

@bramp

  1. The code generation was implemented in a similar way to codecs() with python. If you like the Enum method, I will write a safer code.

  2. That's what I've been thinking. It's difficult for developers to know which codecs are available in IDE only with the codecs() function. I'm still thinking about how to leave comments on codecs that are not possible for each target system. So far, we still have to cross check through codecs(), but I think it's also important to let them know what codecs FFmpeg provides.

  3. If you can develop it like the picture below, I'm thinking of considering any way. Recommendations are also welcome. image

bramp commented 6 months ago

Ok cool. Thanks for the responses, so

1) If you could supply the python / simple java app, we can leave instructions for folks to run/update this in future.

2) Ok, so if you would add some documentation to the enum, I think that'll be sufficient.

3) Yes, that actually looks helpful when you show it within a IDE. So keeping it a Enum with a String codec property seems reasonable

@Euklios do you have any opinions on the above.

van1164 commented 6 months ago

@bramp Thanks for the responses 😁

Would document be okay as below? If you like it, I'll commit

image

, 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 😋

bramp commented 6 months ago

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.

van1164 commented 6 months ago

@bramp Thank you for your advice 😁.

I committed document and enum generator python code for enum.

Thank you.

Euklios commented 6 months ago

@bramp @van1164

An enum would convey a notion of "completeness" and "correctness"; neither can be guaranteed. Wouldn't a class containing public static final fields convey better that this is a common set of codecs?

bramp commented 6 months ago

Oh, that is a good point. So this maybe should be more like the AUDIOSAMPLE* constants. They are not complete, but are handy for folks use.

So yes, I think a public static final fields is better.

Euklios commented 6 months ago

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. As for the type, I don't have a preference, but we could reuse the existing Codec class. Do you have any thoughts on that one?

bramp commented 6 months ago

ah yes, keeping it out of FFmpeg sounds good to me. We could add it to Codec, but I suspect we are not making a full Codec instance here. So maybe two new class VideoCodec, and AudioCodec

van1164 commented 6 months ago

@bramp @Euklios

I changed the enum to a codec class with a string value. I also committed the python code that generates it.

How about applying it like this?

When using it, the picture is shown below image

bramp commented 6 months ago

The Build fails now with this error:

Error:  /home/runner/work/ffmpeg-cli-wrapper/ffmpeg-cli-wrapper/src/main/java/net/bramp/ffmpeg/builder/AudioCodec.java:97: error: bad HTML entity
Error:      /**ADPCM IMA Simon & Schuster Interactive*/

This seems a little strict, but I suspect it want's you to change & to &