MiSTer-devel / Main_MiSTer

Main MiSTer binary and Wiki
GNU General Public License v3.0
3.06k stars 331 forks source link

Sending video meta data through direct video #808

Open thehughhefner opened 1 year ago

thehughhefner commented 1 year ago

Hi developers, I'm reaching out to you requesting the possibility of adding metadata information through direct video so that external hardware such as scalers can use that information. A use case is using a retrotink 4k where it takes metadata with the direct video on how to exactly crop and scale the game. Thanks

mikechi2 commented 10 months ago

I didn't get anywhere with the NEW_VS_PARAM on the ADV7611. Unfortunately it still glitches out like crazy in both settings. But regardless, the current solution of direct_video = 1 plus metadata in the SPD infoframe works great and is stable.

I like the SPD metadata approach because it provides a lot more info and is far more flexible, while requiring fewer core modifications.

marqs85 commented 10 months ago

but I assume a de-jitter compatibility option should be anyway added to the core if its direct video is to be used with HDMI receivers.

in new version DE is same as originally in direct_video=1 mode. So any irregularity of DE size should be irrelevant already. Scaler gets SPD info and also core name to get more hints. Thus scaler may add additional tweaks if required.

De-jitter isn't needed with the SPD metadata since the ADV7611 (and presumably ADV7610 too) only barfs on the first DE, so the glitch is on a blank line.

It's not just about DE irregularity but variable clock count per frame. In framelocked mode scaler has to dynamically adjust output clock frequency or overall frame size, neither of which is great for display compatibility.

ADV7610 started to report v_total correctly when I set NEW_VS_PARAM=0 (opposite to recommended ADI setting).

so, now you can capture perfectly framed video when you apply SPD parameters?

I suppose reading H/V totals from registers and rest from SPD works even though it's not ideal solution. Would it be possible to add some fixed handle like 'MisterDV' on the first 8 payload bytes so that receiver automatically knows H/V timings should be read with this special method? EDIT: just noticed it's there already.

sorgelig commented 10 months ago

Yeah. DV1 is there. I didn't want to make longer signature, so i've got more bytes for name of core at the end.

sorgelig commented 10 months ago

I think current version is satisfying for everyone, so I'm going to push updates to framework. By the way, i remind about osd. Scaler need to decide if it will switch temporary to video clock or not. Check if osd flag gets updated.

mjj03301977 commented 10 months ago

I have the RT4K now and the unstable MiSTer and Mega Drive cores to test.

direct_video=1

What else do I need to set in both mister.ini and on the RT4K? The Mega Drive image is split in half at the moment.

mikechi2 commented 10 months ago

I have the RT4K now and the unstable MiSTer and Mega Drive cores to test.

direct_video=1

What else do I need to set in both mister.ini and on the RT4K? The Mega Drive image is split in half at the moment.

Thanks!

Wait for me to release an update that uses the meta data to auto drive the scaler. Need a bit more time since I'm still in the middle of packing and shipping the launch orders.

mjj03301977 commented 10 months ago

@sorgelig Will all the cores need to be updated for this to work? I ask because I noticed you supplied an updated test Mega Drive core build along with the test MiSTer build.

Also, can you let us know which mister.ini video related settings will still work after setting direct_video=1? I am assuming most get disabled/no longer apply.

((Thanks for putting up with my questions. I just have this fancy new scaler and I cannot really use it yet, so I am trying to understand the various settings I will need to configure.))

marqs85 commented 10 months ago

I've now done preliminary changes on Ossc Pro firmware to support decode of custom SPD metadata. Overall it works but a few minor issues were identified:

sorgelig commented 10 months ago

Will all the cores need to be updated for this to work?

yes. But it's still a testing phase, so i didn't push update to Template yet.

  • Infoframe payload seems to be off by 1 byte as first byte is read as 'V' instead of 'D' (or then some issue with ADV7610)

infoframe is sent by hardware (ADV7513), so i can't change it. May be there is an issue on receiver side? I cannot test it myself, unfortunately.

  • DE offsets do not seem to be exactly correct with the MD core. From the description it's also unclear whether 'fall' refers to leading edge of negative sync, or trailing edge in general (I'm assuming the latter for now).

