Fix #623 - Add Quality selection options to ConfigTool for Audio/Video output

[farazs]

Fix #623. Easier configuration for output quality of video and audio by using a combo box to select. The bitrate determined for video will be based on the system resolution.

Tasks:

• [x] Add ComboBoxes for quality selection
• [x] Have ComboBox selection set qualities for Ogg Output
• [x] Have ComboBox selection set qualities for all plugins
• [x] Save ComboBox selection in profile and whether it should be used or has been overridden by manual quality entry
• [x] Add config support for Ogg Icecast
• [x] Disable config button and quality comboboxes when RAW or WebM Output is selected
• [x] Support scaling
• [x] Support non-screen inputs (NOT POSSIBLE)
• [x] Recompute video bitrate based on quality and resolution on record if "No Scaling"

Screen:

[farazs]

I want Ogg Output settings to be initialized for the first time freeseer is run (no conf files) based on a function i've defined in configtool, without the user opening the configtool. How should I go about getting this done?

Also, the default audio/video input options are test sources, correct? should we change this?

[dideler]

Also, the default audio/video input options are test sources, correct? should we change this?

That's correct. I think @zxiiro made these the defaults, probably to make testing/development easier. Do you think we should have different defaults?

I want Ogg Output settings to be initialized for the first time freeseer is run (no conf files) based on a function i've defined in configtool, without the user opening the configtool. How should I go about getting this done?

Why is initializing the settings in FreeseerConfig not a viable solution in your case?

[farazs]

Is the output resolution determined by the output plugin or the input plugin's native resolution? I know for screen recording without scaling i can just use the screen resolution, but for other inputs, is the resolution determined by the input or the output? How can I determine the resolution that's going to be output by a particular file or stream output plugin?

[farazs]

For development I think the test sources as defaults is fine, but is this different for the non-dev version?

Also, for the initialization, I want to set the audio/video quality variables inside each of the output plugins, which are in the Config classes for each plugin, not FreeseerConfig. I could initialize them there, but the function that determines the video bitrate is defined in configtool. Should I be importing it into the plugin init.py files or moving the functio definition into IOutput in plugin.py?

[zxiiro]

Output resolution is determined by the Mixer plugins in the case of if scaling. If no scaling is applied then it's just whatever the input resolution is.

[farazs]

How do I determine the input resolution for inputs that are not the desktop?

[mtomwing]

Check the video input and mixer interfaces for a method that would give the output resolution.

[dideler]

Should I be importing it into the plugin init.py files or moving the functio definition into IOutput in plugin.py?

Which function(s) are you referring to exactly, compute_bitrate()?

I think it would be useful to have the method definition in the IOutput class if all output plugins could use it. Then you can override it in the child output classes if necessary.

[farazs]

@dideler Yeah I need update_video_quality() in a bunch of places actually. In the video passthrough plugin, if the scaling is changed, but this is easy because plugins have a reference to configtool through self.gui. Though I also need update_video_quality() in multimedia.py when it loads the backend. If "No Scaling" is selected, it should recompute the video bitrate based on the resolution, incase the resolution changed (or was never computed).

I need compute_bitrate() wherever I end up initializing the bitrate setting of an output plugin if no config file existed for it. It seems that the config is not loaded until either you try configuring the plugin, or try recording with it. For eg. in multimedia.py when loading the backend, there's a call to plugin.plugin_object.load_config(), but I don't see a way of determining whether a conf file already existed or has been created.

[farazs]

@mtomwing I can't seem to find any method in the video input and passthrough classes that would return the input resolution. I can determine which scaling is being used for video passthrough, but if no scaling is being used, then I can't figure out how to get the input resolution for non-desktop sources.

[farazs]

@zxiiro I spent a bunch of time with Dennis and Michael after the weekly meeting to try and figure out how to get the resolution from the input plugins if scaling is not being used, and we couldn't figure it out. We tried videosrc.do_get_caps(videosrc) but that seems to print out what possible values various fields of the plugin can hold. There seems to be something called element pads that hold 'width' and 'height' variables in 'details' but we couldn't figure out how to use it/them. Would you know any more about this?

If we can't figure out a way to get the resolution from the USB and Firewire input plugins, I was thinking of just having the Video Quality combobox disabled if any of those inputs are selected. That way the user would know that our quality settings don't work properly for that.

[zxiiro]

This might help: http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-gstvideo.html#GST-VIDEO-INFO-WIDTH:CAPS

Otherwise I don't know really know, you might want to ask the question on GStreamer's mailing list.

[farazs]

I've been reading up more on this and it seems like there should be a way to get this information from the caps or pads once 'negotiation' is done between the input and the sink/mixer/output. Not sure exactly where the negotiation will happen.

It seems for PIP we're hardcoding the resolution to 640x480. I can get the Video Passthrough resolution from the caps structure if we set it through scaling but not otherwise. I'll look into this more later.

