search

erikkaashoek / Comskip

A free commercial detector
GNU General Public License v2.0
598 stars 80 forks source link

Update to modern ffmpeg codec APIs #152

Closed rcombs closed 1 year ago

rcombs commented 2 years ago

Partly derived from #150; fixes #142. Seems to work in my experimenting, but I don't have a broad test suite for this; in particular, this likely breaks some of the AC3-specific code (I have no idea what that code is supposed to do, whether or not it's still relevant with modern ffmpeg, or how to test it).

This works with both lavf/lavc≤58 and lavf/lavc≥59, but requires at least lavf 57.33.100 / lavc 57.37.100 (versions from 2016).

tmm1 commented 2 years ago

Thanks for tackling this!

The simple test in CI passes so that's definitely encouraging.

I think @erikkaashoek has a more extensive suite locally that may surface any ac3 issues.

tmm1 commented 2 years ago

@erikkaashoek are you able to try this out?

erikkaashoek commented 2 years ago

Yes, but will take some days before I have time

marhmm commented 2 years ago

Hello, this is the only build compiling on my arm64 device with latest debian docker image. But when running it crashes with a segmentation fault. After some testing I found the place responsible for this crash: mpeg2dec.c, Line 571: for (l=0;l < is->audio_st->codecpar->channels;l++ ) volume += *((fb[l])++) * 64000;

When commenting this line out (or when always evaluating the if statement in Line 570 to false), the program is running fine and even finding commercials without a problem.

Is this an issue caused by your changes or a different problem? Any ideas what could be the reason for it?

tmm1 commented 1 year ago

Merging this because master is pretty useless with last couple ffmpeg releases.