Set AAC codec for audio in mp4 files, add transcoding utility

[achille-fouilleul]

When the output format is mp4, pyav seems to reject wav-format audio. Convert to AAC.

Also, certain versions of pyav reject frame rates specified as float. Ensure frame_rate has type Fraction.

[behackl]

I am somewhat sure that I've tried aac-transcoding to fix the documentation build issue at some point yesterday in the context of #3953. But even in case it still isn't fixed, this is probably a good idea; I am in favor of this change. 👍

(Crossing fingers for the docbuild issue too ...)

[behackl]

Please know that you have made my day; thank you very much!

I am still not quite sure why the handling of pcm_s16le-encoded audio in mp4s would have changed in the latest major release of av, but this solution now looks and behaves quite robust, at least on the devices I just tested this on.

Left two minor questions, please take a look -- otherwise this is good to go. Again, thank you!

[achille-fouilleul]

I think I understand your earlier comments a bit better.

TIL .mp4 (MPEG-4 Part 14) was standardized from .mov (QuickTime File Format). While .mp4 and .mov may overlap to a large extent, this is not true with other formats like .webm or .gif.

That both the output file suffix and config.format (and also config.transparent) influence the output is confusing. The test suite may be passing but the logic is not correct. A bit more work is needed to clean this up.

[behackl]

TIL .mp4 (MPEG-4 Part 14) was standardized from .mov (QuickTime File Format). While .mp4 and .mov may overlap to a large extent, this is not true with other formats like .webm or .gif.

The key difference is that .mp4 containers do not support transparency, which is why at some point we introduced this "automatic" format switch if transparency is requested.

That both the output file suffix and config.format (and also config.transparent) influence the output is confusing. The test suite may be passing but the logic is not correct. A bit more work is needed to clean this up.

I think this deserves to be cleaned up; following the Python idiom of "explicit is better than implicit", I think requesting transparency with an incompatible container format should just raise an error. (We have started working on making the config less confusing and more inherently consistent in the context of #3466.)

For this PR it would be sufficient to check the currently constructed format based on config.movie_file_extension instead of out_suffix / config.format.

[achille-fouilleul]

OK. Still, I think using config.format directly is wrong. For example:

        if config.format == "webm":
            partial_movie_file_codec = "libvpx-vp9"

Suppose config.format is None but the output file suffix is .webm. The "then" branch should be taken.
I will add a simple conversion function and do a consistency check in it.

[JasonGrace2282]

Thanks for the PR! I just noticed/had some questions about a few things as I was poking around.

[JasonGrace2282]

Out of curiosity, where did these numbers come from?

[achille-fouilleul]

Maybe I should have added a comment about these epsilons.
pyav wants fractions but at this point in the code all we have is a float. Though, strictly speaking, IEEE 754 floats are fractions, converting from floats to meaningful fractions is not obvious. I say meaningful because floats can only represent powers of two so something like (50 * 2 / 3) does not have an exact representation in binary. For (50 * 2 / 3), Fraction.from_float does not return Fraction(100, 3), as one would naïvely expect:

>>> from fractions import Fraction
>>> Fraction.from_float(50 * 2 / 3)
Fraction(4691249611844267, 140737488355328)

If a frame rate is obtained via some calculation, implicit float rounding rules may result in frame rate values like 49.999... instead of 50. Fraction.from_float returns:

>>> Fraction(50.0 - 2**-47)
Fraction(7036874417766399, 140737488355328)

Using this value rather than Fraction(50, 1) could lead to subtle issues.

I don't think this problem may be solved in general. I made the assumption that frame rates would be either whole values or multiples of 1000 / 1001 (because NTSC) specified with two decimals.
I chose epsilon1 and epsilon2 such that:

• the epsilon1 check accepts 59.99999... as 60 but rejects 59.94
• the epsilon2 check accepts common multiples of (1000/1001)

If the need to support other fractions arises, this function will have to be revisited.

[behackl]

I've pushed two changes -- assuming the pipeline / docbuild is still happy with this, I'll merge it. Further fixes and improvements can be made in follow-up PRs, if necessary.

[behackl]

Looks like we are back in the luxurious world where we have a working docbuild process. Thanks again for your efforts @achille-fouilleul, much appreciated!

[achille-fouilleul]

I'm glad it helped.