Zxiiro, I couldn't find the function you mentioned in gst.video.

[zxiiro]

Sorry I'm not sure what you mean by you couldn't find?

My link brings you directly to the function GST_VIDEO_INFO_WIDTH()

[farazs]

I mean in python. I couldn't find where it's defined.

[zxiiro]

Ah, yeah you'll have to look up how to do it in Python. I'm not familiar with how that all works with python bindings. (sometimes this can be a pain to find as the pygst docs are a bit lacking...)

[dideler]

Thanks for the update. Out of interest, did you try asking on the GStreamer mailing list?

[farazs]

I was hoping to figure it out without doing that but I guess I still haven't figured it out so should probably try that.

Are all the gst.element_factory_make() calls getting us gstreamer plugins? Is the flow, videosrc plugin -> videorate plugin in video passthrough -> oggmuxer -> filesink?

[zxiiro]

yes gst.element_factory_make() is how you get gstreamer plugins to be instantiated. The best place for GStreamer help is definitely directly from the source on the gstreamer mailing list and/or chatrooms on IRC.

FYI the mentors on this project are not expert GStreamer developers so we may not be able to answer all questions related to the internals of that library :)

[farazs]

Yeah I was just hoping to get some more info before asking on the GStreamer mailing list so that they understand what my question is. Thanks!

[farazs]

I emailed the gstreamer mailing list a couple of days ago and this is the response I got:

"You seem to be using 0.10 and the old Python bindings. You'll want to add a buffer pad probe to the src pad of your video source and examine a passing buffer, from which you can get the caps and parse them to give you dimensions. In 1.0 caps aren't attached to buffers but are transmitted with events, so you'd do an event pad probe instead. I'd give you code snippets but I'm using 1.0 only for some time now."

[farazs]

This seems to suggest that the resolution cannot be determined until after recording has started, and then by analyzing a buffer that comes in. So I will modify my solution so that it only works with Desktop Source, and gets disabled otherwise.

[farazs]

One problem I have now is that I have a few methods duplicated in configtool and multimedia. I put this in so that you could see which methods are being duplicated and how they are used. What would be the correct place for defining and importing these? I don't think I have a reference to a configtool instance in multimedia so I don't know if keeping them as configtool methods would work. Maybe I could put them in the configtool module and not the class and use them that way?

Also, when configtool is launched (or Freeseer is) is there a way to determine if there were saved configurations that were loaded, or whether they were initialized from defaults?

[zxiiro]

Have you looked at the multimedia initializing function at all?

https://github.com/Freeseer/freeseer/blob/master/src/freeseer/framework/multimedia.py#L49

[farazs]

That's not a reference to configtool though. It's a reference to an instance of FreeseerConfig, which contains the various config attributes like videodir, default language, videomixer etc.

[zxiiro]

The configtool should only be allowing the user to set settings. It shouldn't even be calculating anything. All actual calculations should be done in multimedia.py backend.

So if you're allowing the user to set HIGH, MEDIUM, LOW. Then those are the options configtool set let the use pick. Then any arithmetic that you need to do to convert those should be done in multimedia.py.

[farazs]

oh okay i'll do that. thanks.

[farazs]

Configtool doesn't currently have a reference to an instance of Multimedia. Should I make it so that RecordApp passes configtool it's instance of Multimedia when it constructs configtool? Is that the correct way to go about it?

[farazs]

Okay I made changes according to zxiiro's suggestions on IRC.

There is now a "Custom" option for qualities. The video-bitrate/audio-quality fields in the setup dialogs for file and stream outputs are only visible if "Custom" is selected.

The computation for audio/video quality are now done when "Prepare to Record" is pressed and so there is no need for computation in configtool, it is all done in multimedia.py.

SInce only Desktop video quality is supported in Video Passthrough, when anything other than Desktop is selected, the video quality combobox is disabled and the quality is set to "Custom". When the user switches back to desktop, the combobox is enabled and the quality is set to "High".

I think I'm all done with what I wanted to implement. If you want me to make any changes to the overall design or some code changes, let me know :).

[dideler]

Thanks Faraz. Here are screenshots to save time for other reviewers.

Old recording config UI

New recording config UI - note the quality options

video and audio input enabled - both custom quality

video and audio input enabled - custom video quality, low audio quality

video and audio input enabled - high video quality, custom audio quality

video input only - custom quality

video input only - low quality

---

To adjust the quality when it's set to custom, you have to click on the "set up" icon of the "File Format" option under "Record to File". This was the most confusing part for me, since I couldn't remember where the custom quality options originally were.

I think it would be more intuitive and discoverable if a gear icon popped up next to the quality drop down if custom was selected, and clicking that allows you to change the bitrate.

[dideler]

I think it would be more intuitive and discoverable if a gear icon popped up next to the quality drop down if custom was selected, and clicking that allows you to change the bitrate.

Another benefit is that this confusion would no longer exist:

audio and video input disabled, recording to file enabled - custom quality for audio and input

Changing the quality doesn't make sense if the inputs are disabled. (In a more general sense, recording to file doesn't make sense when the inputs are disabled -- we should probably disallow that.)

[farazs]

@dideler But what if "Record to file" and "Record to stream" are both enabled? Those have different configuration dialogs (and different units for Audio Quality). Should only the file output dialog open?

[farazs]

Could have two gear buttons? One for file and one for stream?

[dideler]

Good point. I think a single gear button per quality setting, but the window it opens could have a section for file and for stream.

[farazs]

Okay that works.

So should the audio/video options no longer be visible in the file/stream output config dialogs even when "Custom" is selected? Or should they be visible through those dialogs as well as the new audio/video dialogs?

Also, when the user clicks the gear for video, if both audio and video are on "Custom" should both audio and video fields be present in the popup dialog or only video?

[dideler]

So should the audio/video options no longer be visible in the file/stream output config dialogs even when "Custom" is selected?

Right, because then we'll have it in multiple places which I think is confusing.

Or should they be visible through those dialogs as well as the new audio/video dialogs?

I'm not sure what you mean with "new audio/video dialogs", can you elaborate?

Also, when the user clicks the gear for video, if both audio and video are on "Custom" should both audio and video fields be present in the popup dialog or only video?

How I see it, the quality setting for video input will only control the video quality, even if it's set to custom. Similarly for audio.

If both audio and video are enabled, and both audio and video quality are set to custom, and the user clicks on the gear icon for video quality, then I think only video quality settings should show up.

This means that it will require more clicks from the user to configure the custom quality of the video and audio (if they happen to use that set up), but the location of the options will make more sense as they're logically separated. That is, if the task is to set a custom quality setting for video, which is done through the video settings, then it's expected that only the video quality settings will show up when clicking the gear icon.

In summary: one way to get to the setting; multiple ways is confusing.

[farazs]

Okay thanks, I'll do that.

By the "new audio/video dialogs" I meant the dialogs for setting video bitrate or audio quality that will come up when the new gear button next to quality will bring up.

[farazs]

Added a new gear button next to the combobox for audio and video quality that lets you configure the settings for output

[dideler]

The quality options aren't always disabled when they should be. You might have to switch between different file formats to reproduce.

Edit: If you have at least one of "Record to File" or "Record to Stream" enabled, then the quality options are enabled. I think it'd be better to only enable them when audio/video input is enabled, otherwise it's confusing why half the options are disabled/enabled.

[dideler]

I think the quality controls can be made more user friendly by disabling the appropriate spinbox instead of having the controls disappear for file/stream when file/stream output is disabled.

[farazs]

@dideler The bugs you mentioned should be fixed now.

I've made it so that when one of output to file or stream is disabled, the quality setup is available and the other option is visible but disabled (unless the selected output plugin does not support quality eg. WebM and RAW).

[dideler]

I'm getting some errors when I have only "Record to file" enabled and I open Freeseer, open the config tool, and go to the recording tab.

This is low priority if you have more exams or projects, you can fix it afterwards.

[farazs]

It's okay I only have one exam left and it's in 8 days.

The errors should be gone now. It was an issue with the order in which the initialization of the avWidget was done.

[farazs]

@dideler Made all the changes you requested except for some docstrings for plugin methods. It seemed like the convention for methods defined in plugin.py was to have the docstring there and not repeated in each plugin class that implements them. Is this not the case? I can add them to the implemented versions as well but it would basically be the same docstring in like 3 more files for a bunch of methods.

[dideler]

Looks good. After you address my last comment, can you squash the commits please?

edit: Don't bother with those docstrings.

[farazs]

@dideler I made the class in freeseer.framework.multimedia. It's used by ConfigTool, multimedia, AVWidget and in the plugins that handle quality. Let me know if the class should be moved to somewhere else. Squashed as well.

[farazs]

Had to import the Quality class in settings.py as well so that the default option for quality settings is custom.

On a side note: the test_schema() in tests.framework.test_config is a rather frustrating test. It just tests to make sure the default settings = the default settings. Which really just means if you change any defaults you have to go change this test, which is all hardcoded values anyway. Is there a benefit to having this test?

[dideler]

On a side note: the test_schema() in tests.framework.test_config is a rather frustrating test. It just tests to make sure the default settings = the default settings. Which really just means if you change any defaults you have to go change this test, which is all hardcoded values anyway. Is there a benefit to having this test?

It can be frustrating having to update failing tests for small code changes, but I find this test adds value and should be kept. Its focus is testing that the schema() methods build a proper JSON schema. So it's less about testing default settings == default settings, and more about testing that all options are in the JSON schema, all the fields are present in the schema, all the fields in the schema have correct values, etc.

[farazs]

@dideler Moved the values to Quality. The different audio codecs have different possible quality values, so I've done some stuff to deal with that.

[dideler]

Nice work, thanks Faraz!