Conversation
…ket. Will utilize for discussion and agreement on MVP
|
|
||
| The `Audio` datatype is similar to `Scene` but stores audio-only media (i.e. Audiobooks, music, ASMR, etc). | ||
|
|
||
| ## Scope |
There was a problem hiding this comment.
@Gykes / @WithoutPants : You can see a limited scope here. The database design pkg/sqlite/migrations/86_audio.up.sql shows the extent of the change.
If I get a thumbs up, then I will start work. If you want to increase/reduce scope, or to wait for any other PRs, let me know here
There was a problem hiding this comment.
I think we should nail down where this is planning on going. XXX vs non XXX
I prefer the former as that's what this app is kind of aimed at. If we do XXX then I believe that having Performer and Artist in the audio metadata would be confusing and would conflict. I say we just roll with what we currently have and not add Artists.
There was a problem hiding this comment.
I am okay with all suggested changes.
Still waiting for some confirmation that the scope will be accepted before investing time into this.
There was a problem hiding this comment.
@Gykes if you feel comfortable giving the thumbs up on this, then I can go ahead.
For all of your suggestions, I placed a 👍 to indicate that I agree and will make the change once I start working.
I will be using this for SFW, but don't mind building it for NSFW. I will adopt all your suggestions (except the rating100, see PR comment to discuss)
There was a problem hiding this comment.
Based on the fact that the main dev has said it's okay in previous comments and casually in passing then I'm okay giving the thumbs up. I can test/review as you go or in chunks, whatever you are more comfortable doing.
There was a problem hiding this comment.
@Gykes this will be a large PR, if you want to provide reviews as commits come, or wait till the end; whatever you think is best to ensure this gets merged. I am also open to excluding/deferring sections to reduce the PR size
There was a problem hiding this comment.
Just give me a ping when you feel it's ready for a review/test
Gykes
left a comment
There was a problem hiding this comment.
I want to preface this by saying I'm not the main dev and, quite frankly, have limited experience with design implementation so take what I say with a grain of salt. If something sounds dumb or wrong it probably is and feel free to call me on it.
|
|
||
| The `Audio` datatype is similar to `Scene` but stores audio-only media (i.e. Audiobooks, music, ASMR, etc). | ||
|
|
||
| ## Scope |
There was a problem hiding this comment.
I think we should nail down where this is planning on going. XXX vs non XXX
I prefer the former as that's what this app is kind of aimed at. If we do XXX then I believe that having Performer and Artist in the audio metadata would be confusing and would conflict. I say we just roll with what we currently have and not add Artists.
| - duration | ||
| - audio codec | ||
| - OPTIONAL (can be added now or later) | ||
| - channels (mono, stereo, 5.1, 7.1) |
There was a problem hiding this comment.
Would tthis really be needed? Currently scenes have audio and we don't track this at all.
There was a problem hiding this comment.
I would suggest looking at bitrate again, as this is tracked for scenes (the video portion).
And it might be better to add now, instead of planning to add later.
There was a problem hiding this comment.
Mostly I was referring to the channels. I think bitrate would be fine.
| `created_at` datetime not null, | ||
| `updated_at` datetime not null, | ||
| `code` text, | ||
| `artists` text, |
There was a problem hiding this comment.
If we decide to go with performers this would need to be changed.
| `updated_at` datetime not null, | ||
| `code` text, | ||
| `artists` text, | ||
| `album` text, |
There was a problem hiding this comment.
Studio was in the Audio Metadata but album is used here. How and where would we store albums? In the case of audio, studios seem like the best fit.
| `title` varchar(255), | ||
| `details` text, | ||
| `date` date, | ||
| `rating` tinyint, |
There was a problem hiding this comment.
Probs better to use rating100 similar to scenes.
There was a problem hiding this comment.
the migrations for scenes uses rating (ref: pkg/sqlite/migrations/1_initial.up.sql). I can only find rating100 in the graphql, which I do provide in graphql/schema/types/audio.graphql (line 51)
| foreign key(`audio_id`) references `audios`(`id`) on delete cascade, | ||
| PRIMARY KEY("group_id", `audio_id`) | ||
| ); | ||
| CREATE INDEX `index_movies_audios_on_movie_id` on "groups_audios" ("group_id"); |
There was a problem hiding this comment.
renamed to index_group_audios_on_group_id
|
tbh there is no reason why audios should not just be scenes and why there would be a dedicated section for them. |
Lots of errors to fix and TODO notes
This is a copy-paste setup for the backend to support AUDIO, similar to how Scene's and Images are handled.
The files allow for easy reference for discussion to agree on acceptable MVP.
Please see the
docs/dev/AUDIO.mdfor the SCOPE and as a place for general discussion.Relates to: #1258