Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix for VFR files play issue (ticked #12012 i've opened). #1387

Closed
wants to merge 2 commits into from

8 participants

@jpsdr

Signed-off-by: jpsdr jpsdr.psx@free.fr

@jpsdr

Ok. Resume properly the pull request #1369. I shouldn't mess things this time...

@FernetMenta
Collaborator

As far as I can see this patch does still sacrifice existing well working behavior for a new one. Note there is plenty of material with variable frame rate which does not show this bad behavior. In this cases you want to have refresh rate switched to a lower rate regardless how often this occurs.

BTW: We use gnu code style, this is no tabs.

@jpsdr

@FernetMenta
Beleive me : If you have a device wich take several seconds to sync to new display mode, as it's often, not all, but often, the case on TV, wich results in several seconds of black screen, you don't want to have refresh rate switched often while you're watching !!
Apparently you are not concerned by this, and good for you. Just out of curiosity : What do you use to watch ?

And when you look at the code, you'll see that after a while (when "m_iFrameRateLength" reach 128, so after 8 streps), frame rate is apparently considered stable, and not adjusted again.

@jpsdr

After a little testing, i've noticed that VFR information provided by mediainfo is not reliable : Files are sometimes tagged as VFR when in practice they are not. So, detection of VFR situation can only be done "on the fly", and any information tagged in stream can't be considered reliable.

Actualy, i've noticed two kind of "true" VFR files :

  • Those like the one i've provided, which have frame rate changing all the time, even several times during one second.
  • Those which have things like : Opening at 29.97fps, episode at 23.976fps, ending at 29.97fps. You can also consider it can exist the oposite (23.976/29.97/23.976 -> Claymore will fit perfectly on this). Of course, when ending occurs, the algorithm have stopped since a long time (m_iFrameRateLength have reached 128 since a long time).
@FernetMenta
Collaborator

you'll see that after a while (when "m_iFrameRateLength" reach 128, so after 8 streps), frame rate is apparently considered stable, and not adjusted again.

Indeed, IMHO this isn't correct either. Considering your second case and the opening is long enough it won't switch. All I want to say is that we should distinguish between those 2 cases you mentioned and handle both differently. I played your sample and the switches just occur by chance, actually the frame rate is changing all the time like you said. For those cases it might be best to switch to highest available frame rate.

@jpsdr

Unless you do some kind of a full analysis of the file at the opening, you can't distinguish both cases on the fly. Because the 1rst case may allways have a situation you can mix with second. I've begin to make a lot of sought about it, there is no ideal solution. The best, or "least worst" i've think about is to lock on the highest frame rate detected, and detected quick enough => do not increase m_iFrameRateLength, and so don't stop the algorithm. Add where frame rate is calculed from time stamp, so detection of max frame rate.
When mature, i think i'll propose an update.

@FernetMenta
Collaborator

Playing your sample you can observe that pullup correction has trouble, it continuously loses the pattern. Don't you think this is sufficient to distinguish those 2 cases?

@jpsdr

I don't know for sure, but it seems a very interesting idea. It can lead to a detection of what could be considered "Case 1".
As my knowledge for now of xbmc is near noob, do you mean having in the log several things like "13:17:44 T:2796 DEBUG: CPullupCorrection: pattern lost on diff 41000.000000", and so, try to figure out something inside "CPullupCorrection::Add" using this information ?

Edit : Or the number of time it detects a new pattern : We can see things like "12:49:39 T:6708 DEBUG: CPullupCorrection: detected pattern of length 1: 41708.33, frameduration: 41708.333333".
Well, there is appartently interesting things to check/track in "DVDTSCorrection".
That's what i've intended to do in my next step. Slower, having less time now that my vacations will be over.

So, for now, i think we could label VFR files as :

  • Case 1 : Those like the one i've provided, which have frame rate changing all the time, even several times during one second.
  • Case 2 : Those which have things like : Opening at 29.97fps, episode at 23.976fps, ending at 29.97fps. You can also consider it can exist the oposite (23.976/29.97/23.976 -> Claymore will fit perfectly on this). Of course, when ending occurs, the algorithm have stopped since a long time (m_iFrameRateLength have reached 128 since a long time).

One idea would be to have an efficient and quick way to detect "Case 1", and in that case, allow only to go up on frame rate changes.

@jpsdr

It seems i don't screwed things...

Here a new proposal.

So, i've made some tests, logging the full play of several files, and result is :
For VFR files, there is around 100 loss/detection of pattern for a full episode.
For no VFR files, there is 1 detection and 0 loss.

The purpose is to detect as soon as possible VFR case, not tacking minutes. I've noticed that it took around 30s to have 3 loses of pattern, so, i've decided to put the threshold to 3. This make around 30s to achieve detection, it's not too quick, and not too long. Higher value will take too long.

I've also changed the fps validation in from 100 to 120. Case is very very rare, but i've encounter files with 120fps. So, 120 is not an invalid value.

@jpsdr

I've made a few tests. No regression noticed for now, and detection of VFR situation is quick enough.

@FernetMenta
Collaborator

@jpsdr
Thanks for the update. Just to inform you that this is not ignored, people are just busy.
In the meantime you can improve your git skills :)

