Clear ISO C++ Warning and Enable ffmpeg V5.0 Support #14
Merged
SlavekB
merged 2 commits from ffmpeg-5.0 into master 2 years ago
SlavekB
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'ffmpeg-5.0'
Deleting a branch is permanent. It CANNOT be undone. Continue?
static char *and initialises them with string constants. This produces the compiler warning "ISO C++ forbids converting a string constant to 'char*'". Both declarations have been changed to "const".a0033d986dto1e526c47042 years agoI had to search around to find out how to actually get the ffmpeg plugin to run. Turns out, you need to convert a .wma file:
The output seems to be written to a directory entitled "unknown - unknown" as a .wav file. The conversion is giving a playable file.
WIP: Clear ISO C++ Warning and Enable ffmpeg V5.0 Supportto Clear ISO C++ Warning and Enable ffmpeg V5.0 Support 2 years ago::AVFormatContext *formatContext;::AVCodec *codec;::AVStream *audio_stream;::AVCodecContext *audio_stream_ctx;Shouldn't these be a conditional #if based on version? audio_stream and audio_stream_ctx.
AVCodecContextis not new – there are also other already existing occurrences. So there is no need to wrap it into#ifdefs. At the same time,AVCodecContextis used below for both older versions and newer versions, so it is not desirable to be conditional.The removal of the "codec" state variable from AVStream is a common cause of problems when building against ffmpeg 5. Unfortunately, it's usually used across functions/methods (as it is here) and since the new way of getting the AVCodecContext is to call "avcodec_alloc_context3" to allocate it you need somewhere common to stow the returned pointer, private state being a good place to use.
As with the AVPacket problem, it adds a little overhead when building for the older API (one pointer) to avoid more conditional code to support both versions.
Ok, my bad. Thanks for the feedback.
d->formatContext = NULL;d->codec = NULL;d->audio_stream = NULL;d->audio_stream_ctx = NULL;Shouldn't these be a conditional #if based on version? audio_stream and audio_stream_ctx.
This seems to be okay – see the comment above.
::AVSampleFormat sampleFormat;::AVFrame *frame;::AVPacket packet;::AVPacket *packet;Shouldn't these be a conditional #if based on version?
I understood that for older versions, a combination of
*packetand_packet(which is wrapped in#ifdefs) is used here to make the code easier and use*packetfor all cases. That seems to me a good solution.It's based on something I picked up while doing a change to another project using ffmpeg.
The older "av_init_alloc" takes a pointer to an existing AVPacket whereas its replacement, "av_packet_alloc" returns a pointer to a new AVPacket. The net result is that all the references to the AVPacket state have to be converted from "." to "->" for the newer call. Declaring both in this way means everything can be converted to "->" without having to add more conditional code.
Yes, I understood this reason and I liked the solution you used.
So far I haven't done tests, but there are some little comments.
if (d->audio_stream->codecpar->codec_type != AVMEDIA_TYPE_AUDIO) {#elsed->audio_stream_ctx = d->audio_stream->codec;if (d->audio_stream_ctx->codec_type != AVMEDIA_TYPE_AUDIO) {I propose to move the opening parenthesis of the block below
#ifdefsso that there is one common parenthesis opening the block instead of two parentheses related to the same block.OK, 'tis done.
// open the codec on our contextkdDebug() << "(K3bFFMpegFile) found codec for " << m_filename;if (::avcodec_open2(codecContext, d->codec, NULL) < 0) {kdDebug() << "(K3bFFMpegFile) found codec for " << m_filename << "\n";Instead of
"\n"it may be good to useendl.I did try that initially but couldn't get it to work. I'd put "std::endl" which it doesn't seem to like. Just tried it with only "endl" and that's worked fine, so done!
Yes, the simple
endlis what I meant.Thank you.
1e526c4704to8e7556b9a82 years agoI did a test building with an old distribution and it was successful.
I will wait for reactions and approval from @MicheleC.
It's ok from my side. I did a quick review yesterday but without going too deep, hence my comments turned up to be inaccurate. You can proceed and merge.
8e7556b9a8into master 2 years agoReviewers
8e7556b9a8.