Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate local timezone in jam recorder folder #497

Open
victorypoint opened this issue Aug 6, 2020 · 28 comments
Open

Incorporate local timezone in jam recorder folder #497

victorypoint opened this issue Aug 6, 2020 · 28 comments
Labels
feature request Feature request

Comments

@victorypoint
Copy link

victorypoint commented Aug 6, 2020

In an effort to have Jamulus log entries match up with associated jam recorder folder names (without having to manually calculate UTC offsets), it is proposed that the jam recorder folder names incorporate local timezone rather than UTC timezone. The current recording functionality uses UTC date/time only - currentDateTimeUtc(). This would involve revising jamrecorder.cpp to use local timezone date/time - currentDateTime().

To illustrate the problem, current jam recorder folder names are based on UTC time, whereas the associated log entry that initiates the recording is logged using local time. For example:

"Folder Jam-20200423-185758274" - corresponds to log entry - "23.4.2020, 11:57:58, 70.67.70.31, connected"

An alternative solution, as proposed by corrados, is to give users the choice of using either local or UTC timezone in the recorder folder name by using server command-line parameters as per the following comments:

#497 (comment)

"Could we somehow encode that information in the given string? E.g.
./Jamulus -s -R [ltz]/home/test/jamulusrecordings
So that we parse the directory name for a given "[ltz]" at the beginning. Not the nicest solution but would work.

#497 (comment)

We already have command line arguments with combined informations. For consistency to these, we should use this format instead:
./Jamulus -s -R "localtimezone;/home/test/jamulusrecordings"
I.e. we use a ";" as a separator."

Also, having both the jam recorder folder and corresponding log file entry presented in standard ISO format (yyyy-MM-dd HH:mm:ss) would also be helpful to match the two up.

Original post and discussion with detailed example - sourceforge post, have the recorder folder name reflect the current date/time of the local timezone.

@corrados corrados added the bug Something isn't working label Aug 7, 2020
@corrados corrados assigned corrados and unassigned corrados Aug 7, 2020
@pljones
Copy link
Collaborator

pljones commented Aug 7, 2020

This can be achieved by renaming the folder and files.

I don't see a need to change how it's currently done. Clients can be from anywhere - they're more likely to know their own offset from UTC than the timezone of the server, so finding a recording by UTC timestamp is clearly easier.

@corrados
Copy link
Contributor

corrados commented Aug 7, 2020

The reported issue was that the date of the log file and the recorder directories are not in sync. So if it is not a big deal, I think it would be good to be consistent within the Jamulus server.

@pljones
Copy link
Collaborator

pljones commented Aug 7, 2020

I'd prefer everything in UTC, really. I don't like timezones at all.

@victorypoint
Copy link
Author

Right, the issue is that the Jamulus log entries don't sync with the jam recorder folder name as I described in my sourceforge post. So, for example -

"Folder Jam-20200423-185758274" - corresponds to log entry - "23.4.2020, 11:57:58, 70.67.70.31, connected".

My only other concern is that the two don't display in standard ISO format (yyyy-MM-dd HH:mm:ss) which would be nice - but that's a separate issue.

This 'jam-folder/log-entry' sync would be helpful for jam scheduling and management. As a sys admin, local timezone support is expected, especially for logs, filenames, folders, etc for debugging and basic file management. Windows requires a timezone, Linux is easily set; in my case, 'timedatectl set-timezone America/Vancouver'; Docker uses timezones for its virtual logs, etc; in my case, 'environment: TZ: America/Vancouver'. Providing current timezone for jam recording just seems like a natural feature to support.

Peter, although I appreciate your opinion, for me, local timezones are a pain to convert from UTC, especially in the American zones. Renaming jam folders afterwards is not that efficient. How about a compromise? Perhaps a new Jamulus command-line argument such as --timezone, -tz? On to use the current timezone, off to use UTC.

@corrados corrados added improvement and removed bug Something isn't working labels Aug 7, 2020
@corrados
Copy link
Contributor

corrados commented Aug 7, 2020

Let's keep the number of command line arguments small. We have one for the recorder directory. Could we somehow encode that information in the given string? E.g.
./Jamulus -s -R [ltz]/home/test/jamulusrecordings
So that we parse the directory name for a given "[ltz]" at the beginning. Not the nicest solution but would work.

@victorypoint
Copy link
Author

Sound like it would work fine. Can you give an example of how a recording folder would be named?

@corrados
Copy link
Contributor

corrados commented Aug 8, 2020

Just a quick note: We already have command line arguments with combined informations. For consistency to these, we should use this format instead:
./Jamulus -s -R "localtimezone;/home/test/jamulusrecordings"
I.e. we use a ";" as a separator.

@corrados
Copy link
Contributor

corrados commented Jan 3, 2021

@pljones Are you intend to implement this?

@pljones
Copy link
Collaborator

pljones commented Jan 6, 2021

#497 (comment)
No.

@victorypoint
Copy link
Author

All I'm asking is to have the Jamulus log entries match up with the associated jam recorder folder name without having to manually calculate UTC offsets. Is that an unreasonable request? Also, having the two in standard ISO format (yyyy-MM-dd HH:mm:ss) would also be helpful.

Currently, for example:
"Folder Jam-20200423-185758274" - corresponds to log entry - "23.4.2020, 11:57:58, 70.67.70.31, connected"

@corrados
Copy link
Contributor

corrados commented Jan 6, 2021

Is that an unreasonable request?

No, it isn't :-). I just wanted to check if we have a volunteer for the implementation.

