AvFormatDecoder cleanup #1009
Closed
AvFormatDecoder cleanup #1009
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
If (Stream == nullptr || Stream->codecpar == nullptr) is true, a context will never be inserted into the map, so check this first and return nullptr. If the input value Codec != nullptr, it would always fail the checks in the else branch. avcodec_parameters_to_context() does not check for nullptr inputs, so ensure a nullptr is not dereferenced. Be explicit with comparisons to nullptr. No functional change.
The context returned by MythCodecMap::GetCodecContext() would have its codec_id and codec_type set equal to those of curstream and it makes more sense to reference them directly. The context will be passed to the various Process.*Packet() functions in a later change. No functional change.
av_dup_packet() no longer exists. No functional change.
This resolves an abort on an ATSC 3.0 sample related to m_streamsChanged being set later in GetFrame(). Since m_streamsChanged is no longer always set on an AC-4 packet, ScanStreams() is no longer repeatedly called unnecessarily, which also prevents log spam regarding not finding a decoder for AC-4 since it doesn't exist. I think AC-4 would already not have been available for pass-through since no track was added to AvFormatDecoder::m_tracks[kTrackTypeAudio].
par = m_ic->streams[strm]->codecpar and MythCodecMap::GetCodecContext() is called with m_ic->streams[strm] which will call avcodec_find_decoder() with par->codec_id. avcodec_find_decoder() already iterated over the list of AVCodecs, comparing with the input AVCodecID, therefore the code could only incorrectly select an encoder. Instead of spamming the log with a very long set of lines, recommend printing them separately via `mythffmpeg -codecs`, which is also formatted better. (See show_codecs() in FFmpeg/fftools/opt_common.c) Also reduce the log level to warning from error since a number of AVCodecIDs do not have a decoder and it is expected to not find one for those. Remove now only used once codec variable.
originally from 2005: MythTV@879a964 corresponding message about video codecs removed in 2017: MythTV@5c551a0
Simple find and replace, no functional changes.
PreProcessVideoPacket() unconditionally dereferences the AVCodecContext* returned by m_codecMap.GetCodecContext(curstream), so move the call after the check ensuring there is an AVCodecContext.
It does not make semantic sense to modify m_tracks from outside DecoderBase or its subclasses. DecoderBase::InsertTrack() is only used by cc708reader.cpp for kTrackTypeCC708 The call graph eventually goes back to AvFormatDecoder::DecodeCCx08() which will call AvFormatDecoder::UpdateCaptionTracksFromStreams() afterwards, which calls AvFormatDecoder::UpdateATSCCaptionTracks() which will clear m_tracks[kTrackTypeCC608] and m_tracks[kTrackTypeCC708]. Therefore, the inserted track is immediately deleted before it is used. However, the emitted signal may do something, although I don’t know why it sends a signal to itself instead of being able to call it directly. The signal would be emitted if a track for that service_num was not in AvFormatDecoder::m_tracks. However, this does not make sense since the selected track has not changed and this track did not exist, so it was not in use to change. If there were no kTrackTypeCC708 tracks, the signal will still be emitted when AvFormatDecoder::GetFrame() next calls DecoderBase::AutoSelectTracks().
The only functional change is to also set m_currentTrack[kTrackTypeVideo] to fully select the video track since those variables are not not used before the call to autoSelectVideoTrack() later in ScanStreams(). m_currentTrack[kTrackTypeVideo] would have been set to the same value later by AutoSelectTrack().
by not calling AutoSelectTracks(). Instead call AutoSelectTrack() in the function that clears m_tracks. Not calling AutoSelectTracks() in GetFrame() prevents log spam from AutoSelectAudioTrack(). Do not ResetTracks() in ScanStreams() for kTrackTypeCC608 or kTrackTypeCC708, which ScanStreams() does not effect. kTrackTypeVideo is already fully selected in autoSelectVideoTrack() so do not reset it either. These modify m_tracks: AvFormatDecoder::UpdateATSCCaptionTracks() clears and adds to kTrackTypeCC608 and kTrackTypeCC708 AvFormatDecoder::ScanTeletextCaptions() adds to kTrackTypeTeletextCaptions and kTrackTypeTeletextMenu AvFormatDecoder::ScanRawTextCaptions() adds to kTrackTypeRawText AvFormatDecoder::autoSelectVideoTrack() adds to kTrackTypeVideo and also sets m_selectedTrack and m_currentTrack AvFormatDecoder::remove_tracks_not_in_same_AVProgram() removes from all types AvFormatDecoder::ScanStreams() clears kTrackTypeAttachment kTrackTypeAudio kTrackTypeSubtitle kTrackTypeTeletextCaptions kTrackTypeTeletextMenu kTrackTypeRawText kTrackTypeVideo calls ScanTeletextCaptions ScanRawTextCaptions adds to kTrackTypeAttachment kTrackTypeSubtitle kTrackTypeAudio calls autoSelectVideoTrack remove_tracks_not_in_same_AVProgram AvFormatDecoder::SetupAudioStreamSubIndexes() modifies kTrackTypeAudio, but is only called by AvFormatDecoder::ProcessAudioPacket() which will then call AutoSelectAudioTrack(). AvFormatDecoder::ProcessVBIDataPacket() adds to kTrackTypeTeletextMenu when empty MythDVDDecoder::PostProcessTracks() modifies kTrackTypeAudio and kTrackTypeSubtitle, but is only called in ScanStreams(), so call AutoSelectTrack() after that call in ScanStreams().
… function It is only used once and was only ever connected to itself. This basically reverts 4567527.
The only difference is it will no longer produce a log message saying "0 available audio streams"
by refactoring out selectBestAudioTrack(). Also, return early from the new helper function to avoid unnecessary comparisons.
to the functions that use it, instead of reading it from the map again.
6 tasks
|
Rebased and merged. |
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.
The first few of these commits were originally added to #950, but were separated out since they were mostly extraneous.
Ignoring AV_CODEC_ID_AC4, which was added in FFmpeg 7.1, does resolve an abort on an ATSC 3.0 sample.
Checklist