@jpsdr

Long... long... the busy...

@FernetMenta
Collaborator

I don't think just observing pattern lost is enough to presume VFR. A toggling deinterlace flag would cause pattern lost as well. In this case you don't want to switch to max fps. Maybe it should also compare to previous results.

@jpsdr

Can you explain you're idea a little more ? Compare what to... what..?
And don't forget the time, as described in post... Grrr... No nombers on post !... Post 8 up, don't count the commit block. So, it's useless if half of the episode is already (bad) passed when system decide to go ok. I'm using this since... almost one year ! Never noticed unwanted effect. Maybe i never get the kind of file you're talking to. If you can provide me one, or give me a link/torrent/newsgroup to download it, i'll be very happy to test to see what happens with it.

@jpsdr

I forgot. While the ticket number 14499 i've opened here : http://trac.xbmc.org/ticket/14499 is not resolved, no work can be done anymore for VFR files (or at least, those i have...).

@FernetMenta
Collaborator

Regarding 14499 I will look at it. On Windows the threads die silently after an exception.

Can you explain you're idea a little more ? Compare what to... what..?

Consider e.g. live tv, 50Hz interlaced. If the interlace flag is set, some decoders like vdpau output 50 fps. If the flag is not set 25 fps. Both cases get rendered at 50hz display refresh rate. Toggling between 25 and 50 is not a problem. In this case you don't want to set refresh rate to max fps.

@jpsdr

Can you provide me a file like this, i still can test it with my old still working version to see what is happening. Toggling fps is a problem !! Maybe not on PC monitor, but on "real TV", HDMI resync take several seconds. So, you may don't mind, but personnaly, having constanstly a black screen with no sound during 2 to 4s during the play of a file because fps toogles, will realy piss me off very quick !!!

@FernetMenta
Collaborator

You missed the point. Currently there is no change in refresh rate when switching between 25 fps and 50 fps. Also a missing frame in a stream can cause a pattern lost. Losing the pattern is NO indicator for VFR.

@jpsdr

Ah, Ok. From my very very little knowledge of XBMC and video decoding, that was the only thing noticeable in debug log. Also, looking at the part of the code : Where adjust of framerate is made, it seemed a good and esay way to correct incorrect behavior i had with theses files. As i said, i'm using this for a year now, and never noticed a downside effect. As said : It takes around 20s to have 3 patterns lost (the threshold i've choosen), i don't how missing frame will cause a pattern lost, don't know how will cause 3, and even more, if framerate don't change, the change of display will not trig. And also, as it can be seen in the code, after a while, the algorithm where i've put my part stop itself. So, you don't have VFR <=> Losing pattern, but you have VFR => Losing pattern. After a year of use, i can say that it's working, and never noticed side effect. Maybe there is something better which have VFR <=> ????, but what i've offered here is the only thing i can do with my little knowledge of xbmc. You don't have Losing pattern => VFR, but as you have VFR => Losing pattern, using it with some safety, like the threshold, is apparently doing the job, at least, it has done it for me since a year.

@FernetMenta
Collaborator

