Opened 11 years ago

Closed 10 years ago

Last modified 9 years ago

#5643 closed enhancement (fixed)

Match refreshrate with input framerate to reduce judder.

Reported by: henrik.sorensen@… Owned by: JYA
Priority: minor Milestone: 0.22
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

Film, PAL and NTSC video material all have different frame rates, ideally each should be shown at or a multiple of that frame rate, so each frame is shown the intended amount of time, otherwise slight motion judder occur in scenes where the camera pans and the like.

I made a patch against 0.21fixes that extends the custom video mode code to automagically attempt to choose a refresh rate based on the video frame rate.

Basically I modified the videoout_xv and base class so they require the video fps on initialisation, which nuppelvideoplayer already has at the point it inits a videoout instance. (only base and xv videoout classes are modified in the patch)

Then it's mostly a matter of getting the video fps to DisplayResScreen::FindBestMatch?() which is slightly modified so it also accepts multiples of your video fps, say 50hz for 25fps, 60hz for 30fps etc.

One annoyance I found with auto changing of refresh rates, was that when you switch inputs in live tv, it would switch gui to resolution/refresh only to switch back to the same resolution and refresh rate on the new input.

To work around that I introduced a new setting GuiVidModeRefreshRate? in the style of the other mode settings. If you set that to the likely TV refresh rate (50hz in my case in PAL land), you avoid the switch cycle. Read support for GuiVidModeRefreshRate? is in the patch, but no gui configuration of it.

Attachments (17)

mythtv_refresh_rate_auto_pickv2.patch (8.1 KB) - added by henrik.sorensen@… 11 years ago.
mythtv_refresh_rate_auto_pickv2-trunk.patch (8.1 KB) - added by henrik.sorensen@… 11 years ago.
same patch against trunk rev 18198
mythtv_refresh_rate_auto_pickv3.patch (18.0 KB) - added by henrik.sorensen@… 11 years ago.
0.21 patch
mythtv_refresh_rate_auto_pickv3-trunk.patch (18.0 KB) - added by henrik.sorensen@… 11 years ago.
adjust_rate.diff (37.6 KB) - added by jyavenard@… 10 years ago.
Handle nvidia refresh rates and non-integer frequencies…
adjust_rate_2.patch (39.6 KB) - added by jyavenard@… 10 years ago.
Updated patch for 0.21-fixes. If the exact rate can't be found then will try the nearest int instead. So 23.976 fps will use 24Hz for example
adjust_rate3-fixes.patch (80.1 KB) - added by jyavenard@… 10 years ago.
For 0.21-fixes
adjust_rate3-trunk.patch (37.5 KB) - added by jyavenard@… 10 years ago.
For trunk (20573)
adjust_rate3-fixes.2.patch (78.3 KB) - added by jyavenard@… 10 years ago.
patch for 0.21-fixes.
gen-refresh-rate-map.pl (1.3 KB) - added by Boleslaw Ciesielski <bolek-mythtv@…> 10 years ago.
script to generate refresh rate map file
adjust_rate4-fixes.patch (28.6 KB) - added by jyavenard@… 10 years ago.
Patch for 0.21-fixes
adjust_rate4-trunk20764.patch (29.0 KB) - added by jyavenard@… 10 years ago.
Patch for trunk #20764
adjust_rate5-fixes.patch (226.3 KB) - added by jyavenard@… 10 years ago.
patch for 0.21-fixes with Dynamic TwinView? support
adjust_rate5-trunk20787.patch (225.7 KB) - added by jyavenard@… 10 years ago.
patch for trunk 20787 with Dynamic TwinView? support
adjust_rate6-fixes.patch (233.2 KB) - added by jyavenard@… 10 years ago.
patch for 0.21-fixes
adjust_rate6-trunk20787.patch (234.6 KB) - added by jyavenard@… 10 years ago.
patch for trunk 20787 with Dynamic TwinView? support
modeline-free.patch (392 bytes) - added by Boleslaw Ciesielski <bolek-mythtv@…> 10 years ago.
patch for a bogus free() in GetNvidiaRates?()

Download all attachments as: .zip

Change History (36)

Changed 11 years ago by henrik.sorensen@…

Changed 11 years ago by henrik.sorensen@…

