scePsmf() : add isPlayingStatus check

[dbz400]

Fixes #5178

[dbz400]

It is referencing to apply psmfplayer status check here. It fixes the hanging in return PSMF video/audio when end.

https://code.google.com/p/jpcsp/source/browse/trunk/src/jpcsp/HLE/modules150/scePsmfPlayer.java?spec=svn3454&r=3454#482

[unknownbrackets]

I don't really understand your condition:

(
  (
    psmfplayer->status != PSMF_PLAYER_STATUS_PLAYING
    && psmfplayer->status != PSMF_PLAYER_STATUS_PLAYING_FINISHED
    && psmfplayer->status != PSMF_PLAYER_STATUS_ERROR
  )
  || psmfplayer->status == PSMF_PLAYER_STATUS_NONE
)

The NONE part seems completely superfluous. Like saying, "if I don't have 3, 4, or 5 apples - or if I have 9 apples." 9 isn't one of 3, 4, or 5 anyway.

I assume these statuses aren't tested and is just from JPCSP, right?

-[Unknown]

[dbz400]

Yep , it is borrow from JPCSP though .

Sorry the NONE part is wrongly put in during testing and has been removed here

[dbz400]

This is the ref code from JPCSP and translate here in this commit

public int checkPlayerPlaying(int psmfPlayer) {
    psmfPlayer = checkPlayerInitialized(psmfPlayer);
    if (psmfPlayerStatus != PSMF_PLAYER_STATUS_PLAYING && psmfPlayerStatus != PSMF_PLAYER_STATUS_PLAYING_FINISHED && psmfPlayerStatus != PSMF_PLAYER_STATUS_ERROR) {
            if (log.isDebugEnabled()) {
                    log.debug(String.format("checkPlayerInitialized player not playing (status=0x%X)", psmfPlayerStatus));
            }
            throw new SceKernelErrorException(ERROR_PSMFPLAYER_NOT_INITIALIZED);
    }

    return psmfPlayer;
}

[hrydgard]

Would have been nicer to centralize the check in a function (like, bool IsPlayingStatus(u32 status) ).

By the way, it looks pretty odd to have _ERROR in there. Is there a motivation for that?

[dbz400]

I'll have further check here to see if that _ERROR is necessary and also consolidate it in a single function and call it .

[dbz400]

Done .Should be clean now.

[dbz400]

@hrydgard , okay to merge?

[hrydgard]

Hm, didn't you flip all the checks to the opposite now? Shouldn't it be:

if (!isPlayingStatus(psmfplayer->status)) {
    ERROR_LOG ....
    ...
}

[unknownbrackets]

Confusingly, isPlayingStatus() returns true when it's not playing, and false when it is playing.

-[Unknown]

[hrydgard]

Ah right, so it's correct but wrongly named. Please rename it "isNotPlaying" or reverse the return value and the checks.

[dbz400]

Done. Should be good this time.

[dbz400]

Just tested Crimson Room Reverse , this commit also fixed it .