Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mythplayer: fix exiting playback at EOF #512

Closed
wants to merge 2 commits into from

Conversation

ulmus-scott
Copy link
Contributor

Fixes #511

This also applies cleanly to fixes/32.

Checklist

@gigem
Copy link
Contributor

gigem commented Mar 3, 2022

I won't pull this change to master as as as it conflicts with larger, in-progress changes I already have that will be commited soonish. I will include the ultimate fix then or soon after.

I like the change with the short circuit return as the logic is clearer. However, it is provably wrong for the function to return true when there are no frames available. It is likely the clearing of m_buffering that is the important change here. Please verify that doing that and still returning false in fixes/32 fixes the problem. If it doesn't, please keep digging for the underlying bug.

@ulmus-scott
Copy link
Contributor Author

However, it is provably wrong for the function to return true when there are no frames available.

If it works, and is documented in a comment, it seems fine to me.

PrebufferEnoughFrames() appears to only be called in one place: https://code.mythtv.org/doxygen/mythplayerui_8cpp_source.html#l00646

It is likely the clearing of m_buffering that is the important change here. Please verify that doing that and still returning false in fixes/32 fixes the problem.

It doesn't (tested on master):
With:

    else if (GetEof() != kEofStateNone)
    {
        // other code detects EOF to exit playback, so keep playing
        SetBuffering(false);
        return false;
    }

at EOF, playback is frame-by-frame with each play/pause.

With:

    else if (GetEof() != kEofStateNone)
    {
        // other code detects EOF to exit playback, so keep playing
        SetBuffering(false);
        return m_videoOutput->ValidVideoFrames() != 0;
    }

it stops playback (presumably on the last frame) as before my fix.

With:

    else if (GetEof() != kEofStateNone)
    {
        // other code detects EOF to exit playback, so keep playing
        if (!m_avSync.GetAVSyncAudioPause())
            m_audio.Pause(false);
        SetBuffering(false);
        return m_videoOutput->ValidVideoFrames() != 0;
    }

It sometimes works, but mostly not.

If it doesn't, please keep digging for the underlying bug.

I'll have to try tracing the call(s) to PrebufferEnoughFrames() more fully to determine if/where additional EOF code should go.

@gigem
Copy link
Contributor

gigem commented Mar 4, 2022

Fixed differently.

@gigem gigem closed this Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: video hangs at end of file instead of exiting
2 participants