GNOME Bugzilla – Bug 582268
Option to choose photo and video size independently
Last modified: 2011-01-08 02:19:02 UTC
"Taking photos and videos at the same resolution doesn't really make sense. Photos are relatively light so I might want a higher resolution for them, but videos are heavy so a smaller size might be better. I think there shouldn't be a single "resolution" preference, but instead photo and video resolution should be set independently" Reported in Ubuntu: https://bugs.launchpad.net/ubuntu/+source/cheese/+bug/275925
Created attachment 134464 [details] Mockup of camera settings The video resolution combo should default to "Same as Photo" or similar.
Nice. We're rejigging the properties dialog box soon. I also think that implementing your request would lead to cleaner code in cheese-webcam, so I'm inclined to give it a go myself. Expect this for 2.29 or so.
Created attachment 172353 [details] [review] in progress patch in progress patch This is work in progress for this feature, lots of changes and finishing to do.
Created attachment 172354 [details] [review] in progress patch in progress patch with the previous patch and this one, these are the changes added: new photo resolution combo in preferences dialog, loading of resolutions for selected device into photo resolution combo. selecting the photo resolution combo item according to conf if no saved settings in conf, select same resolution as video by default. on combo resolution change, save selected resolution to conf added nedded variables and enums in CheeseGconf to save this new setting changed variable names to separate video and photo resolutions
Review of attachment 172353 [details] [review]: ::: data/cheese-prefs.ui @@ +130,3 @@ + <property name="position">2</property> + </packing> + </child> i think photo resolution is much more important, so please change it to be first in the ui, but also in the code ::: libcheese/cheese-gconf.c @@ +187,1 @@ CHEESE_GCONF_PREFIX "/burst_repeat", you will notice that there is a gsettings-vala branch too, which will be merged any minute from now and will replace all the gconf stuff. you would be very cool (and i think that you are very cool), if you could also patch that branch to make it work once we merge it ::: src/vapi/cheese-common.vapi @@ +153,3 @@ + public int gconf_prop_photo_x_resolution {get; set;} + [NoAccessorMethod] + public int gconf_prop_photo_y_resolution {get; set;} does this work out? imho you have to change the gconf_prop_y_resolution to video_resolution too?
Review of attachment 172354 [details] [review]: good work!
Thank you for your encouragement! =) It makes me very happy. I will change the order so the photo resolution is first everywhere. I also will look into the changes from gsettings-vala branch when they are merged. > does this work out? Yes, before adding those lines to the .vapi file, I couldn't use the Cheese.GConf properties for photo resolution that I added from cheese-preferences.vala, I got an error in the form "The name `foo' does not exist in the context of `bar'" when compiling it ('bar' being CheeseGConf). That is because we are accessing a .c library (libcheese) from vala and it needs to know the properties it has before run-time. This looks superficially similar to COM objects interfaces in windows, I guess it's something like that. The resolution values chosen by the user on combo change do indeed get saved and loaded when running the app next time. So yes that part is working. > imho you have to change the gconf_prop_y_resolution to video_resolution too? yup, this was in the second patch :)
Created attachment 172517 [details] [review] Combo order change: photo combo is above video combo In progress patch Changed the order in preferences dialog: photo resolution combo now comes first. Changed the code order to reflect importance and order of photo resolution and video resolution. Changed cheese_camera_get_current_video_format to cheese_camera_get_current_format to avoid confusion: video resolution refers to the resolution in which videos will be saved and of the preview in video mode. Photo resolution refers to the resolution in which photos will be taken and saved in photo and photo burst modes. Current camera format could be any of these two options. Some minor indentation and spacing fixes. Left cheese-gconf files unchanged because they will be deprecated soon.
> Photo resolution refers to the resolution in which photos will be taken and saved in photo and photo burst modes. The preview will also show the photo resolution when in "photo" and "photo burst" modes, since "photo" is the mode by default, we show the "photo resolution" chosen in preferences dialog in the preview by default.
would you mind combining all three patches to one? that would make the review much easier ;) thanks a lot!
Created attachment 172547 [details] [review] squashed commits These are all the changes from the previous three patches I attached to this bug in one single patch
your patch does not apply, maybe because of the commits i did just before. Applying: My squashed commits /home/dgsiegel/projects/gnome/cheese/.git/rebase-apply/patch:142: trailing whitespace. break; /home/dgsiegel/projects/gnome/cheese/.git/rebase-apply/patch:181: trailing whitespace. break; /home/dgsiegel/projects/gnome/cheese/.git/rebase-apply/patch:228: trailing whitespace. /home/dgsiegel/projects/gnome/cheese/.git/rebase-apply/patch:360: trailing whitespace. } /home/dgsiegel/projects/gnome/cheese/.git/rebase-apply/patch:362: trailing whitespace. error: patch failed: libcheese/cheese-widget.c:316 error: libcheese/cheese-widget.c: patch does not apply Patch failed at 0001 My squashed commits
hmm what a pity, I checked for trailing whitespace in cheese-widget.c in the parts I modified but can't find any: http://img153.imageshack.us/img153/3553/whitespace.png what can we do?
But I found some errors in other files, ouch! http://img219.imageshack.us/img219/3616/screenshot0001mysquashe.png will try to fix this asap
Created attachment 172556 [details] [review] all commits without trailing whitespace Removed trailing whitespace and tabs, lets try now :p
i'm sorry, it is not about the whitespaces, but about the conflict. please do a "git pull" and then fix the conflict ;)
Created attachment 172560 [details] [review] sqashed commits after merging with remote Ok let's see if this one works
Created attachment 172561 [details] gitg screencapture for reference, a screencapture of gitg
(In reply to comment #18) > Created an attachment (id=172561) [details] > gitg screencapture > > for reference, a screencapture of gitg note: the tooltips commit is not included in this patch
Created attachment 172651 [details] [review] photo and video resolution separately - work in progress I solved the conflict and tested the patch in a fresh cheese repository checkout. Sorry about the mess (the previous patches that didn't work).
Created attachment 172787 [details] [review] resolution changes when changing mode Now the resolution changes when changing modes: in photo and photo burst modes, the preview displays the photo resolution chosen in preferences, and in video mode, it displays the video resolution chosen in preferences. But! I encountered a new problem: if user selects video mode, opens preferences dialog and changes the video resolution, resolution does not change. If user changes photo resolution (while in video mode) the resolution does change, allowing to record a video in a different resolution than that configured. We need a way to know the current mode inside preferences dialog. Note that we can change the mode with the preferences dialog already open because it is not modal. So this still needs work, but I'll keep uploading "in progress" patches if it's ok :p
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.