cirquit / trdrop

trdrop - a raw video analysis program
MIT License
381 stars 33 forks source link

TRDrop v. 1.10 Issues #66

Closed karaokefreak closed 3 years ago

karaokefreak commented 3 years ago

Hi Thanks again for your work with TRDrop. Glad to have it! I have worked a little bit with version 1.10 and this is what I found out

NEW FEATURES

BASIC FEATURES:

PROGRAM BASICS

cirquit commented 3 years ago

Thanks for the review. Everybody else is also free to comment here as we're not a big project and this thread does not derail too much.

  1. Graph centering buffer on new video load - I'll check how to mitigate this behavior, but it may be not easy as the live changing of the options are also dependent on that buffer.
  2. Framerate Analysis Range Text from seconds to frames - I'll change that in the next release.
  3. Add an enable button for the analysis range text - Sure, I'll add that too.
  4. 60 vs 61 fps videos - Feel free to upload a video somewhere and write me mail so I can debug it myself. The difference frames should be helpful at the very least.
  5. TRDrop Icon - Sure, I'll look into it, maybe @MujoA can do that?
  6. Windowed mode - Currently the application is already windowed (compared to previously fullscreen). So this issue only arises when you open options? When I open it there is still a windows-like border so it looks windowed to me as well. Please elaborate what you mean with windowed / send a screenshot please.

Edit: Added numbers for the issues.

karaokefreak commented 3 years ago

Hi there (Tach auch, gerade gesehen, dass du aus Deutschland kommst... ich bleib aber bei Englisch)

As I explained before, the HDR issue appears every time I open a menu in TR drop. So when I open the file menu, i get a flickering screen, when I go to the option menu in order to configure my layout, I get a flickering screen. It's a bit annoying, since it also falsifies my Windows colour settings after a few encounters. My whole colour scheme will turn overbright eventually. Anyway, the flicking also comes up when I start up TRDrop.

About the 61 FPS issue - i'll make some extra checks first, so I can rule out a miscalculation of the average FPS because of adaptive Vsync, which suggests 60 FPS, but swallows one or two screens each second very early on in the drawing process. Just by the math this would equal in theoretical 61 FPS due to one or two frames being "finished" much earlier than in 16,6 ms

cirquit commented 3 years ago

Guten Rutsch wünsche ich dann schonmal :)

So I never heard or encountered this issue, not sure if I can even reproduce it. I can do some random searches of Qt-based applications having these kind of issues, but I can't promise anything.

Otherwise feel free to send me some videos you're having issues with if you feel this is a trdrop issue.

ImplyingKinpatsu commented 3 years ago

Something else to consider: if the framerates or frametimes exceed the maximum of the graph, they'll be drawn off of it. This is manageable if you know the range of what you're testing, but the maximum allowed ms for frametimes is 200

cirquit commented 3 years ago

@ImplyingKinpatsu So this issue now only arises because I limited the maximum value of the graph. What would be the best solution?

  1. Enable / Disable maximum of the graph
  2. Increase the max range of the graph to "unlimited"
  3. Both 1 + 2

What would your maximum values for both frametime and framerate be?

karaokefreak commented 3 years ago

In my opinion it is best to keep it compact, so if the frame time graph exceeds the chosen limit, the rest of the graph should disappear. Actually, the frame time has only so much value once the framerate goes below 30FPS (or 25 for PAL). The magic numbers are 33 or 16,6ms. Of course it's interesting to get the frametimes of odd and uneven framerates, too, so you can analyze where and when frames took more time to render. But everything above 66 MS just indicates a major hiccup in the pipeline in which the exact numbers barely ever matter.

just my 2 cents.

ImplyingKinpatsu commented 3 years ago

I think keeping it at a max of 200 (or even slightly bumping it up to 250) and cutting off anything higher like mentioned would be fine and if you'd like to know exact numbers of anything higher, the frametimes could be exported in the *.csv along with the framerates, but to pick between the three options, I'd say number 2

cirquit commented 3 years ago

Disabling the frametime graph on certain issues automatically is not a good UX I feel. In doubt I'd always just presume that the user can re-export the video with the correct settings.

I got another idea - what about setting the "maximum" the same way as we have right now, but I'll just make it adaptive afterwards. Think a fixed 100ms frametime but if you get something higher the plot scales like before. If no values above the threshold are there, you return to the fixed 100ms window.

cirquit commented 3 years ago

Thanks for the review. Everybody else is also free to comment here as we're not a big project and this thread does not derail too much.

  1. Graph centering buffer on new video load - I'll check how to mitigate this behavior, but it may be not easy as the live changing of the options are also dependent on that buffer.
  2. Framerate Analysis Range Text from seconds to frames - I'll change that in the next release.
  3. Add an enable button for the analysis range text - Sure, I'll add that too.
  4. 60 vs 61 fps videos - Feel free to upload a video somewhere and write me mail so I can debug it myself. The difference frames should be helpful at the very least.
  5. TRDrop Icon - Sure, I'll look into it, maybe @MujoA can do that?
  6. Windowed mode - Currently the application is already windowed (compared to previously fullscreen). So this issue only arises when you open options? When I open it there is still a windows-like border so it looks windowed to me as well. Please elaborate what you mean with windowed / send a screenshot please.

Edit: Added numbers for the issues.

I fixed the issue 1 + 2 + 3 already. I'll add the idea from @ImplyingKinpatsu to the feature list so you can export the frametimes as well in the .csv file. Otherwise I'll check if I can update the logo and fix all the tooltips and wait for your answer concerning the last issue we talked about.

Then we'll be ready for the next release and my holidays are almost over as well :D

cirquit commented 3 years ago

What do you think about unique filenames? Something of the sorts of a switch button that adds a timestamp prefix every time you start exporting?

My idea was that you always create a new export directory so there are no filename collisions, but is this also how you use the program?

I noticed that the whole csv export is really scuffed right now so I'll definitely add an option to make a custom .csv filename and a correct header.

cirquit commented 3 years ago

CSV Features - added a header based on the video name:

FPS <Video1>, Framerate <Video1> etc. Also exporting the values only of the videos that are actually loaded, so no more 0th values.

Also added new random colors on startup cause I saw way too many people using the default whitish grey on the white plot. Only missing tooltips now + the new icon, then I think I'll be posting the final release someday tomorrow.

I also decided to make an FAQ so stay tuned.

cirquit commented 3 years ago

I noticed that the plots look very very basic - I decided to tune them up a bit by making the inner lines half transparent and making the outer ones crisp. Fixed some overlay issues as well. image

There was also a bug that didnt scale the text shadow offset based on the resolution, so now it looks way better. image

cirquit commented 3 years ago

New release is out, I'm gonna close this issue. Please create new issues for each new feature / bug as it seems way too convoluted to follow this thread already. Hopefully all of your concerns got solved besides the strange HDR settings. Please feel free to make a new issue where we can gather HDR specific problems.