Skip to content

Transcoding: Use single oiiotool call for sequences, instead of frames one by one #1217

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

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Mar 31, 2025

Changelog Description

Use single oiiotool command and some other optimizations.

  • Prepare attributes to remove list just once.
  • Process sequences as a single oiiotool call
  • Enable --parallel-frames argument to oiiotool
    • Warning: This requires OIIO 2.5.2.0+ whereas ayon third party currently ships with ancient OIIO 2.3.10
  • Support sequences with holes by using dedicated --frames argument on oiiotool

Additional info

⚠️ WARNING ⚠️

This requires OIIO 2.5.2.0+ whereas ayon third party currently ships with ancient OIIO 2.3.10
I believe @antirotor has an OIIO 3+ build in the process suitable for ayon third party release.

✅ This should now be valid with the newer AYON Third Parties release 1.3.0 https://github.com/ynput/ayon-third-party/releases/tag/1.3.0


It seems a somewhat similar "deprecated" implementation existed in the codebase - it's unclear why excatly that was dropped.

Thoughts?

Testing notes:

  1. Publishing .exr with review should work.

- Prepare attributes to remove list just once.
- Process sequences as a single `oiiotool` call
@BigRoy BigRoy added community Issues and PRs coming from the community members type: enhancement Improvement of existing functionality or minor addition labels Mar 31, 2025
@BigRoy BigRoy self-assigned this Mar 31, 2025
@ynbot ynbot added the size/S label Mar 31, 2025
@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 1, 2025

If could update beyond 2.5.10 for OIIO, then we could also use the --parallel-frames argument here so that it's processing multiple frames in parallel within the one process.

BigRoy added 4 commits April 23, 2025 19:44
- Support ExtractReview convert to FFMPEG in one `oiiotool` call for sequences
- Support sequences with holes in both plug-ins by using dedicated `--frames` argument to `oiiotool` for more complex frame patterns.
- Add `--parallel-frames` argument to `oiiotool` to allow parallelizing more of the OIIO tool process, improving throughput. Note: This requires OIIO 2.5.2.0 or higher. See AcademySoftwareFoundation/OpenImageIO@f40f980
https://github.com/BigRoy/ayon-core into enhancement/oiio_transcode_parallel_frames

# Conflicts:
#	client/ayon_core/lib/transcoding.py
@BigRoy BigRoy requested review from kalisp and antirotor April 23, 2025 17:53
@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 23, 2025

If could update beyond 2.5.10 for OIIO, then we could also use the --parallel-frames argument here so that it's processing multiple frames in parallel within the one process.

As I've been testing with @antirotor with recent OIIO 3.x builds the performance in recent releases seems much better - and is even greatly improved beyond that with --parallel-frames implemented in OIIO with this commit in this PR which seems to have been in there since OIIO 2.5.2.0 this PR now implements the usage of --parallel-frames argument as well.

BigRoy added 2 commits April 25, 2025 09:03
…ancement/oiio_transcode_parallel_frames

# Conflicts:
#	client/ayon_core/plugins/publish/extract_color_transcode.py
@BigRoy BigRoy marked this pull request as ready for review April 25, 2025 07:29
@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 25, 2025

This now appears to work. As long as you're using a more recent oiio build than AYON third parties provides.
The speed improvement is significant however so if you're facing long running publishes (taking a long time on many oiiotool calls) then this will definitely make a difference.

@kalisp
Copy link
Member

kalisp commented Apr 28, 2025

Warning: This requires does this mean it will fail with current version of oiio, so this will need to get merged ONLY AFTER new ayon-third-party is created (and dependency bumped up in this PR)?

@BigRoy
Copy link
Collaborator Author

BigRoy commented Apr 29, 2025

Warning: This requires does this mean it will fail with current version of oiio, so this will need to get merged ONLY AFTER new ayon-third-party is created (and dependency bumped up in this PR)?

Correct - the old OIIO will complain about the --parallel-frames argument being unknown.

