Closed SpotlightKid closed 4 years ago
You have already changed a few double quotes to single quotes, but for my taste you could change even more.
I haven't been completely consistent TBH, but #77 should improve this.
Thanks for the review!
I'm currently a bit busy, but I will try to address these issues / questions in a few days.
@mgeier Please review again.
Sorry @SpotlightKid, I kinda forgot about this PR ...
Thanks for your recent changes, what do you think about my still open comments?
There is still a lack of an official way to use jack._lib.JackPositionBBT
. Do you want to implement this or should I?
Do you have a suggestion how we should do this?
Since jack_position_bits_t
is a bit field, i.e. values are meant to be ORed together, I would just export these as module level constants:
JACK_POSITION_BBT = _lib.JacKPositionBBT
JACK_POSITION_TIMECODE = _lib.JackPositionTimecode
JACK_POSITION_BBT_FRAME_OFFSET _lib.JackBBTFrameOffset
JACK_POSITION_AUDIO_VIDEO_RATIO = _lib.JackAudioVideoRatio
JACK_POSITION_VIDEO_FRAME_OFFSET = _lib.JackVideoFrameOffset
what do you think about my still open comments?
TBH, I don't really care too much, just change these things it in the way you like.
what do you think about my still open comments?
TBH, I don't really care too much, just change these things it in the way you like.
OK, cool, will do.
But I'd first like to hear your opinion on which of those options is the best:
dict2position()
, see #78 JACK_POSITION_*
module constants, as mentioned above (https://github.com/spatialaudio/jackclient-python/pull/74#issuecomment-549089422)I have the feeling that adding only one of them would suffice for now. If needed, the other one could be added later.
I personally would prefer JACK_POSITION_*
constants. I find dict-based interfaces cumbersome and ugly to use, and there's the danger of mistyping dict keys.
I was also skeptical about #78 and created a branch with the JACK_POSITION_*
. I just seem to have forgotten to make PR, which I've done just now: #82.
@SpotlightKid I've re-based and squashed your commits from this PR into #83 and added a few commits there.
Follow up to #72. Obsoletes #19.
Signed-off-by: Christopher Arndt chris@chrisarndt.de