-
-
Notifications
You must be signed in to change notification settings - Fork 280
Adds the beginning of support for audio/video #849
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Adds the beginning of support for audio/video #849
Conversation
kalebbroo
commented
Jun 27, 2025
- Allows drag and drop audio and video files in the gen tab
- Does not prevent copy and paste into Swarm for audio and video
- Adds player for audio and video
- Allows drag and drop audio and video files in the gen tab - Does not prevent copy and paste into Swarm for audio and video - Adds player for audio and video
@@ -1,4 +1,4 @@ | |||
namespace SwarmUI.Utils; | |||
namespace SwarmUI.Utils; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot get this to not be a diff. I copied and pasted from the mater and still shows as a diff. idk..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine, git is being silly about file format stuff
default: | ||
{ | ||
return $"image/{(Extension == "jpg" ? "jpeg" : Extension)}"; | ||
// TODO: Should we throw an error here or just return image? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would you like here? I also prefer switch case but I can change it back to if else if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default: // image
and remove the image case
{ | ||
return $"video/{Extension}"; | ||
case ImageType.ANIMATION: | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cases don't normally have {} braces
} | ||
case ImageType.AUDIO: | ||
{ | ||
return $"audio/{Extension}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mime types for audio are a bit silly, check https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/MIME_types/Common_types
Notably eg mp3 is audio/mpeg
, opus must condense to ogg, idk what else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(also, ftr, shoving audio into Image at all is a pretty dirty hack. It was already dirty when videos were being shoved through it, audio is making it a lot worse. Some systems need to be rethought)
@@ -561,14 +561,15 @@ function getImageFullSrc(src) { | |||
return fullSrc; | |||
} | |||
|
|||
function setCurrentImage(src, metadata = '', batchId = '', previewGrow = false, smoothAdd = false, canReparse = true, isPlaceholder = false) { | |||
function setCurrentImage(src, metadata = '', filename = null, batchId = '', previewGrow = false, smoothAdd = false, canReparse = true, isPlaceholder = false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ehwha? This change doesn't look right.
srcTarget = sourceObj; | ||
sourceObj.type = `video/${src.substring(src.lastIndexOf('.') + 1)}`; | ||
img.appendChild(sourceObj); | ||
img.controls = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't make unrelated changes
} | ||
srcTarget.src = src; | ||
img.className = 'current-image-img'; | ||
img.id = 'current_image_img'; | ||
img.dataset.src = src; | ||
img.dataset.metadata = metadata || '{}'; | ||
img.dataset.batch_id = batchId; | ||
img.onclick = () => imageFullView.showImage(img.dataset.src, img.dataset.metadata); | ||
if (!isAudio) { | ||
img.onclick = () => imageFullView.showImage(src, metadata); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
??? why were the inputs here altered? This is just re-introducing already fixed bugs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops I messed that up when a merge conflict happened I think.
label.textContent = `${name} (${img.naturalWidth}x${img.naturalHeight}, ${describeAspectRatio(img.naturalWidth, img.naturalHeight)})`; | ||
elem.dataset.width = img.naturalWidth; | ||
elem.dataset.height = img.naturalHeight; | ||
let mediaPreviewHtml; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is code specifically for image inputs? It should not be changed at all by an audio pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(or if it is changed, it needs recognition on param type. and, yknow, audio/video param types need to exist)
Closing this in favor of a more robust solution that adds smarter support for audio and video. |