search

h4tr3d / avcpp

C++ wrapper for FFmpeg
Other
459 stars 80 forks source link

`api2-decode-encode-video.cpp` example and correct stream tear-down sequence #113

Closed mmomtchev closed 1 year ago

mmomtchev commented 1 year ago

I am using avcpp to create Node.js bindings for ffmpeg as a PoC for nobind

It is a very good C++ wrapper and it would be a pity to leave it to fall into disrepair.

It seems that the example/api2-samples/api2-decode-encode-video.cpp does not work with ffmpeg 5.0/6.0.

It does not set the PTS/DTS when writing and ffmpeg complains and (sometimes) eventually chokes.

I added some rudimentary PTS/DTS calculations - they should probably go into the example - and now the problem is the flushing of the last frame.

Looking from the code: https://github.com/h4tr3d/avcpp/blob/4aaaaa11d18f5f4d2e364796dffada105943ba48/example/api2-samples/api2-decode-encode-video.cpp#L158

This expects that an EOF is signaled by decode returning a falsy frame. However it does not do that - at the last packet, readPacket returns a falsy packet, but when pusing it through the codec, it returns one more valid frame. When trying to read again, readPacket returns another falsy packet and this time decode throws an EOF error - which probably means that it shouldn't have been called at this point.

Do you know what is the correct sequence to tear down the stream?

mmomtchev commented 1 year ago

This is what I do to manually count the frames w/ resynchronization on the input stream when it is possible: https://github.com/h4tr3d/avcpp/compare/master...mmomtchev:avcpp:example-pts-dts

mmomtchev commented 1 year ago

There are two separate issues that prevent the clean teardown

First one: https://github.com/h4tr3d/avcpp/blob/4aaaaa11d18f5f4d2e364796dffada105943ba48/src/codeccontext.cpp#L953

There are two possible sources for the timebase here - either from the current frame - which works for all frames except the last one where the encoder is called with an empty frame - either from m_stream when m_stream.isValid(). However the encoder is not constructed with the stream - only the decoder is. The encoder must use its own timebase field here - and all three options must be mutually exclusive - currently a valid stream will replace the timebase of the frame.

Second one: https://github.com/h4tr3d/avcpp/blob/4aaaaa11d18f5f4d2e364796dffada105943ba48/src/codeccontext.cpp#L85

This should be: if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF) just like in the function above and below it

h4tr3d commented 1 year ago

Thanks for reporting and fix!