@vdellamea
Copy link
Contributor

While I understand that in public server you might have clients from any timezone (although I suspect for most of them UTC is as exotic as the other 36 they do not know :) ), I support this request for the private server case, where most likely all the people is in the same timezone. In my own server, I recompiled with currentDateTime() instead of currentDateTimeUtc().

@gilgongo
Copy link
Member

Hi - in an effort to make the Issues a list of actionable specs for work tickets going forward, @victorypoint can you please update your request based on the discussion above so that anyone who wants to pick up the work can do so easily? Otherwise, if it needs more discussion we can move this.

@victorypoint
Copy link
Author

Sure thing. In an effort to have Jamulus log entries match up with associated jam recorder folder names (without having to manually calculate UTC offsets), I propose that jam recorder folder names incorporate local timezone rather than UTC timezone. The current recording functionality uses UTC date/time only - currentDateTimeUtc(). I propose revising jamrecorder.cpp to use local timezone date/time - currentDateTime().

To illustrate the current problem, jam recorder folder names are based on UTC time, whereas the associated log entry that initiates the recording is logged using local time. For example:

"Folder Jam-20200423-185758274" - corresponds to log entry - "23.4.2020, 11:57:58, 70.67.70.31, connected"

If this proposal is not agreeable, corrados has proposed an alternate solution using parameters as follows (see comments above):

"Could we somehow encode that information in the given string? E.g.
./Jamulus -s -R [ltz]/home/test/jamulusrecordings
So that we parse the directory name for a given "[ltz]" at the beginning. Not the nicest solution but would work.

We already have command line arguments with combined informations. For consistency to these, we should use this format instead:
./Jamulus -s -R "localtimezone;/home/test/jamulusrecordings"
I.e. we use a ";" as a separator."

Also, having both the jam recorder folder and corresponding log file entry presented in standard ISO format (yyyy-MM-dd HH:mm:ss) would also be helpful to match the two up.

@vdellamea
Copy link
Contributor

I propose revising jamrecorder.cpp to use local timezone date/time - currentDateTime().

regarding this part, if acceptable it can be very easily implemented, I can do a pull request it but I suppose @victorypoint can do it too.

@gene96817
Copy link

Considering the recording file can be copied and shared, it would be good if the time is clearly labeled UTC. Local time could be added. However, one of the delights of Jamulus is jamming with new and old friends outside my timezone. (For instance, the last few of my chorus rehearsals had visitors from two other timezones.) If I have a folder with a collection of recordings, it would be good to know the time (and I regularly visit three other timezones). If you really want local time, at least the server name should be part of the file name. (better yet server name + city)

@victorypoint
Copy link
Author

Good point. Considering all the excellent feedback on this issue, it's probably best to not enforce local timezone use in the recording folder name, but rather give users the option of naming the recording folder by UTC or local timezone via the command line as corrados proposes:

We already have command line arguments with combined informations. For consistency to these, we should use this format instead:
./Jamulus -s -R "localtimezone;/home/test/jamulusrecordings"
I.e. we use a ";" as a separator."

I feel it's also important to implement standard ISO format (yyyy-MM-dd HH:mm:ss) in both recording folder names and log file.

Does this sound agreeable?

@gilgongo
Copy link
Member

gilgongo commented Feb 21, 2021

@victorypoint Just checking: is your original ticket now up to date such that it reflects what needs to be done? Not sure if additional/changed things are still scattered through the comments or not.

@victorypoint
Copy link
Author

I would say yes. Just trying to get consensus on how to proceed.

@gene96817
Copy link

@gilgongo I am still learning the github and our repository. Where is the ticket?

@victorypoint
Copy link
Author

Sorry, I misunderstood. I thought you were talking about this ticket/thread. I'm in the same boat as you, not knowing github and haven't posted a ticket there.

@gene96817
Copy link

and haven't posted a ticket there

I think you know more than me. where is "there"?

@gilgongo
Copy link
Member

@gene96817 Apologies, by "ticket" I meant this issue.

@gilgongo
Copy link
Member

gilgongo commented Feb 21, 2021

@gene96817 @victorypoint Yes, it's something we've just recently being trying to sort out. Anything that needs to be substantially discussed before agreeing on an actionable specification for the work needs to be in Discussions, otherwise it makes it very hard to work out what the Issue is really about if people are busy modifying it in the comments. Does that make sense? Think of the GitHub Issue as a bit like a Wikipedia page, perhaps.

All this will I hope become a bit clearer once we write it all up better in the contributing section of the site.

@gene96817
Copy link

I believe we have a consensus and it would be helpful (and good housekeeping) to summarize the consensus into a ticket.

@victorypoint
Copy link
Author

Definitely agree. My previous post I feel sums up the state of this issue - some folks like the idea of using local timezone in the recorder folder name, some folks don't, and some folks like to have the choice of enabling or not.

#497 (comment)

@gilgongo
Copy link
Member

@victorypoint Yes - although ideally if you could edit your original so that people don't need to scroll down to find what was eventually decided. I have in fact put a link in there to it but a proper edit would be best.

@victorypoint
Copy link
Author

Done! Let me know if any revisions are required.

@ann0see ann0see added this to Tracking Jul 1, 2023
@github-project-automation github-project-automation bot moved this to Triage in Tracking Jul 1, 2023
@pljones pljones added feature request Feature request needs documentation PRs requiring documentation changes or additions and removed needs documentation PRs requiring documentation changes or additions labels Aug 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
Status: Triage
Development

No branches or pull requests

6 participants