You have to consider not only your use cases. Add the following code to CDVDPlayerVideo::OutputPicture, line 1142. It simulates missing frames. You will see that your method of detecting VFR fails.

  static int counter = 0;
  counter++;
  if(counter > 200)
  {
    counter = 0;
    return EOS_DROPPED;
  }

In order to get this reliable I think it needs to take more variables into account: values of detected frame rate, reason for having lost pattern: new pattern detected or just lost old one, maybe exclude CFR from detection, etc.
I don't have a solution either. I am just saying that it must not harm existing behavior.

@jpsdr

Ok, thanks for these informations. If i've time in the future (i don't have now), i'll try to see if i have ideas and understand a little more how xbmc works. But for now, i can't test your exemple, because my standard/dev PC, where i compile xbmc and i can easely (and not very hard/painfully) make test is under Windows XP. So, i'm stuck with the ticket 14408 i've opened here : http://trac.xbmc.org/ticket/14408
But, who knows, maybe with luck, if you can take a look and solve 14499, it may by side effect solve 14408, because both occured at the same time.

@elupus elupus was assigned
@MartijnKaijser

@FernetMenta @elupus
anything that needs to be done on this?

@FernetMenta
Collaborator

@MartijnKaijser nothing has changed here. this pr does not provide a reliable method for vfr detection which has the impact of regression for other use cases. it would be nice to have vfr detection but the demand seems not big enough for someone pushing this forward.

@jpsdr

Can you provide me a link to a file (or several links to several fils) where this regression happen ? I'm using this PR on my personnal builds since i've created it, but never encounter an issue, which means, i probably don't have this kind of file in the files i'm watching. I would like to see on my build on my TV what happens with these files.
Thanks.

@FernetMenta
Collaborator

I already told you how to simulate a missing frame which is not uncommon for live tv.

@jpsdr

I don't want to simulate, i want a real existing file, which exhibit real world happening behavior, to see what happen in this case. Simulating may be interesting if you have the statistic behavior pattern of dropped frame in live broadcast, otherwise, it's not realy relevant.

@FernetMenta
Collaborator

You don't have to simulate. We don't have to care about this feature. If you want to see progress on this you can try harder or leave it.

@jpsdr

... I'm even less convinced of the regression issue. As more than 80% of the files i'm watching come from tv broadcast... Unless you tell me that you've already tested this PR and personnaly noticed issue on files you've played, in that case, i'll frankly trust your words, and stop with Ok ! Otherwise, this is too much "It may theoricaly provide some issues on some eventualy files which may exists somewhere... in the world..."
For now, i see things like this :
From a theorical point of view, there may be some cases where this PR may be wrong. I'll never deny this, and it's too often the case. Question is : Are these theorical situations happening in real life ? Are their probability high enough ?
I'm testing this PR since i've created it, more than a year, and never encounter an issue. So, according my datas, i can answer you : There is no wories about what you were afraid of.
You tell me an articial way to disturb the play. Too much articial, this is not happening, it's not relevant. You are worried about something which doesn't happend in real files. To avoid misunderstanding, i mean files which trig incorrect behavior, i never said dropped frames inside files are not happening in real life files.
Try harder to fix something which is not happening on real files, what the point ? I've more than a year of testing/use without issue.
For this case, i prevail testing and see if there is occurance instead of pure eventualy theorical situation. And result of testig is that there is no occurance. No point trying to do immeasurably complex when simple works.
Now, if someone else has implemented and tested this PR, and encounter files which behave improperly, this is in this case, of course, a total different story !

@jpsdr

Good night sleep help think about details.
Don't forget already original existing behavior !! If the files you're talking have drop enough to trig the frame rate detection change, this files will constantly generate a change on frame rate. Otherwise nothing will happen.
With this PR, if the files you're talking have drop enough to trig the frame rate detection change, this files will generate a change on frame rate only a few time (and even maybe once). Otherwise nothing will happen.
And issue with change of frame rate is that on some TV, you can have 2s to 4s of black screen the time HDMI resync.

@FernetMenta
Collaborator

Missing frames on live tv is not a theoretically constructed case.

@jpsdr

