Yahweasel / libav.js

This is a compilation of the libraries associated with handling audio and video in ffmpeg—libavformat, libavcodec, libavfilter, libavutil, libswresample, and libswscale—for emscripten, and thus the web.
291 stars 18 forks source link

Support of mp4 files encoding using libopenh264 for H.264 codec. #5

Closed pushpagarwal closed 2 years ago

pushpagarwal commented 2 years ago

1) Added two fragments libopenh264 and mp4 to support mp4 files. 2) Disabled building some programs which were not needed.

libopenh264: BSD 2-Clause "Simplified" License LICENCE

pushpagarwal commented 2 years ago

I have added only fragments as options but didn't modify any pre-defined configuration or release process.

What would you recommend for that?

Yahweasel commented 2 years ago

I had started to look into porting OpenH264, but didn't do it because it didn't fit my needs (it would still fall under patent, since their patent release only applies to binary releases). Glad to see the whole stack is working well enough that somebody else can do it!

A couple minor nits:

(1) The reason there isn't an "mp4" fragment is that there is an "ipod" fragment, and that's always what you want to use. The "ipod" format is just MP4 with fewer restrictions; if you use it with MP4 codecs, it creates bog-standard MP4 files (and it reads standard MP4 files regardless). So, there's no need for the mp4 format.

(2) You've got a patch to OpenH264 that adds a parameter to ForceIntraFrame, and a consummate patch to ffmpeg that adds the argument, but always as -1. Is this a mistake? Maybe something you had been working on but scrapped? The fewer patches are needed, the better.

Regarding pre-defined configurations, the best option would be to add a new one. I don't want anything with patent implications in any of the existing configurations. If you have a suggested configuration, I'll add it to the mkconfig scripts. Though it might be time to make the fragment/configuration system a bit more flexible, so the fragments don't have to be joined into a particular configuration to be usable...

pushpagarwal commented 2 years ago

Thanks for the review.

I was not aware about support of mp4 through ipod fragment. I will remove that fragment from the changes.

Patch in OpenH264 is necessary. It is due to a bug in OpenH264 code for C interface. FFMPEG is c code and get compiled as c code. OpenH264 is C++ library and has exposed C interface through a hack. Vtable of class is is exposed as structure of function pointers to C code.

There is no default arguments in C and default arguments in C++ is supplied by caller. Callee always expects all arguments. This causes problem in WASM for signature mismatch at run time and this patch fix everything with correct code. I would try to raise it with OpenH264 and get fixed.

Yahweasel commented 2 years ago

Ahh, of course, C++ Problems™. How obnoxious. Oh well :) . I shall await the mp4 (un)patch and then merge. Any suggestions for a configuration? (Presumably you're using some configuration yourself, so it at least helps you :) )

pushpagarwal commented 2 years ago

I have removed mp4 fragment. For configuration, I am using "mediarecorder-transcoder" + openh264 (in place of h264). H264 encoding was missing from that config, decoding was already there.

Yahweasel commented 2 years ago

See #6 . I'll be adding a config Soon™.

Yahweasel commented 1 year ago

The newest release has a config. Thanks!

pushpagarwal commented 1 year ago

Thanks @Yahweasel.