Upstreaming FAAD2 Patches

[qcsv]

Hi, I want to package NRSC5 for Gentoo. Right now though, the build behavior of patching faad2 is not ideal, it would be best if these patches were upstreamed to FAAD2.

Are there any plans to do that?

[pclov3r]

There has been some talk of doing this but it seems the consensus is that it's not quite ready for the patch to be upstreamed yet.

[qcsv]

What needs to be done?

[argilo]

Unfortunately, the original author of the faad2 patch no longer contributes to nrsc5, and I don't understand the codec or the faad2 patch well myself. But my impression is that the code at present is somewhat hacky, and so may or may not be accepted upstream. I think we'd want to find someone who understands the codec and faad2 itself to help potentially clean up the patch and be around to answer any questions that the faad2 maintainers might have.

[TheDaChicken]

In my opinion, the faad2 patch isn't super hacky as much as before since I refactored it. I have thought of ways to improve the patch even more since I only refactored the syntax code.

[argilo]

True, it has improved recently. When last I looked at it, my one concern was whether it made sense for the HDC patch to piggy-back on the DRM / DRM_PS macros. But if HDC is very similar to DRM, then maybe it does make sense.

The patch also adds a couple messages logged to stderr, which should perhaps be removed.

I have also noticed robustness problems when fuzzing the decoder, but I'm not sure whether the bugs are pre-existing in faad2 or introduced in the patch.

[argilo]

@TheDaChicken Since you know the patch better than anyone else at this point, I trust your intuition on it. If you ever feel like taking a stab at upstreaming it, you're welcome to. We would of course want to credit everyone who has contributed to the patch.

[TheDaChicken]

When last I looked at it, my one concern was whether it made sense for the HDC patch to piggy-back on the DRM / DRM_PS macros

Some of that issue is faad2. Some of the DRM macros are a hack. There isn't many functions that split up work. My enhanced stream decoding patch refactored some of faad2 by splitting functions up. That may be good to add upstream. I don't want to create more bugs. I would need to be careful.

I have also noticed robustness problems when fuzzing the decoder, but I'm not sure whether the bugs are pre-existing in faad2 or introduced in the patch.

You should let me know what those are. If those are in faad2, I could fix them and upstream them?

@TheDaChicken Since you know the patch better than anyone else at this point, I trust your intuition on it. If you ever feel like taking a stab at upstreaming it, you're welcome to. We would of course want to credit everyone who has contributed to the patch.

I am not sure if we could upstream the patch at all, sadly. There is only one person who is active that has commit rights. They mentioned that they do not want to include proprietary stuff:

Honestly, this code being part of this repository inevitably implies a certain degree of responsibility for maintenance etc. As I am already unable to reply to most requests posted here against the regular code, I doubt that adding another code tree will improve the situation. Also, I think FMOD is proprietary code and I wouldn't want to advertise its use (e.g. over openal-soft) here.

Quote from: knik0/faad2#207 (comment)