Skip to content
This repository was archived by the owner on Apr 21, 2025. It is now read-only.

Conversation

@jcdr428
Copy link
Collaborator

@jcdr428 jcdr428 commented Jan 10, 2020

Hopefully last bug on fps reading...

  • correct text descriptions from sps and vps
  • remove junk from metaDemuxer.cpp (calling getTrackFps which always sets fps to 0 !!)

Hopefully last bug on fps reading...
@jcdr428
Copy link
Collaborator Author

jcdr428 commented Jan 10, 2020

Should fix #89.

@justdan96
Copy link
Owner

Has this been tested with a variety of sources, so we can check for regressions?

@jcdr428
Copy link
Collaborator Author

jcdr428 commented Jan 10, 2020

@justdan96 I've tested with five files: .mkv, .ts, .hevc, hevc .m2ts, avc .m2ts
fps was fixed on these five tests.

Edit : you can test result on Unreal_Engine_4_-_Elemental_Demo___2160p60_x264_CRF16_YUV420-HEVC-x265.mp4, eg by muxing this file with an srt in an mkv container, and inputting in m2ts.

@justdan96 justdan96 added the bug Something isn't working label Jan 10, 2020
@lighterowl
Copy link
Contributor

Wanted to merge this tonight, but I can't reproduce the issue no matter how hard I try. Let's wait until we get a reply on #89.

@jcdr428
Copy link
Collaborator Author

jcdr428 commented Jan 11, 2020

@Xavery agreed. The two samples I was sent (from W10x64 users, same as me) didn't produce any issue, it is strange that the behavior is not consistent between PCs.
The part with "getTrackFps" may have been added by Roman between 2.6.12 and 2.6.15 but seems unfinished.
Muxing AV and srt into an mkv is the only way I found to produce the issue. So let's see what is the feedbak...

Edit: I've received additonal feedback: srt with hevc now seems to be fixed, the bug is still there on h264. So I'll look the code to see if the same fix is to be applied to the other stream readers (AVC, VC-1...)

@jcdr428
Copy link
Collaborator Author

jcdr428 commented Jan 11, 2020

@Xavery see my comment on issue #89: the bug comes from the decimal "comma" in the locale.
This fix #130 is still needed for hevc to work correctly.

@lighterowl
Copy link
Contributor

GUI issue reproduced and fixed. Roman was trying too hard to do the right thing and it came back to bite us on the butt - in his credit, though, locale-dependent behaviour in text parsing functions changed a bit between Qt 4 and Qt 5. I'll prepare a bigger commit which hopefully cleans this up. For now, this solves the immediate issue.

@lighterowl lighterowl merged commit 99bcae7 into justdan96:master Jan 11, 2020
@jcdr428 jcdr428 deleted the fix_fps branch January 11, 2020 11:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants