-
Notifications
You must be signed in to change notification settings - Fork 751
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
Refactor feature tools to share CLI & video input #42
Comments
@tdaede : I believe there are 6 executables that existed for sorts of different reasons (some are historic and some are not): The duplicate code you are referring to I believe are those residing in xxx.c/xxx(.) where xxx is moment, psnr, ssim, .... The refactoring of these code has sit on my to-do list for too long. How about let me do a quick refactoring of them before you continue? Is there anything else? |
@tdaede : now I see that your main issue is that for y4m input, you want to change the interface such that you don't need to input width, height or yuv format. That's why these _main.c files matters. I am actually wondering if there is a light-weight solution for your integration into AWCY. Is it possible to 1) pre-read the header of the y4m file to get the width and height and etc, and 2) create a binary file that take the y4m as input and outputs the y4m payload (essentially yuv) and pipe that to vmafossexec. The advantage of this approach is that it won't over-complicate other users who don't need y4m as input, and the disadvantage is that there needs to be one more process created (I don't know how much concern it would be). What do you think? |
Yeah, you can refactor first. Piping is a good idea - I didn't realize that vmafossexec did all of the metrics in one pass (in combo.c), which is why I thought piping wouldn't work. The In fact vmafossexec is much easier to modify for y4m than the Python one, I probably should have started with it first. The process overhead isn't a concern for AWCY, so I'll look at the pipe method first. |
@tdaede : since the refactoring is non-blocker for the pipe method, I will prioritize other things first. Please go ahead and try the pipe method, and let me know if you need anything else. |
@tdaede not sure if you have done this, but now |
I did something very similar, with a y4m2yuv tool and pipes: https://github.com/xiph/daala/blob/master/tools/y4m2yuv.c I might still like to write native support at some point, but this suffices for now. |
Adding y4m support is ending up a complex undertaking because there is a lot of duplicate code in both .c and _main.c files for reading yuv files and parsing the CLI. It would be a lot nicer to refactor all of this duplicate code into one place.
One place to start would be to drop the individual binaries and always use the main vmaf binary. Are the individual ones needed for anything?
The text was updated successfully, but these errors were encountered: