Update FFmpeg to version 7.1 #950
Open
+443,810
−222,366
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Scott, |
|
@warpme I had made a typo in mythvaapiinterop.cpp which is fixed in the first new commit and hopefully fixes the double rate VAAPI deinterlacing. I did not see anything of interest in the logs either. If the new commit doesn't work, try testing at 93a9bc4 and, if that still doesn't work, at 84c1781 before the FFmpeg update. Before merging I want to move some of the final commits earlier and squash some of them together. |
|
Scott, Current head works perfectly now! |
ulmus-scott
force-pushed
the
ffmpeg/7.1
branch
from
07c5b72 to
66fa888
Compare
|
Rebased onto master with a few commits reordered and squashed. |
This was referenced Dec 21, 2024
Closed
|
I have rebased and merged into branch devel/ffmpeg-resync. |
|
Works OK for me. No regression found. I do have a few ATSC3.0 recordings and they do play back with a lot of stuttering and complains about the ac4 audio codec: With the current master there is not even an attempt to play the audio but the video plays smooth. |
It was originally for a different function av_destruct_packet_nofree: MythTV@bf432d9#diff-4b6a432208acd44651f07cdde943169aba8c5611b26a83764ad784d00c250f7fR899
This reverts commit 049fabc. Reduce changes to FFmpeg.
Instead of the internal FFmpeg header compat/cuda/dynlink_loader.h. This leaves only one internal FFmpeg header used by MythTV. Per the comment on win32_dlopen() in FFmpeg/compat/w32dlfcn.h, Windows will now additionally search the current directory for nvcuda.dll and nvcuvid.dll, which FFmpeg considers less secure. This is an atomic block and the order must be preserved: #include "libavutil/log.h" #define FFNV_LOG_FUNC(logctx, msg, ...) av_log(logctx, AV_LOG_ERROR, msg, __VA_ARGS__) #define FFNV_DEBUG_LOG_FUNC(logctx, msg, ...) av_log(logctx, AV_LOG_DEBUG, msg, __VA_ARGS__) #include <ffnvcodec/dynlink_loader.h> #include "libavutil/hwcontext_cuda.h" must come after #include <ffnvcodec/dynlink_cuda.h> which is also transitively included by #include <ffnvcodec/dynlink_loader.h>
This reverts commit 5f9b27c. MythTV no longer uses that file, so remove the modification.
…h (part 1) Since we don't need the full buffer, we could use avio_read_partial() instead.
Originally from 2003: MythTV@ec136a9
…h (part 3) I don't know if any of this is necessary since FFmpeg's avio API is already buffered. However, I did not investigate using avio directly with avio_open() nor did I investigate what our buffer actually does. I'm not sure why MythAVFormatBuffer::Seek() with Whence = SEEK_END did not ever actually seek. libavformat/avio.h says this about read_packet: " A function for refilling the buffer, may be NULL. For stream protocols, must never return 0 but rather a proper AVERROR code. " So I'm not sure the first return of 0 was correct. AVERROR(EINVAL) is a more useful return value for all of the functions, instead of 0.
They are const in mpegts.c.
The only STREAM_TYPE_* define used is STREAM_TYPE_PRIVATE_DATA. STREAM_TYPE_AUDIO_DTS is already defined by FFmpeg to a different value. 0x8a is used in MISC_types[]. STREAM_TYPE_AUDIO_HDMV_* defines are from: MythTV@d15e482 They are now unused and FFmpeg has added the entries to HDMV_types[].
STREAM_TYPE_SUBTITLE_DVB is from: MythTV@aaa9372#diff-ae852538fd61b663231c8bdda1fdbade40c97e8305401fcc6f1ad9f395efdf18 STREAM_TYPE_VBI_DVB is from: MythTV@6da1854 However, the corresponding uses in MISC_types[] can never be used. in mpegts_set_stream_info() pes->stream_type = stream_type; ... mpegts_find_stream_type(st, pes->stream_type, MISC_types); mpegts_set_stream_info() is called either with stream_type = 0 or stream_type = get8(&p, p_end) 0x100 and 0x101 exceed UINT8_MAX and thus will never be found in the search.
Use the names given in ETSI 300 468 and list all of the values used, even if the defines aren't.
I'm not sure that the new name is correct since I don't have a copy of ISO/IEC 13818-6. However, it should be close.
UpdateStoredFrameNum(), WriteStoredData(), ClearStoredData()
and fix Doxygen syntax for other functions.
and eliminate unnecessary transitive includes of those files.
The first return ensures (available < count) is always true. If m_ateof is true, then Count = available, which means (available < count) is false and all subsequent conditionals are false. Now, (available < count) is only needed for the while loop condition.
This reverts commit db3dfa505a2a5a7e2b6ff898f9c3ad5d74fbf01a. I am not sure the "optimization" was correct and removing the customization appears to have no effect of any kind. My MHEG sample did not trigger the log message for "All DSM info found".
The code in libavcodec/mpeg12dec.c will never create a cc_type of 2 for SCTE-20 and cc_type is guaranteed to be less than 2 otherwise by the if condition. AvFormatDecoder::m_invertScteField is now always 0, so remove it. AvFormatDecoder::m_lastScteField is now set but unused, so remove it.
…captions see mpeg_decode_a53_cc() Differences: FFmpeg does not have the initial two bytes that MythTV added to signal valid and length. FFmpeg will only use the first type of embedded CC that it sees, whereas MythTV would switch between ATSC and SCTE-20 under certain conditions. If multiple types were present MythTV would concatenate them. FFmpeg allows forcing the subtitle format with the cc_format option in mpeg12dec.c. DVD CC: FFmpeg does not check p[4] ATSC A/53: FFmpeg checks the process_cc_data_flag in p[5]. SCTE-20: FFmpeg ignores line_offset, which MythTV ensures is 11 for line 21 data. FFmpeg does not set one_bit or the 4 reserved bits before cc_valid and cc_type (cc608hdr vs cap[0]), but these are only to avoid emulating start codes and are not checked, so it doesn't matter. p[0] == 0x05 && p[1] == 0x02: Moved to mpeg_decode_a53_cc(). Originally from: Fixes #2481, by applying patch from kenny at the-b org. This adds dec… MythTV@49d3294 Referencing: https://code.mythtv.org/trac/ticket/2481 Modified in: Simplify DTV CEA-608 handling a bit & add some range checking. MythTV@be6a2b4 Some tweaks to cc implementation in mpeg12.c: make the C ANSI compati… MythTV@77cff32 See also: https://github.com/CCExtractor/ccextractor/blob/master/src/lib_ccx/es_userdata.c MythTV: SCTE-20 encapsulates CTA-608, which is where XDS is defined, so I don't know why the condition was specific to SCTE-20.
This reverts commit 641109a. David Hampton confirmed on the mailing list that this change is no longer necessary on FreeBSD 13/14/15.
…Android)' Neither header is checked by FFmpeg's configure and neither HAVE_EGL_EGL_H nor HAVE_GLES2_GL2_H is defined. FFmpeg/configure has not tested those since MythTV@018be3f in 2018. FFmpeg's configure never tested eglGetProcAddress and thus would never define HAVE_EGLGETPROCADDRESS. MythTV's configure does check them and (myth)config.h does define HAVE_EGL_EGL_H, HAVE_GLES2_GL2_H, and HAVE_EGLGETPROCADDRESS; however, FFmpeg does not use mythconfig.h, using its own config.h instead, probably since the decoupling of MythTV and FFmpeg's configure in MythTV@9563786 in 2018.
by merging the code into AvFormatDecoder::RemoveAudioStreams() and replacing ff_flush_packet_queue() with avformat_flush(). The original code could have accessed out of bounds if the first stream was an audio stream, since i would have been -1 on the next iteration. This would also cause it to recheck previously checked streams when a stream was removed. By setting the index first, we don't have to iterate twice to renumber the streams. Slightly more is reset by avformat_flush(), see libavformat/seek.c, compared to just ff_flush_packet_queue(), but it is a public function and resetting the extra values is probably better since it more fully resets the AVFormatContext. Additionally, the AVFormatContext is now only reset once instead of after removing each stream.
StreamInfo::m_av_substream_index is only used for kTrackTypeAudio. It is no longer set to 0 by: mythdvddecoder.cpp AvFormatDecoder::ScanStreams() for AVMEDIA_TYPE_SUBTITLE StreamInfo::m_orig_num_channels is only used by AvFormatDecoder::GetTrackDesc() to print the number of audio channels for kAudioTypeNormal. The following no longer set m_orig_num_channels to 0: mythdvddecoder.cpp cc708reader.cpp decoderbase.cpp avformatdecoder.cpp StreamInfo::m_easy_reader and m_wide_aspect_ratio were unused but set by: AvFormatDecoder::ScanATSCCaptionStreams() (incorrectly, setting m_orig_num_channels = easy_reader and m_easy_reader = wide_aspect_ratio) AvFormatDecoder::ScanStreams() for AVMEDIA_TYPE_AUDIO (appeared to set m_easy_reader in an unused attempt to flag the second StreamInfo for dual mono audio) decoderbase.cpp: I don't know why one of the default CC608 set m_language_index to 2, but it is not used for closed captions.
If continue is DVD specific and from MythTV@8bcef59 AVStream::id will never be negative for MPEG-TS or MPEG-PS.
It was a copy of a value from AVCodecParameters and was only used for printing.
If the channel count from FFmpeg is wrong, add this to GetTrackDesc() to get the
value from libdvdnav:
if (m_ringBuffer->IsDVD())
channels = m_ringBuffer->DVD()->GetNumAudioChannels(m_tracks[kTrackTypeAudio][TrackNo].m_stream_id);
When attempting to upstream the change before, it was recommended to instead use AV_CHANNEL_ORDER_CUSTOM with two AV_CHAN_FRONT_CENTER channels. See https://patchwork.ffmpeg.org/project/ffmpeg/patch/[email protected]/ audiotoolboxdec.c will no longer flag dual_mono since it uses AV_CHANNEL_ORDER_UNSPEC. SetupAudioStreamSubIndexes() will create or destroy the second StreamInfo for the second substream, leaving the duplicate StreamInfo from ScanStreams() untouched, allowing playing both channels as stereo.
st and st->codecpar will already have been dereferenced by then.
Originally from: MythTV@6aaf617 references: https://code.mythtv.org/trac/ticket/5978 I could not find the referenced FFmpeg bugs from the comment or MythTV trac since FFmpeg has changed its bug tracker. However, I did find https://samples.ffmpeg.org/ffmpeg-bugs/roundup/issue378/Issue378/ which plays back with subtitles fine. As far as I can tell the change reduces out_size by len + 6 if that section is at the end of p. However, that data is all in the buffer, so I don't see how that would fix anything. If this does actually fix something a new FFmpeg bug report should be created with a sample.
libavcodec/codec_desc.c: move to correct location and correct description libavcodec/codec_id.h: move to correct location in data codecs libavformat/mpegts-mythtv.c: reuse teletext_descriptor code for VBI_teletext_descriptor libmythbase/iso639.h: add used include libmythbase/stringutil.h: add new funtion split_sv() libmythtv/decoders/avformatdecoder.cpp: AvFormatDecoder::ScanTeletextCaptions() was specific to AV_CODEC_ID_DVB_TELETEXT, looking only at the teletext_descriptor in the PMT. ScanTeletextCaptions() no longer uses the MythTV exported PMT, using what FFmpeg already exported when it parsed PMTs. Both AV_CODEC_ID_DVB_TELETEXT and AV_CODEC_ID_DVB_VBI (if there is EBU teletext data in the VBI data) now add to the tracks list and multiple streams are now supported.
update_initial_timestamps(): the AVStream* will already have been dereferenced higher up the call stack. compute_frame_duration(): AVStream::codecpar should always be non-NULL, see avformat_new_stream(), and the AVStream* will already have been dereferenced higher up the call stack. handle_new_packet(): av_assert0() guarantees that the AVStream* is valid if they have been added and removed from the AVFormatContext correctly.
…cc_format() When Closed Captions are discovered, this method is always invoked. Therefore, use it to set the property instead of repeating the statement. Signed-off-by: Marth64 <[email protected]> (cherry picked from commit 11c703be3176174c2ddb0f3893d10f3ed1accb89)
ATSC A/52:2018 Digital Audio Compression (AC-3, E-AC-3), Annex G defines stream_type 0x87 for E-AC-3 bit streams. Signed-off-by: Marton Balint <[email protected]> (cherry picked from commit be784e95ac5cd720fb0da0f13841b0fdf4f90391)
Signed-off-by: Marton Balint <[email protected]> (cherry picked from commit 0dceced45c52acc773e690a7aa7ff2e3fb8c560b)
Reviewed-by: Marth64 <[email protected]> Signed-off-by: Marth64 <[email protected]> (cherry picked from commit 78c4d6c136e10222a0b0ddff639c836f295a9029)
None of the other external libraries have the warnings disabled. In addition, the MythTV modifications no longer generate any additional warnings.
|
@kmdewaal , I have reverted your fix regarding blocking with VDPAU referencing https://trac.ffmpeg.org/ticket/9532. Could you test if it is still an issue and update the FFmpeg trac as needed? FFmpeg does not currently have a decoder for AC-4, but a raw muxer and demuxer were added so there is now AV_CODEC_ID_AC4, so it now recognizes the stream as AC-4 but can't decode it. Is master also using vaapi hardware acceleration? Is ffmpeg/7.1 repeatedly trying to find a decoder for AC-4? |
ulmus-scott
force-pushed
the
ffmpeg/7.1
branch
from
a709196 to
433e7af
Compare
I do not have an Nvidia/VDPAU system running at the moment so I cannot test it right now. Possibly later. I am OK with having the fix reverted.
Yes, I gathered as much. This should in principle make it possible to use audio pass-through for AC4 so that people with an audio system that can decode AC4 can then use ATSC3.0.
|
|
I have reset branch devel/ffmpeg-resync and merged the updated pull request into devel/ffmpeg-resync. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
Commits
The commits in order are:
FFmpeg reversions
#923
Eliminate usage of internal FFmpeg headers
#931
mpegts-mythtv harmonization
#927
Changes to make MythTV compile with FFmpeg 7.1:
Update to FFmpeg 7.1:
Cleanup after update:
Silence new FFmpeg deprecation warnings:
Other cleanup commits made while investigating:
Testing
@kmdewaal , I have reverted your fix regarding blocking with VDPAU referencing https://trac.ffmpeg.org/ticket/9532. Could you test if it is still an issue and update the FFmpeg trac as needed?
I have tested with my various Video samples and US OTA ATSC recordings and everything appeared to work the same. I do not have any hardware acceleration set up, so I could not test with that.
Testing with hardware acceleration or other source types would be helpful.
Quirks in
masterI may look into moreAvFormatDecoder::GetFrame() log spam every ~100μs:
decoding error End of fileif paused near end of file. I can sometimes trigger this with Video files when skipping by 1 second near the end while paused.The seeking code for Videos is very fragile, I suspect multithreading issues.
Sometimes either seek forward or backward does nothing and gives the following error in the log:
AFD: av_seek_frame(ic, -1, 13449820978, 0) -- errorSome further investigation revealed the error code to be -1, which FFmpeg thinks
is EPERM.
For a short (~16s) audio only file I created, skipping backwards usually causes playback to exit instead of skipping.