Skip to content

Commit

Permalink
Playback Subtitle fixes
Browse files Browse the repository at this point in the history
Fix these problems
- External text subtitles displayed at the same time as internal subtitles.
- Selecting external subtitles from the menu not displaying any subtitles.
- Intermittent segmentation faults when enabling subtitles.
- External subtitles not appearing when switching from internal to external.

External text subtitles now being processed by FFmpeg are treated
as AV subtitles but still need to be identified as external text
subtitles.

- Add a flag to identify external subtitles to SubtitleReader.
- When internal subtitles are selected, discard external subtitles
- When external subtitles are selected, discard internal subtitles.
- Add checks to prevent segmentation faults on invalid frames.
- Set needsync. When switching from AV subtitles to external
  subtitles the subtitles were not showing.

Refs #773
  • Loading branch information
bennettpeter committed Dec 1, 2023
1 parent 4214ecb commit b47e464
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 10 deletions.
18 changes: 16 additions & 2 deletions mythtv/libs/libmythtv/captions/subtitlereader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ void SubtitleReader::SeekFrame(int64_t ts, int flags)

bool SubtitleReader::AddAVSubtitle(AVSubtitle &subtitle,
bool fix_position,
bool allow_forced)
bool allow_forced,
bool isExternal)
{
bool enableforced = false;
bool forced = false;
Expand All @@ -66,7 +67,7 @@ bool SubtitleReader::AddAVSubtitle(AVSubtitle &subtitle,
forced = forced || static_cast<bool>(subtitle.rects[i]->flags & AV_SUBTITLE_FLAG_FORCED);
}

if (!m_avSubtitlesEnabled)
if (!m_avSubtitlesEnabled && !isExternal)
{
if (!forced)
{
Expand All @@ -87,6 +88,19 @@ bool SubtitleReader::AddAVSubtitle(AVSubtitle &subtitle,
enableforced = true;
}

if (m_textSubtitlesEnabled && !isExternal || m_avSubtitlesEnabled && isExternal)
{
FreeAVSubtitle(subtitle);
return enableforced;
}

// Sanity check to prevent seg fault
if (subtitle.num_rects > 1000)
{
FreeAVSubtitle(subtitle);
return enableforced;
}

#ifdef DEBUG_SUBTITLES
if (VERBOSE_LEVEL_CHECK(VB_PLAYBACK, LOG_DEBUG))
{
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythtv/captions/subtitlereader.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class SubtitleReader : public QObject

AVSubtitles* GetAVSubtitles(void) { return &m_avSubtitles; }
bool AddAVSubtitle(AVSubtitle& subtitle, bool fix_position,
bool allow_forced);
bool allow_forced, bool isExternal);
void ClearAVSubtitles(void);
static void FreeAVSubtitle(AVSubtitle &sub);

Expand Down
9 changes: 7 additions & 2 deletions mythtv/libs/libmythtv/captions/subtitlescreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1846,7 +1846,9 @@ void SubtitleScreen::DisplayAVSubtitles(void)

AVSubtitles* subs = m_subreader->GetAVSubtitles();
QMutexLocker lock(&(subs->m_lock));
if (subs->m_buffers.empty() && (kDisplayAVSubtitle != m_subtitleType))
if (subs->m_buffers.empty()
&& (kDisplayAVSubtitle != m_subtitleType)
&& (kDisplayTextSubtitle != m_subtitleType))
return;

MythVideoOutput *videoOut = m_player->GetVideoOutput();
Expand Down Expand Up @@ -1898,7 +1900,11 @@ void SubtitleScreen::DisplayAVSubtitles(void)
#endif
}
lock.relock();
subs->m_needSync = false;

// extra check to avoid segfault
if (subs->m_buffers.empty())
return;
AVSubtitle subtitle = subs->m_buffers.front();
if (subtitle.end_display_time < currentFrame->m_timecode.count())
{
Expand Down Expand Up @@ -2060,7 +2066,6 @@ void SubtitleScreen::DisplayAVSubtitles(void)
#ifdef USING_LIBASS
RenderAssTrack(currentFrame->m_timecode, assForceNext);
#endif
subs->m_needSync = false;
}

int SubtitleScreen::DisplayScaledAVSubtitles(const AVSubtitleRect *rect,
Expand Down
6 changes: 3 additions & 3 deletions mythtv/libs/libmythtv/captions/textsubtitleparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ int TextSubtitleParser::ReadNextSubtitle(void)
sub.start_display_time = av_q2d(m_stream->time_base) * m_pkt->dts * 1000;
sub.end_display_time = av_q2d(m_stream->time_base) * (m_pkt->dts + m_pkt->duration) * 1000;

m_parent->AddAVSubtitle(sub, m_decCtx->codec_id == AV_CODEC_ID_XSUB, false);
m_parent->AddAVSubtitle(sub, m_decCtx->codec_id == AV_CODEC_ID_XSUB, false, true);
return ret;
}

Expand All @@ -275,8 +275,8 @@ void TextSubtitleParser::LoadSubtitles(bool inBackground)
}

// External subtitles are now presented as AV Subtitles.
m_parent->EnableAVSubtitles(true);
m_parent->EnableTextSubtitles(false);
m_parent->EnableAVSubtitles(false);
m_parent->EnableTextSubtitles(true);
m_parent->EnableRawTextSubtitles(false);

local_buffer_t sub_data {};
Expand Down
2 changes: 1 addition & 1 deletion mythtv/libs/libmythtv/decoders/avformatdecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3969,7 +3969,7 @@ bool AvFormatDecoder::ProcessSubtitlePacket(AVStream *curstream, AVPacket *pkt)

bool forcedon = m_parent->GetSubReader(pkt->stream_index)->AddAVSubtitle(
subtitle, curstream->codecpar->codec_id == AV_CODEC_ID_XSUB,
m_parent->GetAllowForcedSubtitles());
m_parent->GetAllowForcedSubtitles(), false);
m_parent->EnableForcedSubtitles(forcedon || isForcedTrack);
}

Expand Down
4 changes: 3 additions & 1 deletion mythtv/libs/libmythtv/mythplayercaptionsui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ void MythPlayerCaptionsUI::EnableCaptions(uint Mode, bool UpdateOSD)
if (kDisplayTextSubtitle & Mode)
{
m_captionsOverlay.EnableSubtitles(kDisplayTextSubtitle);
AVSubtitles* subs = m_subReader.GetAVSubtitles();
subs->m_needSync = true;
msg += tr("Text subtitles");
}

Expand Down Expand Up @@ -423,7 +425,7 @@ bool MythPlayerCaptionsUI::HasCaptionTrack(uint Mode)
if (Mode == kDisplayNUVTeletextCaptions)
return true;
// External subtitles are now decoded with FFmpeg and are AVSubtitles.
if ((Mode == kDisplayAVSubtitle) && m_captionsState.m_externalTextSubs)
if ((Mode == kDisplayAVSubtitle || Mode == kDisplayTextSubtitle) && m_captionsState.m_externalTextSubs)
return true;
if (!(Mode == kDisplayTextSubtitle) && m_decoder->GetTrackCount(toTrackType(Mode)))
return true;
Expand Down

0 comments on commit b47e464

Please sign in to comment.