[DemuxClient] Parse for mid stream changes (optional) #16642

peak3d · 2019-09-18T22:48:52Z

Description

This PR implements the option to keep the ffmpeg parser alive even after codec extradata was found to track mid stream changes (@ksooo ). This happens only if the decoder don't have the capability to parse packets (curently only android mediacodec). For other decoders (e.g. ffmpeg) the demuxer will always stopp parsing after first extradata was found.

For PVR this implementation asks the demuxclient to continue parsing only for MPEG2 streams.
This is just for testing / showing what to do and should / could be implemented different.

Motivation and Context

solve mid stream changes

How Has This Been Tested?

Play PVR SD MPEG2 channel on Windos (ffmpeg) and Android.

Types of change

Bug fix (non-breaking change which fixes an issue)
Clean up (non-breaking change which removes non-working, unmaintained functionality)
Improvement (non-breaking change which improves existing functionality)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that will cause existing functionality to change)
Cosmetic change (non-breaking change that doesn't touch code)
None of the above (please explain below)

ksooo · 2019-09-19T06:03:50Z

The diff is hard to read because the functional changes are well hidden among the code formatting changes.

Could you next time please make separate commits for the functional changes and code formatting changes?

Regarding the PVR change: why is this only needed for MPEG2VIDEO and not for instance also for H264?

FernetMenta · 2019-09-19T06:50:05Z

It is wrong anyway. Not all content is frame interlaced and behaves like mpeg2. ffmpeg detected fps might be correct already.

FernetMenta · 2019-09-19T07:00:12Z

Further, for h264 fps is NOT coded in SPS (extradata). You can parse this as long as you want but ffmpeg won't ever detect a change in fps.

peak3d · 2019-09-19T08:16:46Z

@FernetMenta fps is in VUI (which is part of SPS). And it is read in ffmpeg:
https://ffmpeg.org/doxygen/3.2/vaapi__encode__h264_8c_source.html line 240

Regarding your first post: what would be your solution to fix the wrong fps for live streams?
Check for IsLiveTV ? TVH already provides wrong FPS, so it is currently no alternative to check for 0.

Edit: "Not all content is frame interlaced and behaves like mpeg2." Do you mean "Not all frame interlaced content behaves like MPEG2" ? Then I would understand, because the interlaced check is done in this implementation.

peak3d · 2019-09-19T08:19:41Z

@ksooo this is just a "sample" implementation to give you an idea how to trigger.
Constantly frame parsing cost some resources which may be limited on embedded devices.
-> Its on you to provide an idea how to trigger it. Maybe you also want to set your FPS from TVH always to 0 so I can modify my check (only fix FPS if not provided from demux)

Edit: for RTL SD streams for example TVH provides 1000 / 40 = 25 fps, what is wrong.
Edit2: @ksooo have you ever seen h.264 livetv streams which have stream changes? If so, the question is if you can already detect a change in your PVR implementation (from backend) . This would cost much less compared to let the stream parser active al the time.

CiNcH83 · 2019-09-19T09:00:31Z

Edit: for RTL SD streams for example TVH provides 1000 / 40 = 25 fps, what is wrong.

Still don't get why this is wrong. Of course you do not want the display to switch to 25 and later to 50Hz. I do get that. But input/source is still 25fps (50 fields per second = 25 frames per second, either from interlaced or progressive source). It depends on the deinterlacer what the output will be. So probably the info for refresh rate switching is taken at a wrong stage in the pipeline or 25fps should always result in a switch to 50Hz anyway. I might not be seeing the full picture though...

peak3d · 2019-09-19T09:09:32Z

fps in DVDStreamInfo are meant as progressive fps, our DI produces 50fps for such kind of stream.
You may have noticed, that there is no "interlaced" flag in DVDStreamInfo struct which tells other places to double frame rate later on.

So it depends on how to interpret the fps in stream, in view of kodi or in view of the stream. And they are different (25 / 50). because of the missing IsInterlaced information the only way to provide a stream start with the "correct" fps is using the kodi one (50)

Edit: Conclusion: both values are right or wrong depending where you look at.

CiNcH83 · 2019-09-19T09:13:49Z

fps in DVDStreamInfo are meant as progressive fps, our DI produces 50fps for such kind of stream.

What is "our deinterlacer"? Yadif is configured to 0,-1,1 (0... single frame rate) on Android due to slow boxes/TVs, outputting 25fps. MediaCodec on BRAVIA also outputs 25fps. A switch to 50Hz still isn't bad of course...

peak3d · 2019-09-19T09:24:50Z

@CiNcH83 you are right, this depends on the deinterlacer (compared DXVA DI (=25fps) with Mediacodec DI (=50fps), so the check for doubling has to be done at an other place, thx.

CiNcH83 · 2019-09-19T09:39:45Z

so the check for doubling has to be done at an other place

What's wrong with setting refresh rate to 50Hz for 25fps content? 50Hz results in more responsive GUI...

peak3d · 2019-09-19T12:18:06Z

At this place it is wrong, because it could also be needed in its correct value for the later processing.

FernetMenta · 2019-09-19T12:20:16Z

fps is in VUI (which is part of SPS)

VUI is supplementary information and is not mandatory. IIRC it is not available for most TV channels.

CiNcH83 · 2019-09-19T12:28:13Z

VUI is supplementary information and is not mandatory. IIRC it is not available for most TV channels.

I at least found it on all German/Austrian broadcasts I tried.

FernetMenta · 2019-09-20T05:59:32Z

@CiNcH83 that proves nothing. According to the spec this info is optional and does not need to be present in SPS. At the time I wrote the h264 parser for vnsi I did not rely on this field because it was not reliable.
fps delivered by demuxer is just a hint, hence it is not wrong to use such info if present. BUT overwriting already set parameters here in demuxClient is definitely wrong.

xbmc/cores/VideoPlayer/DVDCodecs/Video/DVDVideoCodec.h

CiNcH83 · 2019-09-23T11:11:16Z

xbmc/cores/VideoPlayer/DVDCodecs/Video/DVDVideoCodecAndroidMediaCodec.cpp

+{
+  int fpsRate = m_hints.fpsrate;
+  if (m_hints.fieldOrder > AV_FIELD_PROGRESSIVE)
+    fpsRate *= 2;


Sorry, but this confuses me so badly.. in case of PAL, if we have interlaced material but source is actually progressive, you can use WEAVE and you‘ll get a perfect 25fps as you always have two matching fields. If source is truly interlaced, the deinterlacer will either interpolate to 25fps or 50fps.

This mustn't cunfusing you badly, this is a PR and open for discussion.
Seems that you have lot of knowledge about different codecs and their behaviour, pls. provide information about how it should be solved.

The info is wrong in many cases anyway. So it does not seem to be crucial.

The source is 25fps, no matter whether content is truly interlaced or from progressive source or what the deinterlacer does.

I did some testing with 2 DVDs on my BRAVIA Android TV...

(1) The Mummy: Tomb of the Dragon Emperor: progressive 576i25 (50 fields per second with always 2 fields originating from the same point in time)
(2) Rocky: truly interlaced 576i25 (50 fields per second with each originating from a different point in time)

(1) can be deinterlaced with a simple WEAVE, putting 2 fields together to build one single frame, resulting in 25fps. (2) is either interpolated to 25fps or 50fps (frame doubler).

I played those DVDs in Kodi through Android MediaCodec and ffmpeg (MediaCodec disabled).

MediaCodec on BRAVIA outputs 25fps for both DVDs. The BRAVIA therefore does not perform frame doubling for truly interlaced material.

ffmpeg in Kodi is configured to perform "half rate" deinterlacing (Yadif: 0,-1,1) on Android by default (I think @fritsch changed that because of the many slow boxes and TVs), therefore outputting 25fps for both DVDs too.
Deinterlacer can be switched to "full rate" (Yadif: 1,-1,1) in video settings. After doing so, Kodi outputs (1) at 25fps (due to its progressive nature) and (2) at 50fps (check with STRG+SHIFT+O).

I would leave the info at 25fps because it is the correct one from a source/demuxer point of view. Either truly interlaced or interlaced from progressive source, 25fps is correct. Determining true output fps is probably a matter of calculating frame time delta at the video renderer. STRG+SHIFT+O already shows the correct info.

As for refresh rate switching, I would always switch to 50Hz in case of PAL if a respective display mode exists, even if you have 25fps at the renderer...

Determining true output fps is probably a matter of calculating frame time delta at the video renderer. STRG+SHIFT+O already shows the correct info.

CVideoPlayerVideo::m_fFrameRate might be the right figure to look at.

Thx @CiNcH83, can you provide a some second snippet of "The Mummy" pls ?
Edit: and some seconds of the other stream, too, pls.

Sure...

The Mummy: Tomb of the Dragon Emperor (progressive 576i25)

Format : MPEG Video Format version : Version 2 Format profile : Main@Main Width : 720 pixels Height : 576 pixels Frame rate : 25.000 FPS Standard : PAL Scan type : Progressive Scan order : Top Field First

Rocky (interlaced 576i25)

Format : MPEG Video Format version : Version 2 Format profile : Main@Main Width : 720 pixels Height : 576 pixels Frame rate : 25.000 FPS Standard : PAL Scan type : Interlaced Scan order : Top Field First

CiNcH83 · 2019-09-24T12:34:31Z

Play PVR SD MPEG2 channel on Windos (ffmpeg) and Android.

Mid-stream changes are not handled for DVBViewer PVR add-on. Is it due to the fact that this add-on uses a file based approach rather than DemuxClient?

peak3d · 2019-09-24T14:02:41Z

Yes, could be, we can look later on it if the other things are fine.

Rechi · 2019-09-26T09:56:33Z

xbmc/cores/VideoPlayer/VideoPlayer.cpp

+        CLog::Log(LOGDEBUG, "%s - ProcessInfo reports fps: %0.4f.", __FUNCTION__, framerate);
+        if (framerate < 1.0E-6)
+          framerate = static_cast<float>(
+              DVD_TIME_BASE / CDVDCodecUtils::NormalizeFrameduration((double)DVD_TIME_BASE *


use C++ style cast

Rechi · 2019-09-26T09:56:37Z

xbmc/cores/VideoPlayer/VideoPlayerVideo.cpp

+    m_fFrameRate = codec ? codec->GetFramerate() : 0.0f;
+    if (m_fFrameRate < 1.0E-6)
+      m_fFrameRate = DVD_TIME_BASE / CDVDCodecUtils::NormalizeFrameduration(
+      (double)DVD_TIME_BASE * hint.fpsscale / hint.fpsrate);


use C++ style cast

zmmr · 2019-10-02T01:25:25Z

0

…

---Original--- From: "Markus Pfau"<notifications@github.com> Date: Wed, Oct 2, 2019 05:44 AM To: "xbmc/xbmc"<xbmc@noreply.github.com>; Cc: "Subscribed"<subscribed@noreply.github.com>; Subject: Re: [xbmc/xbmc] [DemuxClient] Parse for mid stream changes (optional) (#16642) @peak3d commented on this pull request. In xbmc/cores/VideoPlayer/DVDCodecs/Video/DVDVideoCodecAndroidMediaCodec.cpp: > @@ -1042,6 +1042,16 @@ unsigned CDVDVideoCodecAndroidMediaCodec::GetAllowedReferences() return 4; } +float CDVDVideoCodecAndroidMediaCodec::GetFramerate() const +{ + int fpsRate = m_hints.fpsrate; + if (m_hints.fieldOrder > AV_FIELD_PROGRESSIVE) + fpsRate *= 2; Thx @CiNcH83, can you provide a some second snippet of "The Mummy" pls ? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

peak3d · 2019-10-07T15:23:24Z

@CiNcH83 I removed the interlaced handling for fps doubling completely because it seems that information provided in container are often wrong.

CiNcH83

OK. Yes, container information being wrong is indeed another problem. But the *2 thingy has just been as wrong...

As for mid-stream change handling, I can't really test it unfortunately as it is not triggered for file. A sample can be found here.

[EDIT]
PR currently not compiling anyway.

CiNcH83 · 2019-10-07T16:25:37Z

xbmc/cores/VideoPlayer/DVDInputStreams/InputStreamPVRBase.cpp

@@ -344,6 +344,9 @@ void CInputStreamPVRBase::UpdateStreamMap()
      dStream = std::make_shared<CDemuxStream>();

    dStream->codec = (AVCodecID)stream.iCodecId;
+    if (dStream->codec == AV_CODEC_ID_MPEG2VIDEO)


PAR changes may also happen for H.264, see here and here.

jenkins4kodi · 2019-12-13T08:38:24Z

@peak3d this needs a rebase

phunkyfish · 2020-03-09T15:29:25Z

@peak3d do you want to progress this, close it or put it on the backburner?

peak3d · 2020-03-09T15:41:27Z

Back burner pls.

phunkyfish · 2020-03-09T16:34:24Z

PR has merit but will labelled PR Cleanup: Backburner and closed for now. Would be a candidate for inclusion down the line if someone wished to take it on.

If the author would like it kept open please just request so on the PR.

peak3d added Type: Fix non-breaking change which fixes an issue Type: Improvement non-breaking change which improves existing functionality Component: Video v19 Matrix labels Sep 18, 2019

peak3d added this to the Matrix 19.0-alpha 1 milestone Sep 18, 2019

peak3d force-pushed the demux branch from 8199e62 to b3b0c49 Compare September 22, 2019 20:15

peak3d changed the title ~~[DemuxClient] 2x fps for interlaced video / implement stream change check~~ [DemuxClient] Parse for mid stream changes (optional) Sep 22, 2019

peak3d removed the Type: Fix non-breaking change which fixes an issue label Sep 22, 2019

peak3d force-pushed the demux branch from b3b0c49 to b41d8f7 Compare September 22, 2019 20:28

Rechi reviewed Sep 23, 2019

View reviewed changes

xbmc/cores/VideoPlayer/DVDCodecs/Video/DVDVideoCodec.h Show resolved Hide resolved

peak3d force-pushed the demux branch 3 times, most recently from 4b1562b to 95a9e1a Compare September 23, 2019 14:19

CiNcH83 reviewed Sep 24, 2019

View reviewed changes

Rechi reviewed Sep 26, 2019

View reviewed changes

jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Oct 2, 2019

[DemuxClient] Parse for mid stream changes (optional)

140c048

peak3d force-pushed the demux branch from 95a9e1a to 140c048 Compare October 7, 2019 15:19

jenkins4kodi removed the Rebase needed PR that does not apply/merge cleanly to current base branch label Oct 7, 2019

CiNcH83 reviewed Oct 7, 2019

View reviewed changes

jenkins4kodi added the Rebase needed PR that does not apply/merge cleanly to current base branch label Dec 13, 2019

phunkyfish added the PR Cleanup: Ask Author PR appears to still be relevant and mergeable. Comment to author to check on status. label Mar 9, 2020

phunkyfish added PR Cleanup: Backburner This PR has merit as a future feature but has gone stale or is outdated. Label implies closed and removed PR Cleanup: Ask Author PR appears to still be relevant and mergeable. Comment to author to check on status. labels Mar 9, 2020

phunkyfish closed this Mar 9, 2020

Rechi removed this from the Matrix 19.0-alpha 1 milestone Mar 10, 2020

Rechi removed the v19 Matrix label Mar 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DemuxClient] Parse for mid stream changes (optional) #16642

[DemuxClient] Parse for mid stream changes (optional) #16642

peak3d commented Sep 18, 2019 •

edited

ksooo commented Sep 19, 2019

FernetMenta commented Sep 19, 2019

FernetMenta commented Sep 19, 2019

peak3d commented Sep 19, 2019 •

edited

peak3d commented Sep 19, 2019 •

edited

CiNcH83 commented Sep 19, 2019

peak3d commented Sep 19, 2019 •

edited

CiNcH83 commented Sep 19, 2019 •

edited

peak3d commented Sep 19, 2019 •

edited

CiNcH83 commented Sep 19, 2019

peak3d commented Sep 19, 2019

FernetMenta commented Sep 19, 2019

CiNcH83 commented Sep 19, 2019

FernetMenta commented Sep 20, 2019

CiNcH83 Sep 23, 2019

peak3d Sep 24, 2019

CiNcH83 Sep 25, 2019 •

edited

CiNcH83 Sep 25, 2019 •

edited

peak3d Oct 1, 2019 •

edited

CiNcH83 Oct 2, 2019 •

edited

CiNcH83 commented Sep 24, 2019

peak3d commented Sep 24, 2019

Rechi Sep 26, 2019

Rechi Sep 26, 2019

zmmr commented Oct 2, 2019 via email

peak3d commented Oct 7, 2019

CiNcH83 left a comment •

edited

CiNcH83 Oct 7, 2019

jenkins4kodi commented Dec 13, 2019

phunkyfish commented Mar 9, 2020

peak3d commented Mar 9, 2020

phunkyfish commented Mar 9, 2020

[DemuxClient] Parse for mid stream changes (optional) #16642

[DemuxClient] Parse for mid stream changes (optional) #16642

Conversation

peak3d commented Sep 18, 2019 • edited

Description

Motivation and Context

How Has This Been Tested?

Types of change

ksooo commented Sep 19, 2019

FernetMenta commented Sep 19, 2019

FernetMenta commented Sep 19, 2019

peak3d commented Sep 19, 2019 • edited

peak3d commented Sep 19, 2019 • edited

CiNcH83 commented Sep 19, 2019

peak3d commented Sep 19, 2019 • edited

CiNcH83 commented Sep 19, 2019 • edited

peak3d commented Sep 19, 2019 • edited

CiNcH83 commented Sep 19, 2019

peak3d commented Sep 19, 2019

FernetMenta commented Sep 19, 2019

CiNcH83 commented Sep 19, 2019

FernetMenta commented Sep 20, 2019

CiNcH83 Sep 23, 2019

Choose a reason for hiding this comment

peak3d Sep 24, 2019

Choose a reason for hiding this comment

CiNcH83 Sep 25, 2019 • edited

Choose a reason for hiding this comment

CiNcH83 Sep 25, 2019 • edited

Choose a reason for hiding this comment

peak3d Oct 1, 2019 • edited

Choose a reason for hiding this comment

CiNcH83 Oct 2, 2019 • edited

Choose a reason for hiding this comment

CiNcH83 commented Sep 24, 2019

peak3d commented Sep 24, 2019

Rechi Sep 26, 2019

Choose a reason for hiding this comment

Rechi Sep 26, 2019

Choose a reason for hiding this comment

zmmr commented Oct 2, 2019 via email

peak3d commented Oct 7, 2019

CiNcH83 left a comment • edited

Choose a reason for hiding this comment

CiNcH83 Oct 7, 2019

Choose a reason for hiding this comment

jenkins4kodi commented Dec 13, 2019

phunkyfish commented Mar 9, 2020

peak3d commented Mar 9, 2020

phunkyfish commented Mar 9, 2020

peak3d commented Sep 18, 2019 •

edited

peak3d commented Sep 19, 2019 •

edited

peak3d commented Sep 19, 2019 •

edited

peak3d commented Sep 19, 2019 •

edited

CiNcH83 commented Sep 19, 2019 •

edited

peak3d commented Sep 19, 2019 •

edited

CiNcH83 Sep 25, 2019 •

edited

CiNcH83 Sep 25, 2019 •

edited

peak3d Oct 1, 2019 •

edited

CiNcH83 Oct 2, 2019 •

edited

CiNcH83 left a comment •

edited