If you read me, i've never said that, and i insisted on this fact.
"To avoid misunderstanding, i mean files which trig incorrect behavior, i never said dropped frames inside files are not happening in real life files."
And question again, for this case, what is actual behavoir doing ? If nothing happens/is trigged, nothing will change with this PR.

@jpsdr

In DVDPlayerVideo.cpp, in the end of CDVDPlayerVideo::CalcFrameRate() we can see :

  //we're allowed to drop frames because we calculated a good framerate
  m_bAllowDrop = true;
}

And up, there is :

  //if we're calculating the framerate,
  //don't drop frames until we've calculated a stable framerate
  if (m_bAllowDrop || m_speed != DVD_PLAYSPEED_NORMAL)
  {
    result |= EOS_VERYLATE;
    m_pullupCorrection.Flush(); //dropped frames mess up the pattern, so just flush it
  }

So... if i understand properly, pattern is flushed in case of dropped frame, and so data are not used to compute framerate, so, dropped frame can't generate an incorrect frame rate detection. Have i understood properly ?

@FernetMenta
Collaborator

Have i understood properly ?

no, we distinguish between actively dropped frames here and missing frames in the stream. You may have a bad signal on DVB-T or DVB-S. On another case the interlace flag may toggle which results in alternating 2 or 1 frame. There are more cases ...

@jpsdr

I have a question, because i may have misunderstood one point, and i begin to wonder, viewing your answer. The live TV you're talking, they are files recorded from TV decoders, right...? Or... i've missed some point...??? (As i don't know what DVB-T/S is, i begin to have a big doubt...).

@fritsch
Collaborator

No. Direct LiveTV, streamed over the air. You have a TV Card in your backend server which serves the video stream (be it h264 / mpeg2, some interlaced, others progressive) for you.

This is LiveTV integration or formerly xbmc-pvr is all about: https://dl.dropboxusercontent.com/u/55728161/xbmc-pvr.png

You directly get the stream over the air, no local files nothing. If wind blows or snow storms, you can see that in the data quality :-)

@jpsdr

Hmm... This is a feature i didn't know and have no idea about. And so, this is also something i'm absolutely unable to test and see how the PR behave with !! Again, before trying ultra complex things, i want to know how the PR behave with this situation. Anyone who can provide a debug log file using this kind of system with my PR is welcome... On the other hand, a fix may be very easy. Is there a way with the constructor CDVDPlayerVideo::CDVDPlayerVideo() or one of the CDVDPlayerVideo::OpenStream() to know if you're playing a file ? Restricting it only when a file is played will solve the issue.
EDIT : Or, is there an easy way to detect missing frames, like it's allready done, and flush the pattern detection ? By the way, are you sure it's not somehow something already done in the existing code, and so we are worried for nothing ?
EDIT2 : I hope VFR files are not working by generating missing frames...

@jpsdr

Where in CDVDPlayerVideo::OutputPicture are detected/handled missing frames ? I've not been able to figure out.
The pattern for pullup correction is already flushed in two places in CDVDPlayerVideo::OutputPicture to prevent incorrect behavior detection, maybe it will not create incorrect patter.
On the other hand, the suggestion of simulating missing frame is 1/200, so, i said around 1%. We are talking here of an eventual issue specificaly related to broadcast TV, so were talking here of only 25fps or 29.97fps, and of a situation of something normaly watchable, not a situation with 50% of missing frames. Missing frame may only result on a lower estimated frame rate (if i'm wrong with this point, correct me).
If previous statement is not wrong : As my patch try to keep the framerate on the highest detected, i think it will instead improve actual behavior. In case of around 1% of dropped frames, as frame rate is normalized with CDVDCodecUtils::NormalizeFrameduration, 25 reduce of 1% -> 24.75, more close than 25 to 24, so no issue with actual and mine. But with 5% of dropped frames, 25 -> 23.75, so actual behavior may try to change framerate to 23.976 (or 24), while mine will keep framerate to 25.

@elupus
Collaborator

We don't drop frames in dvdplayer anymore due to lateness. Only on output if we have replacement frames ready that are better.

@FernetMenta
Collaborator

@jpsdr you did not get the point. Just a couple of missing frames can make your code incorrectly detect VFR and switch from 50hz to 60hz.

@jpsdr

The fact is that i don't understand "how" => the process. What's happening "in the code", and where the fact that there is missing frames can be detected... My knowledge of this part is too low.
I mean, what's happing ? XBMC expect from TV receiver a frame each x ms. For exemple, at 25fps it expect : frame - 40ms - frame - 40ms ... If there is a missing frame, you'll have, for exemple : Frame - 40ms - Frame - 40ms - nothing - 40ms - Frame - 40ms. In the first place, i don't understand how you can think frame rate is higher. If you mesure the time between frames : It increases because you have 2 at 40ms and one at 80ms, and if you count the number of frames per second, it decreases. Unless there is something in the process i understand wrongly, and it's possible, missing frame can only result in a lower frame rate detected. Switching from 60Hz to 50Hz is not possible, because it lock on highest frame rate. BUT, actual behavior can make it switch from 60Hz to 50Hz, and even stay on 50Hz if you're unlucky. And again, just as i've just explained why, missing frames can ONLY generate a lower frame rate => You CAN'T switch from 50Hz to 60Hz.
Now, if i'm wrong when saying that "missing frames can ONLY generate a lower frame rate", tell me and explain me why.
You said that i miss the point, but, from me it seems (maybe wrongly) that you don't even read what i'm writing, you never tell me where i'm wrong in what i'm saying. I try to explain and demonstrate things about the issue you're concerned with, but you don't say "no, this point x is not correct because...", or "This point y is correc", you just always keep repeating the issue you're concerned about.
My actual points are, for the specific case of missing frames :

  • With actual behavior it can swith from 60Hz to 50Hz, and even stay locked on it if you're unlucky (change of frame rate made on the last time current algorithm test). Do you agree or not ? If not, why ?
  • Missing frames can only result in a lower estimated frame rate. Do you agree or not ? If not why ?
  • Considering the both previous facts, my PR can only improve things.
@davilla
Collaborator

G+1 material

@jpsdr

Sorry... What does "G+1 material" mean ? What is "G+1" ?

@opdenkamp
Collaborator

something for the version after Gotham

@jpsdr

Ah, Ok. But before becoming any material for anything, i would like to have my answers...

@jpsdr

I'm still willing to see and think if i've ideas to how to improve things, but, i can't do anything more until i'll have the answers to the questions i've asked. Unless, there is no one who knows the answers to these questions ?

@FernetMenta
Collaborator

I have not even given you the answers but also the scenarios your code would fail but you don't seem to listen. For tv the average frame rate is not of interest but frame duration. If frame duration is 20ms, it best runs on 50Hz regardless of how many frames missing.

1)
suppose you start a live stream and pc detects a pattern and frame duration of 20ms. then some frame are missing -> pattern lost -> pc detect pattern -> pattern lost. and so on.
switching to 60Hz in this case is wrong and would definitely decrease quality.

2)
suppose interlaced flag toggles and pc detects alternating 25hz and 50hz. Assuming VFR in this case is wrong.

There may be more cases where this code causes regression.

@jpsdr

I'm willing to listen, but the more you explain things, the more i fell (maybe wrongly) you give credit to my code...
Case 1
Why would it switch to 60Hz ? Can the missing frames resulting in xbmc detect a frame duration lower than 20ms ? That's the ONLY point. If not, there is no reason to switch to 60Hz, with my code or even with the actual code. There is 4 possibilties.
1 : Missing frames can result only in xbmc detecting a frame duration lower than 20ms... 17ms for exemple ? In that case, effectively, my code will probably lock on 60Hz. Actual code will switch from 60Hz to 50Hz for finaly stay on... the last when the actual code decide to stoping.
2 : Missing frames don't affect the calcul of frames duration for xbmc. Actual code or mine : The same.
3 : Missing frame can only result in xbmc detecting a frame duration higher, so only lower frame rate. My code will lock on the correct frame rate, actual code will switch and finaly stay on... the last when the actual code decide to stoping.
4 : Missing frame can result in xbmc detecting a frame duration higher or lower, result same as case 1.
So, question : In what case are we ?
Again, i repeat, you said switching to 60Hz is wrong. Yes, i agree, you're right. But according the situation we are, it will simply not switch to 60Hz, impossible. So, i've only one simple question which needs only one simple answer : The number of the situation we are.
Case 2
If pattern detection result in pattern lost, and, frame duration detected switch from 20ms to 40ms, the result is the following :

  • My code, it will lock on 50Hz.
  • Actual code, it will switch from 25Hz to 50Hz. According display device capacity, it may have no visual effect : xbmc configure display to 50Hz for both 25Hz or 50Hz video, so, there will be even no switch. Well, in worst case, there is switching, and HDMI resynch take some seconds, so, during playback, it's not nice to have this switching. So, yes, assuming VFR may be wrong, but, it has yet to assume VFR, and even if it does, is the actual behavior with the risk of switching frame rate (so creating several HDMI resynch and so several black screen during the play) better than locking on 50Hz ?

Again another simple question.
I hope we will stop turning in circle. For now, i need only the simple answers at the two simple questions. No more.

@FernetMenta
Collaborator

Maybe you are right and I have misinterpreted your code. On Gitub it is hard to review due to missing line breaks, wrong indentation, etc.
Please adapt coding style: no tabs, indentation, space between operators line "=" and variable. Also add some comments, then we take a new round after 13.0 release.

@jpsdr

Ok, fine by me.
No tabs, it's should theoricaly already be like this, unless notepad++ screwed me things. I'll check the other things later. There will be time.

@t-nelson t-nelson added the Helix label
@MartijnKaijser

@jpsdr can you rebase?

@jpsdr

Done.

@FernetMenta
Collaborator

ouwld be nice if you could care about the cosmetics: asterisc after xbmc.org, tabs instead of white spaces, etc. It's obvious when looking at the diff.

@elupus I misinterpreted this code last time. I tried it and it seem to work. What do you think?

@jpsdr

I'll take a look later at the cosmetic part you pointed, didn't have time for now. Thanks for testing. I even can't anymore because i can't compile anymore... :(
Edit : Where is the post where you'd pointed me some errors ?

@jpsdr

Argh.... I'm affraid i've done a wrong thing beeing half asleep....

@FernetMenta
Collaborator

what does that mean?

@jpsdr

I'm affraid i've messed-up my branch, hitting "pull" instead of "fetch and rebase" on tortoise git, now, everything seems screwed up. I'll probably have to re-create it. It will be a good time to take care of cosmetic issues (tab/space, etc...).

@jpsdr

Ok, i've redone the branch. It should be back to Ok, and, all cosmetics issues (tab, etc...) should be fixed. I hope...
No other feedback for the moment ?

jpsdr added some commits
@jpsdr jpsdr Fix for VFR files play issue (ticked #12012 i've opened).
Signed-off-by: JPSDR <jpsdr.psx@free.fr>
bfcd268
@jpsdr jpsdr Cosmetic updates.
Signed-off-by: JPSDR <jpsdr.psx@free.fr>
f02dc52
@jpsdr

Ok, update done. I hope i've forgot nothing, and no insidious tabs were inserted.
I can't compile anymore (as i can't install VS2013), so, if you can check it's still compiling and still working (i've not made mistake when re-doing the branch), it would be nice...

@FernetMenta
Collaborator

merged in #5319

@FernetMenta FernetMenta closed this
@tru tru referenced this pull request from a commit in plexinc/plex-home-theater-public
@tru tru Improve the PIN input dialog.
Now it doesn’t try to switch if the PIN is incorrect and has quite a
bit more error handling. Fixes #1387 which was caused by the fact that
when you entered that dialog it automatically logged you out.
db14e0c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 3, 2014
  1. @jpsdr

    Fix for VFR files play issue (ticked #12012 i've opened).

    jpsdr authored
    Signed-off-by: JPSDR <jpsdr.psx@free.fr>
  2. @jpsdr

    Cosmetic updates.

    jpsdr authored
    Signed-off-by: JPSDR <jpsdr.psx@free.fr>
This page is out of date. Refresh to see the latest.
View
10 xbmc/cores/dvdplayer/DVDPlayerVideo.cpp
@@ -190,6 +190,7 @@ bool CDVDPlayerVideo::OpenStream( CDVDStreamInfo &hint )
formats = g_renderManager.SupportedFormats();
#endif
+ m_pullupCorrection.ResetVFRDetection();
if(hint.flags & AV_DISPOSITION_ATTACHED_PIC)
return false;
@@ -231,6 +232,7 @@ void CDVDPlayerVideo::OpenStream(CDVDStreamInfo &hint, CDVDVideoCodec* codec)
m_bFpsInvalid = (hint.fpsrate == 0 || hint.fpsscale == 0);
+ m_pullupCorrection.ResetVFRDetection();
m_bCalcFrameRate = CSettings::Get().GetBool("videoplayer.usedisplayasclock") ||
CSettings::Get().GetInt("videoplayer.adjustrefreshrate") != ADJUST_REFRESHRATE_OFF;
ResetFrameRateCalc();
@@ -238,7 +240,7 @@ void CDVDPlayerVideo::OpenStream(CDVDStreamInfo &hint, CDVDVideoCodec* codec)
m_iDroppedRequest = 0;
m_iLateFrames = 0;
- if( m_fFrameRate > 100 || m_fFrameRate < 5 )
+ if( m_fFrameRate > 120 || m_fFrameRate < 5 )
{
CLog::Log(LOGERROR, "CDVDPlayerVideo::OpenStream - Invalid framerate %d, using forced 25fps and just trust timestamps", (int)m_fFrameRate);
m_fFrameRate = 25;
@@ -1483,9 +1485,11 @@ void CDVDPlayerVideo::CalcFrameRate()
//see if m_pullupCorrection was able to detect a pattern in the timestamps
//and is able to calculate the correct frame duration from it
double frameduration = m_pullupCorrection.GetFrameDuration();
+ if (m_pullupCorrection.VFRDetection())
+ frameduration = m_pullupCorrection.GetMinFrameDuration();
- if (frameduration == DVD_NOPTS_VALUE ||
- (g_advancedSettings.m_videoFpsDetect == 1 && m_pullupCorrection.GetPatternLength() > 1))
+ if ((frameduration == DVD_NOPTS_VALUE) ||
+ ((g_advancedSettings.m_videoFpsDetect == 1) && ((m_pullupCorrection.GetPatternLength() > 1) && !m_pullupCorrection.VFRDetection())))
{
//reset the stored framerates if no good framerate was detected
m_fStableFrameRate = 0.0;
View
61 xbmc/cores/dvdplayer/DVDTSCorrection.cpp
@@ -26,14 +26,24 @@
#include <cmath>
#define MAXERR DVD_MSEC_TO_TIME(2.5)
+#define MINVALIDFRAMEDURATION DVD_MSEC_TO_TIME(8.33) // For VFR detection, max framerate allowed is 120fps
+#define MAXVALIDFRAMEDURATION DVD_MSEC_TO_TIME(200.0) // For VFR detection, min framerate allowed is 5fps
using namespace std;
CPullupCorrection::CPullupCorrection()
{
+ ResetVFRDetection();
Flush();
}
+void CPullupCorrection::ResetVFRDetection(void)
+{
+ m_minframeduration = DVD_NOPTS_VALUE;
+ m_maxframeduration = DVD_NOPTS_VALUE;
+ m_VFRCounter = 0;
+}
+
void CPullupCorrection::Flush()
{
m_pattern.clear();
@@ -82,7 +92,8 @@ void CPullupCorrection::Add(double pts)
{
if (m_haspattern)
{
- CLog::Log(LOGDEBUG, "CPullupCorrection: pattern lost on diff %f", GetDiff(0));
+ m_VFRCounter++;
+ CLog::Log(LOGDEBUG, "CPullupCorrection: pattern lost on diff %f, number of losses %i", GetDiff(0), m_VFRCounter);
Flush();
}
@@ -297,17 +308,59 @@ bool CPullupCorrection::CheckPattern(std::vector<double>& pattern)
}
//calculate how long each frame should last from the saved pattern
+//Retreive also information of max and min frame rate duration, for VFR files case
double CPullupCorrection::CalcFrameDuration()
{
if (!m_pattern.empty())
{
//take the average of all diffs in the pattern
- double frameduration = 0.0;
- for (unsigned int i = 0; i < m_pattern.size(); i++)
- frameduration += m_pattern[i];
+ double frameduration;
+ double current, currentmin, currentmax;
+ currentmin = m_pattern[0];
+ currentmax = currentmin;
+ frameduration = currentmin;
+ for (unsigned int i = 1; i < m_pattern.size(); i++)
+ {
+ current = m_pattern[i];
+ if (current > currentmax)
+ currentmax = current;
+ if (current < currentmin)
+ currentmin = current;
+ frameduration += current;
+ }
frameduration /= m_pattern.size();
+ // Update min and max frame duration, only if data is valid
+ if (m_minframeduration == DVD_NOPTS_VALUE)
+ {
+ if ((currentmin >= MINVALIDFRAMEDURATION) && (currentmin <= MAXVALIDFRAMEDURATION))
+ m_minframeduration=currentmin;
+ }
+ else
+ {
+ if ((currentmin < m_minframeduration) &&
+ ((currentmin >= MINVALIDFRAMEDURATION) && (currentmin <= MAXVALIDFRAMEDURATION)))
+ m_minframeduration = currentmin;
+ }
+
+ if (m_maxframeduration == DVD_NOPTS_VALUE)
+ {
+ if ((currentmax >= MINVALIDFRAMEDURATION) && (currentmax <= MAXVALIDFRAMEDURATION))
+ m_maxframeduration = currentmax;
+ }
+ else
+ {
+ if ((currentmax > m_maxframeduration) &&
+ ((currentmax >= MINVALIDFRAMEDURATION) && (currentmax <= MAXVALIDFRAMEDURATION)))
+ m_maxframeduration = currentmax;
+ }
+
+ //frameduration is not completely correct, use a common one if it's close
+ m_minframeduration = CDVDCodecUtils::NormalizeFrameduration(m_minframeduration);
+ m_maxframeduration = CDVDCodecUtils::NormalizeFrameduration(m_maxframeduration);
+ CLog::Log(LOGDEBUG, "CPullupCorrection: min frame duration %f, max frame duration %f", m_minframeduration, m_maxframeduration);
+
//frameduration is not completely correct, use a common one if it's close
return CDVDCodecUtils::NormalizeFrameduration(frameduration);
}
View
9 xbmc/cores/dvdplayer/DVDTSCorrection.h
@@ -24,6 +24,7 @@
#include <vector>
#define DIFFRINGSIZE 120
+#define VFR_DETECTION_THRESHOLD 3
class CPullupCorrection
{
@@ -31,11 +32,15 @@ class CPullupCorrection
CPullupCorrection();
void Add(double pts);
void Flush(); //flush the saved pattern and the ringbuffer
+ void ResetVFRDetection(void);
double GetCorrection() { return m_ptscorrection; }
int GetPatternLength() { return m_patternlength; }
double GetFrameDuration() { return m_frameduration; }
+ double GetMaxFrameDuration(void) { return m_maxframeduration; }
+ double GetMinFrameDuration(void) { return m_minframeduration; }
bool HasFullBuffer() { return m_ringfill == DIFFRINGSIZE; }
+ bool VFRDetection(void) { return (m_VFRCounter>=VFR_DETECTION_THRESHOLD); }
private:
double m_prevpts; //last pts added
@@ -63,7 +68,9 @@ class CPullupCorrection
double m_ptscorrection; //the correction needed for the last added pts
double m_trackingpts; //tracked pts for smoothing the timestamps
double m_frameduration; //frameduration exposed to dvdplayer, used for calculating the fps
- bool m_haspattern; //for the log
+ double m_maxframeduration; //Max value detected for frame duration (for VFR files case)
+ double m_minframeduration; //Min value detected for frame duration (for VFR files case)
+ bool m_haspattern; //for the log and detecting VFR files case
int m_patternlength; //for the codec info
std::string GetPatternStr(); //also for the log
};
Something went wrong with that request. Please try again.