Trailing edge for both HSync and VSync.

  • Anyway, de_v in SPD is 26 while correct v_backporch should be 24 (v_synclen+v_backporch=27 if leading sync edge was the reference)

i was thinking it might be off by 1. But off by 2 is weird. I will re-check and correct if needed. There might be because of the way how it's counted such as hsync before/after vsync. please refer this code (vid_de_h, vid_de_v): https://github.com/MiSTer-devel/MegaDrive_MiSTer/blob/a9eed425c7ee0126fc96a507bbfbfdaba3916dc1/sys/hps_io.sv#L912-L971

sorgelig commented 10 months ago

In line 967 i need to change to de_v <= 0; which should be correct. So it will be 25. If scaler will use the same way to calculate, then de_v will match.

sorgelig commented 10 months ago

it would be interesting to hear feedback from @mikechi2. Do you have same issues as @marqs85?

mikechi2 commented 10 months ago

I have noticed the exact same 1-byte offset on the SPD infoframe as @marqs85 .

I can double check on the HDMI analyzer too.

Haven't looked into the crop offsets too much. Not too worried about that since it sounds like it's just an offset factor.

sorgelig commented 10 months ago

How about infoframe header? Do you receive 0x83, 0x01, 0x19? In ADV7513 you just need to fill the array including the header, so data comes right after header in array. And in my code it also comes as an array including header. So i have no idea how first byte of date can disappear - it's not the first byte in array.

marqs85 commented 10 months ago

Perhaps packet byte 0 needs to be left empty for checksum (presumably calculated automatically by ADV7513).

mikechi2 commented 10 months ago

Here’s a dump of the SDP packet direct from the hdmi analyzer.

It does look like the first data byte is “V” IMG_5309 IMG_5310 IMG_5311

sorgelig commented 10 months ago

Ok. Yes, 4th byte is CS. Fixed Main: MiSTer.zip

mikechi2 commented 10 months ago

Ok. Yes, 4th byte is CS. Fixed Main: MiSTer.zip

Byte 0: 0xAC (checksum) Byte 1: 0x44 ('D') Byte 2: 0x56 ('V')

So looks good!

sorgelig commented 10 months ago

i've adjusted de_v. Now it's one less which should be correct: MegaDrive.zip

mikechi2 commented 10 months ago

@sorgelig

(uint8_t)(vi->pixrep ? vi->pixrep : (vi->ctime / vi->width)), 

What does 'vi->ctime / vi->width' represent? Does it matter since I assume pixel rep will be the one that is actually transmitted?

sorgelig commented 10 months ago

What does 'vi->ctime / vi->width' represent? Does it matter since I assume pixel rep will be the one that is actually transmitted?

Basically it's pixel repetition too (clock_cycles/pixel_cycles) but less guaranteed (it never been an important parameters). It's for cores with not updated framework. For those cores you won't get de_h neither de_v, but still get pixel repetition, width, height.

mikechi2 commented 10 months ago

So far this seems to work quite well. I can correctly detect the pixel repeat, menu open and set the vertical crops.

The only snag I've found is the horizontal crop:

6 (uint8_t)vi->de_h, 
7 (uint8_t)(vi->de_h >> 8), 

The SPD info frame reports 45 pixels, but it appears to be more like 80 pixels. Basically extra black on the left, image cut off on the right.

Could you sanity check me?

sorgelig commented 10 months ago

Did you try both 256 and 320 pixel modes in MegaDrive and both have wrong de_h?

I just think, may be it would be better to report de_h rather in video clock cycles than pixels. Some core, especially MegaDrive/Genesis may have other pixel rate during hblank/hsync.

marqs85 commented 10 months ago

I didn't have issues with de_h, but it might be still better to report de_h in vclk cycles since backproch is probably not always integer multiple of pixrep.

sorgelig commented 10 months ago

The SPD info frame reports 45 pixels, but it appears to be more like 80 pixels. Basically extra black on the left, image cut off on the right.

make sure you count from trailing edge of hsync, not front one.

mikechi2 commented 10 months ago

