Skip to content
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

[FIX] Temporal fix meanwhile odoo uses -intra option in ffmpeg. This … #599

Closed
wants to merge 1 commit into from

Conversation

josep-tecnativa
Copy link
Contributor

@josep-tecnativa josep-tecnativa commented Feb 28, 2024

@pedrobaeza
Copy link
Member

Put the reference to Odoo code using that option.

 This option is not compatible with ffmpeg lasts versions
Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chore: Add some extra rationale to the commit message:

Debian bookworm and upper versions use version 5 of ffmpeg (used in video test tours artifacts generation) which doesn't support the intra flag used by Odoo (link).

This options has been deprecated for long time and we've proposed a fix (link to pr) for a modern alternative. While these changes aren't incorporated we'll force the version to support the current options.

@ap-wtioit
Copy link
Contributor

ap-wtioit commented Feb 29, 2024

It seems Odoo is also working on / proposed a fix for this: odoo/odoo#153270

@chienandalu
Copy link
Member

Thanks @ap-wtioit !

@chienandalu
Copy link
Member

Once that Odoo PR is merged maybe we should add that test to the doodba CI

@josep-tecnativa
Copy link
Contributor Author

Once that Odoo PR is merged maybe we should add that test to the doodba CI

Meanwhile, I'm working in this PR as a temportal patch. Now I'am facing some compatibility issues between 4.x ffmepg version and bookworm, so we can't merge this PR yet.

@ap-wtioit
Copy link
Contributor

Meanwhile, I'm working in this PR as a temportal patch. Now I'am facing some compatibility issues between 4.x ffmepg version and bookworm, so we can't merge this PR yet.

Another fix, would be to add a script ffmpeg into the odoo users path that accepts the subprocess.run([ffmpeg_path, '-intra', '-f', 'concat','-safe', '0', '-i', concat_script_path, '-pix_fmt', 'yuv420p', outfile]) call and converts it to subprocess.run([ffmpeg_path, '-f', 'concat', '-safe', '0', '-i', concat_script_path, '-pix_fmt', 'yuv420p', '-g', '0', outfile], check=True).

E.g. (not tested):

#!/usr/bin/env bash
if [[ $1 == "-intra" ]] ; then
  # discard first argument -intra
  shift
fi
exec /usr/bin/ffmpeg "$@"

if you put this as ffmpeg into /home/odoo/.local/bin as ffmpeg it should remove the -intra parameter before executing the real ffmpeg. if we also need the added -g 0 i can help you with the wrapper (On monday).

@pedrobaeza
Copy link
Member

Let's wait for the following days to have the Odoo PR merged, and if no response, then let's go to alternative solutions.

@chienandalu
Copy link
Member

The fix is already merged in 16.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants