Videoplayer displays audio files as video

[emveepee]

Bug report

PR 17574 #17574 addresses the transition from PVR TV to Radio when the new stream is a transport stream but does not address the possibility of the radio stream being an mp3 stream.

The issue is actually not PVR related since the problem exists going from video to audio stream in the Video library and played with VideoPlayer.cpp

Describe the bug

The logic to switch to MUSIC_VISUALIZATION is based on the stream not having video. With the current logic the switch to an mp3 file with no video is still flagged as having video

After a video is played the logic the m_HasVideo flag remains true unless the new stream returns a custom flag https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/VideoPlayer.cpp#L1029

The video player checks for this flag here https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/DVDDemuxers/DVDDemuxFFmpeg.cpp#L1022

On transport streams that value will be set however because mp3 streams don't have programs IsProgramChanged() returns false returning here https://github.com/xbmc/xbmc/blob/master/xbmc/cores/VideoPlayer/DVDDemuxers/DVDDemuxFFmpeg.cpp#L1969

Expected Behavior

Going from Video to Audio should always turn off the m_HasVideo flag. Resetting m_HasAudio is probably worthwhile.

Actual Behavior

m_HasVideo is set in the constructor and only disabled if the audio stream has program which mp3 don't have.

Possible Fix

I did test by setting m_HasVideo = false when the input stream is opened. It can be set on close too. Not sure which will have the least impact of switching time if any. It is always reset to true in OpenVideoStream() in any case.

To Reproduce

Steps to reproduce the behavior:

1. Open any video file from the video library or PVR
2. While video is playing, in the video library open a strm file or m3u file with a link to a local mp3 file or mp3 internet stream
3. Display will be the frozen image from the video with the new audio steam playing and audio metadata displayed.

Debuglog

The debuglog can be found here:

https://paste.kodi.tv/qelabovida.kodi

Screenshots

Here are some links or screenshots to help explain the problem:

Additional context or screenshots (if appropriate)

Here is some additional context or explanation that might help:

Other negative impacts of having an open audio file with m_HasVideo true are unknown.

Your Environment

Used Operating system:

• Android

• iOS

• Linux

• OSX

• Raspberry-Pi

• Windows

• Windows UWP

• Operating system version/name: Win 10

• Kodi version: Matrix

note: Once the issue is made we require you to update it with new information or Kodi versions should that be required.
Team Kodi will consider your problem report however, we will not make any promises the problem will be solved.

[phunkyfish]

@djp952 you worked on the previous code change audio/video ready correct? Any ideas?

[djp952]

The suggestion seems very reasonable to me, the use case behind the changes I made was for MPEG-TS streams that didn't contain a video elementary stream at all in them. Maintaining the existing assumptions that the lionshare of these streams will have a video ES was important to the reviewers (and to me, I didn't want to break stuff for a fringe case).

I should still have the test materials I used for that available, I can make the suggested change locally and see if it appears to have any negative consequences for those, but I don't believe it will. Will report back!

[phunkyfish]

Thanks!

[djp952]

I was able to reproduce @emveepee's issue on Leia and making the suggested change resolved it with no ill effects that I could discern with any samples I have available. I happen to be playing around with a Radio PVR at the moment, so I could check that specific scenario as well. Worked well, the player switched from video mode to audio mode and back again just fine.

I opted to add the proposed state flag resets to the end of CVideoPlayer::CloseStream(), it seemed to me to be the most logical place:

  IDVDStreamPlayer* player = GetStreamPlayer(current.player);
  if (player)
  {
    if ((current.type == STREAM_AUDIO && current.syncState != IDVDStreamPlayer::SYNC_INSYNC) ||
        (current.type == STREAM_VIDEO && current.syncState != IDVDStreamPlayer::SYNC_INSYNC) ||
        m_bAbortRequest)
      bWaitForBuffers = false;
    player->CloseStream(bWaitForBuffers);
  }

  current.Clear();

  m_HasVideo = false;  // <--- djp952: added
  m_HasAudio = false;  // <--- djp952: added

  return true;

I see that this typically takes place in ::CloseFile(), but that function will not be called in these scenarios. My only suggestion might be to look more closely at the other member variables to see if there are any others that should also be reset here (m_Edl, perhaps?).

Suggested fix looks good from my basement! Thanks @emveepee! I hope my rudimentary testing helps you out sir, and if you need any testing support for a PR please let me know.

[FernetMenta]

@djp952 your suggestion is wrong for more than one reason. 1) you set hasVideo to false even if CloseStream is called for audio stream. 2) CloseStream can be called at any time with valid video. Nowadays most people forget to consider and test DVDs and Blurays, which is still a very valid use case.

The correct method to reset the state variables is CVideoPlayer::Prepare()

[djp952]

Just trying out the previously suggested solution. Wasn't intending to make a case for it being the best way to solve the Issue.

[emveepee]

@FernetMenta will making any changes to this cause users to perceive that tuning is slower if the the frozen image is replaced with a Kodi screen? I am thinking of long tuning times for satellite or HDMI capture (my STB takes 5 seconds to tuner). This might need to be a state change which turns off video if no video is received when new audio is received.

[FernetMenta]

@emveepee if you see a frozen image on channel change, something is wrong with the platform specific code. whenever a new file or channel is started, video output is supposed to be cleared.

[emveepee]

@emveepee if you see a frozen image on channel change, something is wrong with the platform specific code. whenever a new file or channel is started, video output is supposed to be cleared.

I was referencing my screenshot above when the video image was not being cleared when the new mp3 stream started. I thought this might be intended behaviour to avoid a kodi screen showing during a PVR channel switch.

[FernetMenta]

I thought this might be intended behaviour to avoid a kodi screen showing during a PVR channel switch.

This is a bug on your platform. Intended is a black screen during channel switch. This is the general behavior on any tv or stb.

[djp952]

@emveepee, before this ends up getting closed, I wanted to tell you what I found in regard to the still picture as opposed to a black screen on Windows in case you want to pursue it further. Keeping in mind that I suck at all things VideoPlayer...

This does seem to happen on Windows regardless of the Renderer in use. I asked a few folks to see what they thought, since HDHomeRuns are very slow to tune - it's clearly noticeable if you are looking for it. The consensus was that nobody had ever given it a second thought, even with slow TV channel changes it wasn't bothersome.

That said, what I noticed stepping through the code was that once a Video stream has been opened by VideoPlayer on Windows, the Renderer never gets a Flush(false) to release the buffers until VideoPlayer is destroyed. If I changed the "final" Flush call when playback stops to send a false, the black screen appeared as opposed to a still picture.

I'm not claiming this is valid, let alone being even close to the underlying cause, I can't debug Kodi on anything but Windows so I don't have anything to compare it to :) Just wanted to let you know where I was in looking at it before I decided it wouldn't be worth pursuing. I've caused FernetMenta more than enough grief lately.

[djp952]

Linking to discussion in this issue: #19153. Suggest if this gets reopened it be targeted for Kodi v20.