The SPD info frame reports 45 pixels, but it appears to be more like 80 pixels. Basically extra black on the left, image cut off on the right.

make sure you count from trailing edge of hsync, not front one.

My bad, yup this works great in both 320 and 256 modes on the MD.

Also tried a few cores without the update and can confirm the pixel repeat and active image size is transmitted correctly in the SPD (but no cropping info, obviously).

Will play a bit more, but this is working great right now! Thanks!

mikechi2 commented 10 months ago

https://github.com/MiSTer-devel/Main_MiSTer/assets/23427169/907d7a8c-d84c-4609-ad4a-be6682c68c60

See 'DV1' Line on the screen. Can confirm resolutiuon and crops are transmitted. Opening OSD automatically undos the decimation so OSD can be rendered correctly.

sorgelig commented 10 months ago

New build: MegaDrive.zip de_h now is in vclk cycles as discussed above. Hope this is last change.

mikechi2 commented 10 months ago

@sorgelig the rotation flag -- does it mean the video has already been rotated or that the scaler should apply rotation? If the later, we would need to specify clockwise vs counterclockwise.

sorgelig commented 10 months ago

Rotation flag is mistake. It won't be set in direct video. Just ignore this flag (i will remove it). MiSTer doesn't support rotation over DirectVideo as it's simply impossible. If scaler can rotate, then you can implement it at your own wish. You may use core name sent over SPD to apply rotation per core.

mikechi2 commented 10 months ago

New build: MegaDrive.zip de_h now is in vclk cycles as discussed above. Hope this is last change.

Can confirm this works

EDIT: I also checked 480i mode and it works as well. Have not checked this before.

sorgelig commented 10 months ago

That's great!

@mikechi2 @marqs85 is anything else need to be added? Probably i need to compile couple more cores to try

mikechi2 commented 10 months ago

That's great!

@mikechi2 @marqs85 is anything else need to be added? Probably i need to compile couple more cores to try

Thank you again!! For me this is perfect :) But lets see if @marqs85 has any thoughts.

sorgelig commented 10 months ago

couple more cores to test: SNES.zip TurboGrafx16.zip

mikechi2 commented 10 months ago

couple more cores to test: SNES.zip TurboGrafx16.zip

Thanks!!!

=SNES= -Vertical crops are correct in both interlaced and progressive 224, 239 modes -Horizontal decimation switches correctly when you force 256 mode on the core -Basically, no problems found

=TG16= -Horizontal decimation switches correctly between different modes -Only thing I found is that the top and bottom lines are cut off in all modes. But I'm not sure if this is just inherent to the TG16 (see pic)

2024010310184646

mjj03301977 commented 10 months ago

Will "force 256 mode" in the SNES core show 512 mode when a game makes use of it? (e.g., menus in Secret of Mana)

Just wondering if using 512 mode always by default still makes sense when using an external scaler.

sorgelig commented 10 months ago

Will "force 256 mode" in the SNES core show 512 mode when a game makes use of it? (e.g., menus in Secret of Mana)

it will show it in 256px mode, i.e. lower res.

mikechi2 commented 10 months ago

I don't see a problem with running the SNES core in 512 mode all the time, except for Super gameboy games where you want a 1:1 PAR mode with LCD effects. I would just setup a seperate profile for that use case.

Guspaz commented 10 months ago

The problem with the SNES is that the switch between 256px and 512px can happen arbitrarily mid-frame (or even multiple times per frame), and the MiSTer can't know in advance of sending the frame if there will be 256px or 512px content. So the only safe move is to always send 512, and leave it up to the user to force 256 if they're sure a game won't use 512.

dshadoff commented 10 months ago

=TG16= -Horizontal decimation switches correctly between different modes -Only thing I found is that the top and bottom lines are cut off in all modes. But I'm not sure if this is just inherent to the TG16 (see pic)

I can check the core and real hardware. Let me first understand what you are looking at on the screen - is this a pattern from Artemio's 240P suite ? If so, which version (and which pattern) ?

Note that there are games on this system which use 256 pix, 352pix, and 512 pix per line (21.477MHz master clock divided by 4, 3, and 2 respectively). Multiple horizontal resolutions may appear on different lines on the same field.

