-
-
Notifications
You must be signed in to change notification settings - Fork 134
support audio embeds #2533
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
base: master
Are you sure you want to change the base?
support audio embeds #2533
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,9 +21,9 @@ function LinkRaw ({ href, children, src, rel }) { | |
|
||
const Media = memo(function Media ({ | ||
src, bestResSrc, srcSet, sizes, width, | ||
height, onClick, onError, style, className, video | ||
height, onClick, onError, style, className, video, audio | ||
}) { | ||
const [loaded, setLoaded] = useState(!video) | ||
const [loaded, setLoaded] = useState(!video && !audio) | ||
const ref = useRef(null) | ||
|
||
const handleLoadedMedia = () => { | ||
|
@@ -45,29 +45,40 @@ const Media = memo(function Media ({ | |
className={classNames(className, styles.mediaContainer, { [styles.loaded]: loaded })} | ||
style={style} | ||
> | ||
{video | ||
? <video | ||
{audio | ||
? <audio | ||
ref={ref} | ||
src={src} | ||
preload={bestResSrc !== src ? 'metadata' : undefined} | ||
controls | ||
poster={bestResSrc !== src ? bestResSrc : undefined} | ||
preload='metadata' | ||
width={width} | ||
height={height} | ||
onError={onError} | ||
onLoadedMetadata={handleLoadedMedia} | ||
/> | ||
Comment on lines
+48
to
58
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This way we would have a styled You have some choices:
|
||
: <img | ||
ref={ref} | ||
src={src} | ||
srcSet={srcSet} | ||
sizes={sizes} | ||
width={width} | ||
height={height} | ||
onClick={onClick} | ||
onError={onError} | ||
onLoad={handleLoadedMedia} | ||
/>} | ||
: video | ||
? <video | ||
ref={ref} | ||
src={src} | ||
preload={bestResSrc !== src ? 'metadata' : undefined} | ||
controls | ||
poster={bestResSrc !== src ? bestResSrc : undefined} | ||
width={width} | ||
height={height} | ||
onError={onError} | ||
onLoadedMetadata={handleLoadedMedia} | ||
/> | ||
: <img | ||
ref={ref} | ||
src={src} | ||
srcSet={srcSet} | ||
sizes={sizes} | ||
width={width} | ||
height={height} | ||
onClick={onClick} | ||
onError={onError} | ||
onLoad={handleLoadedMedia} | ||
/>} | ||
</div> | ||
) | ||
}) | ||
|
@@ -101,7 +112,7 @@ export default function MediaOrLink ({ linkFallback = true, ...props }) { | |
if (!media.src) return null | ||
|
||
if (!error) { | ||
if (media.image || media.video) { | ||
if (media.image || media.video || media.audio) { | ||
return ( | ||
<Media | ||
{...media} onClick={handleClick} onError={handleError} | ||
|
@@ -124,28 +135,34 @@ export const useMediaHelper = ({ src, srcSet: srcSetIntital, topLevel, tab }) => | |
const { dimensions, video, format, ...srcSetObj } = srcSetIntital || {} | ||
const [isImage, setIsImage] = useState(video === false && trusted) | ||
const [isVideo, setIsVideo] = useState(video) | ||
const [isAudio, setIsAudio] = useState(false) | ||
const showMedia = useMemo(() => tab === 'preview' || me?.privates?.showImagesAndVideos !== false, [tab, me?.privates?.showImagesAndVideos]) | ||
|
||
useEffect(() => { | ||
// don't load the video at all if user doesn't want these | ||
if (!showMedia || isVideo || isImage) return | ||
if (!showMedia || isVideo || isImage || isAudio) return | ||
|
||
// check if it's a video by trying to load it | ||
const video = document.createElement('video') | ||
video.onloadedmetadata = () => { | ||
setIsVideo(true) | ||
setIsImage(false) | ||
} | ||
video.onerror = () => { | ||
// hack | ||
// if it's not a video it will throw an error, so we can assume it's an image | ||
const img = new window.Image() | ||
img.src = src | ||
img.decode().then(() => { // decoding beforehand to prevent wrong image cropping | ||
setIsImage(true) | ||
}).catch((e) => { | ||
console.warn('Cannot decode image:', src, e) | ||
}) | ||
const audio = document.createElement('audio') | ||
audio.onloadedmetadata = () => { | ||
setIsAudio(true) | ||
setIsImage(false) | ||
setIsVideo(false) | ||
} | ||
Comment on lines
144
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
audio.onerror = () => { | ||
const img = new window.Image() | ||
img.src = src | ||
img.decode().then(() => { | ||
setIsImage(true) | ||
}).catch((e) => { | ||
console.warn('Cannot decode image:', src, e) | ||
}) | ||
} | ||
audio.src = src | ||
} | ||
video.src = src | ||
|
||
|
@@ -154,7 +171,7 @@ export const useMediaHelper = ({ src, srcSet: srcSetIntital, topLevel, tab }) => | |
video.onerror = null | ||
video.src = '' | ||
} | ||
}, [src, setIsImage, setIsVideo, showMedia, isImage]) | ||
}, [src, setIsImage, setIsVideo, setIsAudio, showMedia, isImage, isAudio]) | ||
|
||
const srcSet = useMemo(() => { | ||
if (Object.keys(srcSetObj).length === 0) return undefined | ||
|
@@ -203,7 +220,8 @@ export const useMediaHelper = ({ src, srcSet: srcSetIntital, topLevel, tab }) => | |
style, | ||
width, | ||
height, | ||
image: (!me?.privates?.imgproxyOnly || trusted) && showMedia && isImage && !isVideo, | ||
video: !me?.privates?.imgproxyOnly && showMedia && isVideo | ||
image: (!me?.privates?.imgproxyOnly || trusted) && showMedia && isImage && !isVideo && !isAudio, | ||
video: !me?.privates?.imgproxyOnly && showMedia && isVideo, | ||
audio: showMedia && isAudio | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,8 +33,15 @@ export const UPLOAD_TYPES_ALLOW = [ | |
'video/quicktime', | ||
'video/mp4', | ||
'video/mpeg', | ||
'video/webm' | ||
'video/webm', | ||
'audio/mpeg', | ||
'audio/wav', | ||
'audio/ogg', | ||
'audio/mp4', | ||
'audio/aac', | ||
'audio/flac' | ||
] | ||
export const AUDIO_EXTENSIONS = ['mp3', 'wav', 'ogg', 'flac', 'aac', 'm4a', 'opus'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for some folks, the above lines might not be obviously fine; however, line 44 is about file types, while 37-42 are for MIME types, and there isn't any one-to-one mapping between categories of types. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused and not necessary |
||
export const AVATAR_TYPES_ALLOW = UPLOAD_TYPES_ALLOW.filter(t => t.startsWith('image/')) | ||
export const INVOICE_ACTION_NOTIFICATION_TYPES = ['ITEM_CREATE', 'ZAP', 'DOWN_ZAP', 'POLL_VOTE', 'BOOST'] | ||
export const BOUNTY_MIN = 1000 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,7 +103,20 @@ export function parseEmbedUrl (href) { | |
|
||
const { hostname, pathname, searchParams } = new URL(href) | ||
|
||
// nostr prefixes: [npub1, nevent1, nprofile1, note1] | ||
const audioExtensions = /\.(mp3|wav|ogg|flac|aac|m4a|opus|webm)(\?.*)?$/i | ||
if (pathname && audioExtensions.test(pathname)) { | ||
const extension = pathname.match(audioExtensions)[1].toLowerCase() | ||
return { | ||
provider: 'audio', | ||
id: null, | ||
meta: { | ||
href, | ||
audioType: extension, | ||
title: decodeURIComponent(pathname.split('/').pop().split('.')[0]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there no better guess for the reasonable text label of uncaptioned audio? I'm thinking, some combination of comment author, context [i.e. SN item ID], and filename only if it's not some hash or blob identifier, otherwise some reasonable timestamp. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR focuses on embedding audio from a link, this would just complicate things |
||
} | ||
} | ||
} | ||
|
||
const nostr = href.match(/\/(?<id>(?<type>npub1|nevent1|nprofile1|note1|naddr1)[02-9ac-hj-np-z]+)/) | ||
if (nostr?.groups?.id) { | ||
let id = nostr.groups.id | ||
|
@@ -266,6 +279,9 @@ export function isMisleadingLink (text, href) { | |
return misleading | ||
} | ||
|
||
// Add after IMG_URL_REGEXP | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the above comment is forensically interesting [i.e., "look, Copilot is talking to the operator"], and probably does not belong in the production codebase. |
||
export const AUDIO_URL_REGEXP = /^(https?:\/\/.*\.(?:mp3|wav|ogg|flac|aac|m4a))$/i | ||
Comment on lines
+282
to
+283
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused and not necessary, probably you got inspiration from the other regexes at the bottom, but they don't serve a purpose afaik |
||
|
||
// eslint-disable-next-line | ||
export const URL_REGEXP = /^((https?|ftp):\/\/)?(www.)?(((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:)*@)?(((\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5]))|((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)+(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.?)(:\d*)?)(\/((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)+(\/(([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)*)*)?)?(\?((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|[\uE000-\uF8FF]|\/|\?)*)?(\#((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|\/|\?)*)?$/i | ||
|
||
|
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.
is "notes" the best graphic for representing audio? lots of audio might not be music, strictly speaking...
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.
While arguable, audio files are historically represented by a music note, so it's okay to use it.
But @pory-gone you should use an SVG instead of an emoji: https://remixicon.com/icon/file-music-line