Closed Cryspia closed 9 years ago
The plan for next step:
These plans are abandoned
@Promm just so you know, you can use task lists on GitHub instead of bullet points for your tasks. We prefer task lists, and the best place to put it is in the PR description so it's the first thing someone sees.
I think predefined sizes (kinda like shown in the screenshot) is a better idea than the customizable options you plan on adding. The Freeseer settings are already complicated enough for beginners, so we'd like to keep it simple.
I have added the standard resolution in the settings.py to the list (The way to choose the resolution is just the screenshot). If there is no other requests, this PR is ready for code review.
I found that when output to files, no matter it is ogg or webm format, if the output resolution is over 720p, the video will not be playable (even after changing the bit rate of ogg). I am not sure the reason of it, but it seems that the output format plugin cannot handle with such high input bit rate. It should have nothing to do with the input plugin because if I use the resolution changing function to output them as 480p, there is nothing wrong with it.
This problem is for both Linux and Windows.
In that case, is it still necessary to keep the 720p and 1080p output choices? Or it should be considered as a bug and needs to be fixed?
@Promm I couldn't reproduce the error with unplayable videos at high resolutions. I made a video at 1920x1080 and 1280x720 and they played fine.
~/Videos
↪ midentify TEST-0.ogg
ID_VIDEO_ID=0
ID_AUDIO_ID=0
ID_AID_0_NAME=test
ID_FILENAME=TEST-0.ogg
ID_DEMUXER=lavfpref
ID_VIDEO_FORMAT=theo
ID_VIDEO_BITRATE=0
ID_VIDEO_WIDTH=1280
ID_VIDEO_HEIGHT=720
ID_VIDEO_FPS=\ -nan
ID_VIDEO_ASPECT=1.3333
ID_AUDIO_FORMAT=22127
ID_AUDIO_BITRATE=80000
ID_AUDIO_RATE=44100
ID_AUDIO_NCH=1
ID_START_TIME=-0.00
ID_LENGTH=8.96
ID_SEEKABLE=1
ID_CHAPTERS=0
ID_VIDEO_CODEC=fftheora
ID_AUDIO_BITRATE=80000
ID_AUDIO_RATE=44100
ID_AUDIO_NCH=1
ID_AUDIO_CODEC=ffvorbis
ID_EXIT=EOF
~/Videos
↪ midentify TEST-1.ogg
ID_VIDEO_ID=0
ID_AUDIO_ID=0
ID_AID_0_NAME=test
ID_FILENAME=TEST-1.ogg
ID_DEMUXER=lavfpref
ID_VIDEO_FORMAT=theo
ID_VIDEO_BITRATE=0
ID_VIDEO_WIDTH=1920
ID_VIDEO_HEIGHT=1080
ID_VIDEO_FPS=\ -nan
ID_VIDEO_ASPECT=1.3333
ID_AUDIO_FORMAT=22127
ID_AUDIO_BITRATE=80000
ID_AUDIO_RATE=44100
ID_AUDIO_NCH=1
ID_START_TIME=-0.00
ID_LENGTH=10.95
ID_SEEKABLE=1
ID_CHAPTERS=0
ID_VIDEO_CODEC=fftheora
ID_AUDIO_BITRATE=80000
ID_AUDIO_RATE=44100
ID_AUDIO_NCH=1
ID_AUDIO_CODEC=ffvorbis
ID_EXIT=EOF
@dideler It is strange. I cannot regenerate it every time. But it has a high rate to happen when I have a background video playing and capture the desktop source video. Freeseer will just die and generate unplayable video when I hit "STOP". There is no error log so I do not know when it would happen. It may because that the background video takes to much system resource.
When capture the camera on Windows without any background programs, if I resize the output to 1080p, the output video will be much shorter than it should be (I try to capture a video for 30s, but the output is only 7s and very laggy). Freeseer canniot recognize my camera on Linux so I cannot test that on Linux.
@Promm open your CPU monitoring software and see what your CPU % hits, if it hits 100% that is why you cannot record. Real-Time transcoding is CPU bound. I would expect high bitrate to fail with WebM as real time transcoding WebM is not possible on pure CPU. This problem is worse when you add scaling too, because not only are you now transcoding, you are also doing additional processing via scaling which will increase the CPU workload.
Another thing to keep in mind too is the transcoding codecs are not multi-threaded, only single threaded (last I checked).
Although I can record 1080p in real time with ogg/theora video encoding on my laptop but mine's pretty high end.
Hope this helps.
@zxiiro Actually it is not the problem of my CPU. I got an Intel i7 for my Linux system, and the CPU usage is quite low for that. I wonder whether it could be the problem of hard disk operations.
@Promm is this ready to be merged? If so, can you please squash your commits.
@dideler Can you tell me how to do that? I am not very familiar with the process.
@Promm The gist of it is
$ git checkout 614-resolution-change
$ git rebase -i master
*change 'pick' to 'squash' on every commit EXCEPT for the first one*
$ git push --force
The contributor documentation also links to this which seems to have more detailed steps.
Note to whomever ends up merging this in the future: make sure that #589 is merged before this.
@mtomwing I now read the scale map from settings.py. I also add a compare function in settings,py to sort the scale map key to add them into the ComboBox in order (Otherwise the keys are unsorted and the list seems a mess).
Because this PR changes settings.py, there might be conflicts with #609
@dideler OrderedDict used.
@mtomwing Problems fixed
@mtomwing Changed to "No Scaling"
I disagree that the resmap should be moved to the videopassthrough plugin because what about the video PIP plugin? and all other related mixer plugins?
If anything this should be moved to at least the common class that all video mixers inherit so that it can be used by any mixers. Or moved to the util.py if we can see this being useful by things outside of video mixers.
So I will change it after the decision is made.
In the interest of moving things along, I'm going to suggest that resmap
stay with the plugin. If there comes a time in the future when the other video plugins support resolution scaling, we'll deal with where it should go then.
@mtomwing Change finished
@mtomwing Change StringOption to ChoiceOption will cause the build fail.
@Promm okay.... in what way?
@mtomwing It is hard too say. The build copy extremely long code from many files. And I can hardly understand that. It says that there are 4 errors. If you want to read that, I can temporarily upload 'ChoiceOption' version to fail the build. Do you need that?
@Promm yes let us see the error.
@zxiiro @mtomwing OK. The error is generated. You could have a look.
@Promm take a look at the original resolution
option that you removed from FreeseerSettings
.
There is 4 errors and all of them are the same error. You're trying to read a variable that isn't set. Take a look at the traceback in each of the errors going backwards from the lines below and you should be able to figure it out.
OptionValueNotSetError: resolution
https://travis-ci.org/Freeseer/freeseer/builds/39890329#L1144 https://travis-ci.org/Freeseer/freeseer/builds/39890329#L1341 https://travis-ci.org/Freeseer/freeseer/builds/39890329#L1423 https://travis-ci.org/Freeseer/freeseer/builds/39890329#L2010
@mtomwing @zxiiro The "resolution" variable relies on the resmap in the settings.py. Since the resmap is moved out, the "resolution" cannot be given value. If I need to put back the "resolution" variable, I will need to move the resmap back.
@Promm take another look at this line of your code
https://github.com/Freeseer/freeseer/pull/615/files#diff-8ac569a227391bdf503d15c741f2fc5dR57
@zxiiro Sorry for the stupid bug. It is OK now.
@Promm is this good to merge on your end or do you plan on making any more changes? (FYI, you should consider using task lists instead of lists when outlining what work you did/plan to do in the PR.)
@dideler Those plans has been abandoned because we want not so technical choices. Yes this branch is ready to merge.
Thanks for the contribution Kaiyuan :+1:
I amended the commit message and merged as d93c4b04a237d6ca8f077c176269a7659486f6ef
Fixes #614