sorgelig commented 10 months ago

Many consoles with multiple resolutions may switch it in middle of frame. MegaDrive and TGFX also can switch between resolutions during a single frame.

mjj03301977 commented 10 months ago

I would think there should be three options:

force 256 mode, force 512 mode and original.

This way, external scaler users can use "original" mode and let the scaler handle the resolution changes.

dshadoff commented 10 months ago

Not sure you understood. There are games which use 256pix on top half of screen for graphics, then mid-frame switch resolutions to higher res to fit more text. Uniform resolution will not work properly.

mjj03301977 commented 10 months ago

Fair enough. I thought the force 512 mode was put in place because of scanline filters, etc. I am first to admit that I do not know the under-the-hood reasons, so I trust you all. :)

dshadoff commented 10 months ago

This was why I added the information about the master clock and the dividers. It would be fine to assume a 21MHz pixel clock, sample them at those intervals (as sub-pixels), and then work on the aggregate pixels.

Guspaz commented 10 months ago

Fair enough. I thought the force 512 mode was put in place because of scanline filters, etc. I am first to admit that I do not know the under-the-hood reasons, so I trust you all. :)

There is no force 512 mode.

mjj03301977 commented 10 months ago

Fair enough. I thought the force 512 mode was put in place because of scanline filters, etc. I am first to admit that I do not know the under-the-hood reasons, so I trust you all. :)

There is no force 512 mode.

I know...it is forced by default. ((I am talking about the SNES core))

memmam commented 10 months ago

@mjj03301977 As mentioned, the SNES has a 512-column output mode. One of the famous examples is Jurassic Park, which uses it for a fake transparency effect: OXZUQKJ

The issue is that some SNES games will change the number of columns mid-frame. The game Treasure of the Rudras, for example, switches mid-frame from 256 columns to 512 columns specifically for text boxes for higher resolution text, before switching back (The fact it is actually switching output modes was observed during RetroTINK-4K testing): D4RuEVS

As a result, if you simply made the assumption based on the start of the frame that the entire frame is 256 columns, you could get some...mangled output: 35150-Rudra_no_Hihou_(Japan)_ En_by_Aeon_Genesis_v2 0 _(~Treasure_of_the_Rudras)-1470065311

As a result, it's safer to just push the extra pixels in case a game makes use of them.

birdybro commented 10 months ago

As a result, if you simply made the assumption based on the start of the frame that the entire frame is 256 columns, you could get some...mangled output:

Yes, this is roughly mirrored behavior on the MiSTer core if you do Force 256px in this same situation for a game where it isn't appropriate. Thanks for explaining with examples.

dshadoff commented 10 months ago

=TG16= -Horizontal decimation switches correctly between different modes -Only thing I found is that the top and bottom lines are cut off in all modes. But I'm not sure if this is just inherent to the TG16 (see pic)

2024010310184646

I think I know what's going on here.

I checked this with the CD version 1.14 of Artemio's 240P suite, from November 15, 2023, with "Grids" (under Test Patterns) in various display modes.

The default overscan reduction can be a bit aggressive, because this video processor can display more lines than most consoles, so most people prefer to see a "clipped" view.

Please go into the OSD, in the "Audio & Video" submenu, and switch "Overscan" from "Hidden" to "Visible", and the full display will be shown.

mikechi2 commented 10 months ago

=TG16= -Horizontal decimation switches correctly between different modes -Only thing I found is that the top and bottom lines are cut off in all modes. But I'm not sure if this is just inherent to the TG16 (see pic) 2024010310184646

I think I know what's going on here.

I checked this with the CD version 1.14 of Artemio's 240P suite, from November 15, 2023, with "Grids" (under Test Patterns) in various display modes.

The default overscan reduction can be a bit aggressive, because this video processor can display more lines than most consoles, so most people prefer to see a "clipped" view.

Please go into the OSD, in the "Audio & Video" submenu, and switch "Overscan" from "Hidden" to "Visible", and the full display will be shown.

Thanks! Yup with the setting chnaged to "Visible" there's nothing cut off from the test grid. There are 242 active lines.