same patch against trunk rev 18198

comment:1 Changed 11 years ago by ylee@…

Sadly, the patch for 0.21-fixes causes mythfrontend to hang after it tries to preview a recording.

comment:2 Changed 11 years ago by henrik.sorensen@…

Confirmed :/ Goes mad with a repeating message. I'll invistigate...

2008-08-27 07:26:21.262 GetNextFreeFrame?() unable to lock frame 100 times. Discarding Frames. 2008-08-27 07:26:21.262 GetNextFreeFrame?() unable to lock frame 100 times. Discarding Frames. 2008-08-27 07:26:21.262 GetNextFreeFrame?() unable to lock frame 100 times. Discarding Frames.

comment:3 Changed 11 years ago by henrik.sorensen@…

I had only modified Videoout and VideooutXV with the new Init argument. Apparently recording previews use VideooutNull?.. so it got the wrong embedid passed to it.

I have now modified all the VideoOutXX::Init calls and hopefully all calls to them. I also modified DisplayRes::SwitchToVideo? so it only uses the video fps as a refresh base if the video mode's refresh rate is set to Any (0), otherwise it switches to whatever rate you set in the gui, as in the old behaviour.

Recording Previews work on 0.21fixes. I can't get previews to work in trunk without any patches for reason, PEBKAC I'm sure, so they aren't tested on trunk.

Changed 11 years ago by henrik.sorensen@…

0.21 patch

Changed 11 years ago by henrik.sorensen@…

comment:4 Changed 11 years ago by ylee@…

I've been testing the patch for a few weeks on my 0.21-fixes setup and so far so good. Due to human nature it's harder to notice something not happening than something happening. It does seem, however, that camera pans in recordings from premium-movie cable channels (notorious for fps constantly changing to anywhere between 24 and 30) are now smooth(er) than without the patch.

comment:5 Changed 10 years ago by jyavenard@…

Hi

I have worked on a patch using this one as a starting point. It also gives a mechanism to properly handle Nvidia's xrandr unique refresh rate. Nvidia drivers by default with DynamicTwinView? will generate a unique refresh rate for all the various resolutions and screen. So the refresh rate returned by the xrandr API doesn't match the effective refresh rate.. For example, on my system xrandr would return:

1920x1080 50.0* 51.0 52.0 53.0 54.0 55.0 56.0 57.0 58.0 59.0 60.0 61.0 62.0 63.0 64.0 65.0 66.0 1680x1050 67.0 68.0 1440x900 69.0 1400x1050 70.0 71.0 1360x768 72.0 73.0 1280x1024 74.0 75.0 76.0

Note that all refresh rates start at 50 and keeps increasing by one.

where 1920x1080 50Hz would be 50Hz 1920x1080 51Hz would be 60Hz 1920x1080 52Hz would be 24Hz etc...

As such, mythtv configuration UI in setting custom TV playback video mode ; will report values that are pretty much useless. It also prevents this patch for correctly determining which refresh rate should be used say if the video is encoded at 24fps when the xrandr effective rate to be used is say 52.

Another issues is that MythTV interface to control the refresh rates are using short. In the US, 24p is really 23.976Hz ; while in PAL countries it would be exactly 24Hz. Most 24p TV would handle both framerates ; however as this refresh rate is so close to 24 .. X11 will only allow you to use on or the other. Not both at the same time.

This patch also adds support for using non integer framerates; like 23.976Hz or 59.94Hz.

Call me anal, but there is a difference with all those rates ; and so far MythTV couldn't use them.

As the rate used by mythtv and XRANDR are integers ; those rates weren't supported before.

In MythTV setup -> Appearance, check Separate Video modes for GUI and TV playback. If for the rate you use "Any" this is when MythTV will try to match the TV refresh rate with the video one.

A new field has been added at the bottom of this configuration screen: Custom screen rate definition file. Here you enter the path to a text file containing the description of the video rate. The format of the file is as follow: width,height,real_rate,xrandr_rate like this: #50 - "1920x1080@50" #51 - "1920x1080@60" #52 - "1920x1080@24" #53 - "1920x1080@23.976" #54 - "1920x1080@50i" #55 - "1920x1080@60i" #56 - "1920x1080@59.94" #57 - "1920x1080@59.94i" #1920x1080 mode: 1920,1080,50,50 1920,1080,60,51 1920,1080,24,52 1920,1080,23.976,53 1920,1080,59.94,56

this will tell mythtv that to use a 60Hz refresh rate, the 51 xrandr rate should be used. Note that it handles floating point rates like 1920x1080 @ 23.976Hz.

If a given resolution is defined in this definition file, it will replace the rates automatically calculated by X11. MythTV setup screen will also use and display the real rate (24, 50, 60 etc..) rather than the rate returned by xrandr (50,51,52 etc)...

I'm sure there should be a way to determine what the real refresh rate is from the xrandr ; but I haven't found how yet.

This patch is for 0.21-fixes only ... Will work on a trunk one soon.

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate.diff added

Handle nvidia refresh rates and non-integer frequencies...

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate_2.patch added

Updated patch for 0.21-fixes. If the exact rate can't be found then will try the nearest int instead. So 23.976 fps will use 24Hz for example

comment:6 Changed 10 years ago by anonymous

Seems to work a treat. I've notice a little jerkiness watching Lost - it selected 60Hz, and I would have probably expected 59.94Hz (right??), although I can't tell what the video says it should be (it's an avi).

Anyway... here's my rates file:

1920,1080,60,50
1920,1080,50,51
1920,1080,59.94,54
1920,1080,24,55

comment:7 Changed 10 years ago by jyavenard@…

If no match can be found, myth selects the highest refresh rate available.

Run mythtv with -v playback and see what's the original video framerate was calculated as.

It may be that it selects 60Hz, for another reason than what you think: like no good refresh rate value is found

comment:8 Changed 10 years ago by jyavenard@…

Ported patch for trunk (20573). Updated patch for 0.21-fixes, remove all tabs to use spaces instead..

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate3-fixes.patch added

For 0.21-fixes

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate3-trunk.patch added

For trunk (20573)

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate3-fixes.2.patch added

patch for 0.21-fixes.

comment:9 Changed 10 years ago by anonymous

Old patch worked great on 0.21 and new patch is working nicely on trunk (0.22)

Thanks!

comment:10 Changed 10 years ago by Boleslaw Ciesielski <bolek-mythtv@…>

Not sure how helpful this is but I am attaching a simple perl script to generate the refresh rate mapping file as required by the patch. The script uses xrandr and xvidtune to cycle through all available modes, so it is a bit of a hack.

One thing I could not figure out was how to represent interlaced modes. For the lack of better ideas, it appends 'i' to the refresh rate, i.e. something like this:

1920,1080,60.00,50
1920,1080,50.00,51
1920,1080,24.00,52
1920,1080,23.97,53
1920,1080,50.00i,54
1920,1080,60.00i,55
1920,1080,59.94,56
1920,1080,59.94i,57
1920,1080,25.00,58
1920,1080,29.97,59
1920,1080,30.00,60
800,600,60.32,61

This is not what the patch expects and it ignores these modes, which may be just as well since it's not clear how well they are supported by nvidia anyway. But I will be happy to modify the script to produce whatever format is expected by the patch.

Changed 10 years ago by Boleslaw Ciesielski <bolek-mythtv@…>

Attachment: gen-refresh-rate-map.pl added

script to generate refresh rate map file

comment:11 Changed 10 years ago by jyavenard@…

Total rework. Much simpler patch, doesn't modify anywhere as much files and leave intact all the various videoout_... files, only modifying the ones that do need change.

Now the custom rate will only be used if the underlying xrandr refresh rate actually exist. This prevents incorrect customrate file switching to non-existing rate, and when using a different screen than the one mythtv was configured for.

Additionally, the new structure will allow much easier integration of automatically calculated the real refresh rate when using nvidia drivers with Dynamic TwinView? enabled.

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate4-fixes.patch added

Patch for 0.21-fixes

Changed 10 years ago by jyavenard@…

Patch for trunk #20764

comment:12 Changed 10 years ago by jyavenard@…

New version; big rework again, finally completing what was missing: ease of configuration.

This one fully supports the nVidia NV-CONTROL extension and will properly populate the real refresh rates when using Dynamic Twinview and switch accordingly.

As such, there is no need to create a customrate file anymore...

Warning: MythTV will use the best refresh rate available as reported by xrandr. So if you have set up some custom modelines in your /etc/X11/xorg.conf that your TV doesn't support, it may try to use those: and your TV will switch to a mode it doesn't support.

Only uses modelines that you *know* your TV/Monitor supports. In particular, remove any 25, 30 and 29.97Hz Custom ModeLines? as I had put in my blog. No TV that I know of supports those.

This is important !

Obviously, if you haven't manually modified your xorg.conf file and let nvidia detects what your TV supports then it will be all fine and it will switch automatically.

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate5-fixes.patch added

patch for 0.21-fixes with Dynamic TwinView? support

Changed 10 years ago by jyavenard@…

patch for trunk 20787 with Dynamic TwinView? support

comment:13 Changed 10 years ago by jyavenard@…

Following talks with Mark Kendall:

-Reformat the code to MythTV's coding guidelines -If framerate > 24.5 fps and < 30.5 fps, then try 2X refresh rate first. This is to make sure 50 or 60Hz interlaced video use 50Hz or 60Hz progressive refresh rate first. Otherwise if the TV advertises 25 or 30Hz progressive, previously this is what Myth would use -Add explanation of the meaning of "Any" in the Settings -> Appearance configuration screen -Change the Settings screen to use floating points for refresh rates rather than integers. -Use type "double" instead of "float" to be consistent with the data type used in DisplayRes? class (use same type as for the aspect ratio)

Ideally, you would want to handle interlaced screen differently, but as there's no good support of interlaced screen in Myth or Xrandr, probably better to handle this in a separate patch.

Changed 10 years ago by jyavenard@…

Attachment: adjust_rate6-fixes.patch added

patch for 0.21-fixes

Changed 10 years ago by jyavenard@…

patch for trunk 20787 with Dynamic TwinView? support

comment:14 Changed 10 years ago by JYA

Owner: changed from Isaac Richards to JYA
Status: newaccepted

comment:15 Changed 10 years ago by JYA

Resolution: fixed
Status: acceptedclosed

Fixed in [21489].

Changed 10 years ago by Boleslaw Ciesielski <bolek-mythtv@…>

Attachment: modeline-free.patch added

patch for a bogus free() in GetNvidiaRates?()

comment:16 Changed 10 years ago by Boleslaw Ciesielski <bolek-mythtv@…>

I am seeing an occasional crash on frontend startup (this is with svn 21491). The attached patch (hopefully) fixes it.

#0  __libc_free (mem=0xd230e2ff) at malloc.c:3599
#1  0x00007ffff5393098 in GetNvidiaRates (screenmap=@0x7fffffffc3c0)
    at util-nvctrl.cpp:242
#2  0x00007ffff538c110 in DisplayResX::GetVideoModes (this=0x94a750)
    at DisplayResX.cpp:129
#3  0x00007ffff533fd5c in DisplayRes::Initialize (this=0x94a750)
    at DisplayRes.cpp:99
#4  0x00007ffff538cf1f in DisplayResX (this=0x94a750) at DisplayResX.cpp:22
#5  0x00007ffff534036f in DisplayRes::GetDisplayRes (lock=false)
    at DisplayRes.cpp:25
#6  0x00007ffff53403fb in GetVideoModes () at DisplayRes.cpp:219
#7  0x00000000004b88d2 in AppearanceSettings (this=0x7fffffffcd00)
    at globalsettings.cpp:5095
#8  0x0000000000439676 in WriteDefaults () at main.cpp:647
#9  0x000000000043e4e3 in main (argc=1, argv=0x7fffffffe018) at main.cpp:1369

comment:17 Changed 10 years ago by paulh

Milestone: unknown0.22
Version: unknownhead

comment:18 Changed 9 years ago by Janne Grunau

(In [24332]) videooutX: remove stray semicolon after if disabling it

found by clang, added in [21489], Refs #5643

comment:19 Changed 9 years ago by Janne Grunau

(In [24334]) backport [24332] from trunk

videooutX: remove stray semicolon after if disabling it found by clang, added in [21489], Refs #5643

Note: See TracTickets for help on using tickets.