@jakubjezek001
Copy link
Member

related pr #1277

@BigRoy
Copy link
Collaborator Author

BigRoy commented May 19, 2025

@iLLiCiTiT should this PR make a 'soft' requirement dependency to ayon-third-parties 1.3.0?

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented May 20, 2025

@iLLiCiTiT should this PR make a 'soft' requirement dependency to ayon-third-parties 1.3.0?

If we can check for the OIIO version and use the "enhancement arguments" only if the arguments are supported by the version, then that would be prefered. If no, then soft requirement in package.py it is.

Something like this

def _get_oiio_version(oiiotool_path):
    """Get OIIO version from oiio tool.

    Args:
        oiiotool_path (str): Path to OIIO tool.

    Returns:
        tuple[int, int, int, int]: OIIO version.

    """
    version_parts = []
    try:
        output = subprocess.check_output(
            [oiiotool_path, "--version"],
            encoding="utf-8",
            stderr=subprocess.PIPE,
        )
        version_parts = [
            int(item)
            for item in output.split(".") if item
        ]
    except Exception:
        pass

    # Older versions of OIIO might not have '--version' argument
    if not version_parts:
        try:
            output = subprocess.check_output(
                [oiiotool_path, "--help"],
                encoding="utf-8",
                stderr=subprocess.PIPE,
            )
            lines = output.splitlines()
        except Exception:
            lines = []

        for line in lines:
            result = re.match(
                r"^OpenImageIO ([0-9]+\.[0-9]+\.[0-9]+(\.[0-9]+)?).*",
                line,
                flags=re.IGNORECASE,
            )
            groups = []
            if result is not None:
                groups = [i for i in result.groups() if i]

            if groups:
                version = "".join(groups)
                version_parts = [
                    int(item)
                    for item in version.split(".") if item
                ]
                break

    while len(version_parts) < 4:
        version_parts.append(0)
    return tuple(version_parts)

@BigRoy
Copy link
Collaborator Author

BigRoy commented May 20, 2025

If we can check for the OIIO version and use the "enhancement arguments" only if the arguments are supported by the version, then that would be prefered. If no, then soft requirement in package.py it is.

I personally feel that adding such a version check is just more hassle and will just add more slowness to the process due to the overhead. Basically running it more often is exactly the opposite of what we want to achieve, hehe. The OIIO version we were using was old. Like, really really old. Good riddance, really.

Maybe the better approach is to add the soft dependency and maybe wait a little longer before merging (as then third party package would've been released for a while).

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented May 21, 2025

I personally feel that adding such a version check is just more hassle and will just add more slowness to the process due to the overhead. Basically running it more often is exactly the opposite of what we want to achieve, hehe. The OIIO version we were using was old. Like, really really old. Good riddance, really.

It is not that slow, compared to real oiiotool conversion. And the version can be cached, so it's called only once.

Maybe the better approach is to add the soft dependency and maybe wait a little longer before merging (as then third party package would've been released for a while).

The new version of 3rd party addon was released, but not yet tested on all linux distributions we need to support. So adding soft dependency could mean that we might need freeze some studios on older ayon-core because of that.

@BigRoy
Copy link
Collaborator Author

BigRoy commented May 21, 2025

It is not that slow, compared to real oiiotool conversion. And the version can be cached, so it's called only once.

It's anywhere between 0.15 and 0.02 seconds for me (oiiotool served over network share) with the 0.13 being a cold run, and all other subsequent runs between 0.04-0.02 seconds from server cache over 10 Gbps connections.

The highest time I got was 0.2409 seconds - but that was cold cache + cold run (regex needing to compile perhaps?) - however, that's most likely the case if e.g. a machine just randomly starts to run this every now and then, like in production on a publish or on farm so likely closer to a production environment.

@antirotor
Copy link
Member

I don't really mind soft requirement. We did the same with ocio recently. It is already used in productions and so far no issues reported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues and PRs coming from the community members size/S type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants