User pref timecode in stamp; UI_clean up; and move of timecode formatting to new BLI (BLI_timecode.c)
AbandonedPublic
Actions

Authored by Campbell Barton (campbellbarton) on Jan 18 2014, 7:13 AM.

Details

Reviewers

• William Reynish (billrey)
Andrew Buttery (axb)
Joshua Leung (aligorith)
Jonathan Williamson (carter2422)

Summary

This diff is based on the conversation T38242.

It changes the following aspects of the render stamp feature:

The render timecode burn will now respect the format selected in the user prefs (SMPTE, minimal, etc).
RenderTime becomes Render Time in UI and render burn in
Time becomes Timecode in UI and render burn in
Seq. strip in UI becomes Strip to match the render burn in
Timecode tooltip updated to tell user that the user preference timecode style will be used
Moved the function that formats the timecode based on the user preferences and current frame to Blenlib (BLI_timecode.h and BLI_timecode.c); removed from anim_draw.c and ED_anim_api.h;
Updated CmakeLists.txt in Blendlib
custom timecode format in VSE has been removed - now uses user preference timecode format

Please note that the new timecode format is included in image metadata (PNG tested) but the labels remain the same (Time; RenderTime).

Translation impacts: Yes (tooltip, UI, render text changes)

Documentation impacts: Yes
http://wiki.blender.org/index.php/Doc:2.6/Manual/Render/Post_Process/Stamp

Happy to do the wiki update...

andrew

Diff Detail

Event Timeline

note, this patch looks like its note made against master. It shows edits to BLI_timecode.h which isnt in master.

source/blender/blenkernel/intern/image.c
85	picky: use /**/ comment
source/blender/blenlib/BLI_timecode.h
35	Not sure why timecode would define this. its for drawing.
source/blender/blenlib/intern/BLI_timecode.c
33	low level BLI functions like this shouldnt depend on userprefs.
50	Not sure about taking a scene as an argument either, cant it just take a frame argument? The purpose of utility BLI functions is so any area can call them without for eg, first having to define a scene (or specific library data)

• William Reynish (billrey) accepted this revision.Jan 21 2014, 11:57 PM

Campbell Barton (campbellbarton) requested changes to this revision.Jan 22 2014, 12:36 AM

When running...
arc patch D227

I get the error...
error: source/blender/blenlib/intern/BLI_timecode.c: does not exist in index

Looks like this patch is from a diff created after BLI_timecode.c was added.

Andrew, could you diff against master, git diff master so it includes all changes you committed on a local branch.

OK will make the changes this afternoon. Apologies for the diff problems - first time adding a new file(s) to GIT repository...

andrew

Fixes as requested and discussed in IRC.

To dos:

work out if we can get rid of the pixel_offset;
delete old "simple" timecode generator code after the launch; and
clean up the function further.

I think that's it...

andrew

Quite sure pixel_offset can be removed and replaced with 0.5f (Aligorith?)

source/blender/blenlib/intern/BLI_timecode.c
89	These should be an `enum` defined in `BLI_timecode.h` `BLI_TIMECODE_STYLE_MINIMAL`... for eg. with note that they must be kept in sync with USER_... defines.

Ignore this diff missed last changes - new version of diff (v 3a) coming.

New diff:

#. Removed the VSE-only "special" timecode formatting as it was almost identical to the Seconds user preference setting so it seemed redundant and inconsistent to only have it in the VSE.
#. Removed any timecode code that was now unused...
#. Added DNA_userdef_types.h into BLI_timecode.h so BLI_timecode.c could use the Timecode enum (e.g. USER_TIMECODE_MINIMAL)

andrew

Campbell Barton (campbellbarton) added inline comments.Jan 29 2014, 7:08 AM

source/blender/blenkernel/intern/image.c
1588–1589	Easier for me to make this change while applying the patch - but image stamp shouldn't be different between systems. (annoying when using workstation in renderfarm)

pushed rBf70d9660474c2be5f56d65247df3be5af0479e08

notes

split BLI_timecode_string_from_frame into 2 functions since timecodes was basically making one function act as two.
ensure no buffer overruns with BLI_snprintf
image stamp uses hard coded timecode (this could be a stamp preference, better not use userpref here).

Campbell Barton (campbellbarton) abandoned this revision.Jan 29 2014, 10:49 AM

Note, I intentionally removed changes to user visible strings, rather user visible stuff be handled seperately, also not sure all the changes are good, Strip for example, could be NLA Strip, so think its useful to note that its from the sequencer.

Campbell Barton (campbellbarton) mentioned this in rBS39eb314cb922: UI: Refactor timecode functions into BLI_timecode.Nov 20 2014, 9:13 AM