feature/recording_date

This feature adds the possibility to set the original recording date when uploading video, for more lisibility for viewers.

It should fix #50

Zykino reviewed 3 years ago

prismedia/upload.py Outdated

												
					        options = utils.searchThumbnail(options)
				
					    # If after loading NFO we still has no original date and --no-originalDate is not enabled,
				
					    # then we need to search from the file

Zykino commented 3 years ago

I am not sure looking for the modification date is a sane default for the general case: A user may shoot its video one day and edit it another day. I am not confident the last edit date is "better" than the shoot date. But if the video was shot across multiples day, which file and which date is best here ?

Zykino commented 3 years ago

Ok, I read the issue, I am not sure which defalt is best between the file modification and nothing. Do you know how this is presented in both Youtube and Peertube ? I am interested by the wording they are using.

LecygneNoir commented 3 years ago

Unfortunately, getting "creation" date for a file is really difficult depending the OS, and we cannot know if a file have been copy/pasted before, thus its creation date is also inexact.

Ideally, I wanted to get the recording date from metadata, but this information is missing in most .mp4 files.

So the only "automatic" information available are modification date of the file, and last opening date. Modification seems the best alternative.

Youtube presents it exactly as Original Recording date (also allowing to set the Original recording location in UI, but not in API)

Peertube is slightly different as it's the Original publish date, and it's for example used when downloading video from Youtube using a youtube link in Peertube, it's filled with the date of Youtube publication.

But for simultaneous upload as we do in prismedia, this fields seems relevant to me.

Do you think something like that could be better?

No date by default
add an option to enable the date
If option is empty, take the modificaiton date
Else, take the date passed to the option

I am a big lover of default options to avoid the need to configure everything, but I know it's bias of myself so I am open to discussion ^^

Zykino commented 3 years ago

I know for both of us the no option meaning a value is set is good. But thinking about someone who shoot on Monday, do the edit on Wednsday and upload on Friday (for example) may prefer to set the record date.
Especially if the subject of the video is news related. Like for the recent american election. I feel like "Nothing" is a better default. And you can just set an "auto-original-date" on your NFO ;)

LecygneNoir commented 3 years ago

You have a good point here indeed!

I'll change the default, and add the --auto-originalDate option.

Also, I'll add a strict option for that (--withOriginalDate) to avoid mistake in the same schema that other strict option, as we have it for publishDate for example.

Does it makes sens for you?

Zykino commented 3 years ago

There is no content yet.

Zykino reviewed 3 years ago

README.md Outdated

												
					                    DATE should be in the future
				
					  --peertubeAt=DATE
				
					  --youtubeAt=DATE  Override publishAt for the corresponding platform. Allow to create preview on specific platform
				
					  --originalDate=DATE  Configure the video as initially recorded at DATE

Zykino commented 3 years ago

We are starting to have a lot of options here, we may think about only including important one / some important example. The list is already present if we ask for help so as long as this is explain, I do not see a lot of value to keep both.

LecygneNoir commented 3 years ago

Indeed, the Readme begins to be very long, you are true.

I'll try to select some specific option that are not easy to understand and focus on them :)

Zykino reviewed 3 years ago

prismedia/upload.py

Zykino commented 3 years ago

This is a copy past of validatePublishDate. Maybe just use one named validateDate or something like that.

LecygneNoir commented 3 years ago

I have thought about that, but it's not exactly a copy/paste as there is a difference: publish date should be in the future, while original is in the past ^^"

I have found no satifying way to do that with one function, as in this scope we only have the parameter value as a date, and no its name. So we have no way to know if this is a publish or original date, but if you have any idea, I would be happy to reduce the code!

Zykino commented 3 years ago

Ah right, I missed that :s
Is it possible to pass an argument ? To tell before/after present date ? Or maybe the validation being a call to valid date and a call to before/after present.

But if this is too hard nevermind.

LecygneNoir commented 3 years ago

I found no easy way to pass arguments, I think something is possible with handler instead of validate function, as we do for strict options, but it's more complex...

I'll keep this in memory if we change the docopt/schema system one day, some other help/doc libraries may be better for that.

LecygneNoir marked this conversation as resolved

Zykino reviewed 3 years ago

prismedia/pt_upload.py Outdated

												
					    else:
				
					        fields.append(("privacy", str(PEERTUBE_PRIVACY[privacy or "private"])))
				
					    # Set originalDate except if the user force no originalDate

Zykino commented 3 years ago

The function is already too long and this is the same as for the publish date, call a function to format a date and add it to the fileds here. (For here and the publish date)

Zykino reviewed 3 years ago

prismedia/yt_upload.py Outdated

												
					        body['status']['publishAt'] = str(publishAt)
				
					    # Set originalDate except if the user force no originalDate
				
					    if not options.get('--no-originalDate'):

Zykino commented 3 years ago

The function is already too long and this is the same as for the publish date, call a function to format a date and add it to the fileds here. (For here and the publish date)

LecygneNoir commented 3 years ago

You are sooo true, and so hard with my lazyness xD

I'll update it :p

Zykino reviewed 3 years ago

prismedia/yt_upload.py Outdated

												
					            "privacyStatus": str(options.get('--privacy') or "private"),
				
					            "license": str(license or "youtube"),
				
					        },
				
					        "recordingDetails": {

Zykino commented 3 years ago

This fells strange. But I am not sure that creating the structure would be better. Especially if/when we will add another field in this struct.

LecygneNoir commented 3 years ago

I am not a big fan to add the "recordingDetails" empty, but it does not broke the upload if it's empty, and I found no easy way to add this later because of the dictionnary of dictionnary structure (probably missed something here though, it should be possible somewhat)

As you said, it seems easier to create the structure directly, so it's available for latter use.

Is it ok for you?

Zykino commented 3 years ago

Yes we can keep it. Maybe adding a comment in it saying that (not break upload and easier if we end up with multiples optionals fields).

LecygneNoir closed this pull request 3 years ago

This pull request cannot be reopened because the branch was deleted.

No reviewers

No Label

No Milestone

No Assignees

2 Participants

Notifications

Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

			return True


			def validateOriginalDate(originalDate):

#51 feature/recording_date