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.

Sign up for GitHub

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

Auto filter end tags, add 1 click offset #75

Merged
merged 4 commits into from
Oct 17, 2024
Merged

Auto filter end tags, add 1 click offset #75

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
093125f
add filtering
t3dotgg Oct 16, 2024
63b7874
filter on click of offset label, show tags
t3dotgg Oct 16, 2024
1e76d78
Update src/app/(core)/v/[slug]/player.tsx
t3dotgg Oct 17, 2024
315ae19
Update src/app/(core)/v/[slug]/player.tsx
t3dotgg Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
232 changes: 175 additions & 57 deletions src/app/(core)/v/[slug]/player.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,129 @@ import type { VOD } from "~/utils/twitch-server";
import { Player } from "~/utils/types/twitch-player";
dayjs.extend(duration);

function parseOffsetValue(value: string): number | undefined {
// if there are no colons, assume its seconds
if (/^\d+$/.test(value)) return parseInt(value, 10);

// Supports HH:MM:SS, MM:SS, SS
// If it's not in the format, return undefined
if (!/^([0-5]?[0-9]:){0,2}[0-5][0-9]$/.test(value)) return undefined;

return value
.split(":")
.reduce((acc, cur) => (acc = acc * 60 + parseInt(cur, 10)), 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid assignment within expression in reduce callback

The assignment within the reduce function can be confusing and is unnecessary. Assignments in expressions may lead to unintended side effects and reduce code clarity.

Apply this diff to remove the assignment:

-    .reduce((acc, cur) => (acc = acc * 60 + parseInt(cur, 10)), 0);
+    .reduce((acc, cur) => acc * 60 + parseInt(cur, 10), 0);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.reduce((acc, cur) => (acc = acc * 60 + parseInt(cur, 10)), 0);
.reduce((acc, cur) => acc * 60 + parseInt(cur, 10), 0);
🧰 Tools
🪛 Biome

[error] 24-24: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)

}
Comment on lines +14 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Good implementation of time parsing, with a minor improvement needed

The parseOffsetValue function effectively handles various time formats (seconds, MM:SS, HH:MM:SS) and converts them to total seconds. The use of regex for input validation is appropriate.

However, there's a minor issue in the reduce function. Apply this diff to remove the unnecessary assignment:

-    .reduce((acc, cur) => (acc = acc * 60 + parseInt(cur, 10)), 0);
+    .reduce((acc, cur) => acc * 60 + parseInt(cur, 10), 0);

This change improves clarity and avoids potential side effects from assignments in expressions.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function parseOffsetValue(value: string): number | undefined {
// if there are no colons, assume its seconds
if (/^\d+$/.test(value)) return parseInt(value, 10);
// Supports HH:MM:SS, MM:SS, SS
// If it's not in the format, return undefined
if (!/^([0-5]?[0-9]:){0,2}[0-5][0-9]$/.test(value)) return undefined;
return value
.split(":")
.reduce((acc, cur) => (acc = acc * 60 + parseInt(cur, 10)), 0);
}
function parseOffsetValue(value: string): number | undefined {
// if there are no colons, assume its seconds
if (/^\d+$/.test(value)) return parseInt(value, 10);
// Supports HH:MM:SS, MM:SS, SS
// If it's not in the format, return undefined
if (!/^([0-5]?[0-9]:){0,2}[0-5][0-9]$/.test(value)) return undefined;
return value
.split(":")
.reduce((acc, cur) => acc * 60 + parseInt(cur, 10), 0);
}
🧰 Tools
🪛 Biome

[error] 24-24: The assignment should not be in an expression.

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.

(lint/suspicious/noAssignInExpressions)


// Example marker labels
// "Talking about Chrome" - no metadata, just label
// "START: Talking about Chrome" - start marker, same as above
// "END: Talking about Chrome" - end marker, tagged accordingly so it can be filtered out
// "End of Talking about Chrome" - end marker, same as above
// "OFFSET 00:40:31" - offset marker

const START_LABELS = ["START:", "START OF", "START"];
const END_LABELS = ["END:", "END OF", "END"];
const OFFSET_LABELS = ["OFFSET", "OFFSET:"];
const LABELS = [...START_LABELS, ...END_LABELS, ...OFFSET_LABELS];

function parseMetadataFromMarker(marker: string) {
for (const tl of START_LABELS) {
if (marker.toLowerCase().startsWith(tl.toLowerCase())) {
return {
type: "start",
label: marker.slice(tl.length).trim(),
};
}
}

for (const tl of END_LABELS) {
if (marker.toLowerCase().startsWith(tl.toLowerCase())) {
return {
type: "end",
label: marker.slice(tl.length).trim(),
};
}
}

for (const tl of OFFSET_LABELS) {
if (marker.toLowerCase().startsWith(tl.toLowerCase())) {
return {
type: "offset",
label: marker.slice(tl.length).trim().replaceAll("-", ":"),
};
}
}

return {
type: "start",
label: marker,
};
}

function parseMarkers(props: { vod: VOD; offset?: { totalSeconds: number } }) {
const videoDuration = getDurationFromTwitchFormat(
(props.vod as any)?.duration ?? "0h0m0s"
);

const mockedMarkers = [
{ position_seconds: 0, id: "start", description: "Intro" },
...props.vod.markers,
];

const OFFSET = props.offset?.totalSeconds ?? 0;

const taggedMarkers = mockedMarkers.map((marker, id) => {
let endTime =
(mockedMarkers[id + 1]?.position_seconds ??
(videoDuration as duration.Duration)?.asSeconds?.()) - OFFSET;

endTime += EXPORT_BUFFER;

if (endTime < 0) endTime = 1;

const startTime = Math.max(
marker.position_seconds - OFFSET - EXPORT_BUFFER,
0
);

const duration = dayjs
.duration(endTime * 1000 - startTime * 1000)
.format("HH:mm:ss");

const taggedDescription = parseMetadataFromMarker(marker.description);

return {
startTime,
endTime,
duration,
...taggedDescription,
};
});

const filteredMarkers = taggedMarkers.filter(
(m) => m.type === "start" || m.type === "offset"
);

const ytChapters = filteredMarkers.reduce((acc, marker) => {
const startTime = new Date(marker.startTime * 1000);
const timeStr = startTime.toISOString().substr(11, 8);
return `${acc}${marker.label} - ${timeStr}\n`;
}, "");

const csv = filteredMarkers
.map((marker) => {
return `${marker.startTime},${marker.endTime},${marker.label}`;
})
.join("\n");

return {
taggedMarkers,
filteredMarkers,
ytChapters,
csv,
};
}

// Converts "8h32m12s" format into dayjs duration
// I absolutely hate this function and would love ANYTHING better
const getDurationFromTwitchFormat = (input: string) => {
Expand Down Expand Up @@ -81,62 +204,25 @@ export const VodPlayer = (props: { id: string; vod: VOD }) => {
return cleanup;
}, [props.id]);

const videoDuration = props.vod
? getDurationFromTwitchFormat((props.vod as any)?.duration ?? "0h0m0s")
: "n/a";

const mockedMarkers = [
{ position_seconds: 0, id: "start", description: "Intro" },
...props.vod.markers,
];

const [offset, setOffset] = useState<{
presentational: string;
totalSeconds: number;
}>({
presentational: "0",
totalSeconds: 0,
});
const csv = mockedMarkers.flatMap((marker, id) => {
let endTime =
(mockedMarkers[id + 1]?.position_seconds ??
(videoDuration as duration.Duration)?.asSeconds?.()) -
offset.totalSeconds;

endTime += EXPORT_BUFFER;
const parsedMarkers = useMemo(() => {
if (!props.vod) return;

if (endTime < 0) endTime = 1;

const startTime = Math.max(
marker.position_seconds - offset.totalSeconds - EXPORT_BUFFER,
0
);

if (marker.description.toLowerCase().startsWith("end of")) return [];

return [`${startTime},${endTime},${marker.description.replace(",", "")}`];
});

const ytChapters = mockedMarkers.reduce((acc, marker) => {
const startTime = new Date(
(marker.position_seconds - offset.totalSeconds) * 1000
);
const timeStr = startTime.toISOString().substr(11, 8);
return `${acc}${marker.description} - ${timeStr}\n`;
}, "");
return parseMarkers({
vod: props.vod,
offset: offset,
});
}, [props, offset]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize dependencies in useMemo hook

Including props in the dependency array can cause unnecessary recomputations. It's better to specify the exact properties that affect the memoization.

Apply this diff to adjust the dependencies:

-  }, [props, offset]);
+  }, [props.vod, offset]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const parsedMarkers = useMemo(() => {
if (!props.vod) return;
if (endTime < 0) endTime = 1;
const startTime = Math.max(
marker.position_seconds - offset.totalSeconds - EXPORT_BUFFER,
0
);
if (marker.description.toLowerCase().startsWith("end of")) return [];
return [`${startTime},${endTime},${marker.description.replace(",", "")}`];
});
const ytChapters = mockedMarkers.reduce((acc, marker) => {
const startTime = new Date(
(marker.position_seconds - offset.totalSeconds) * 1000
);
const timeStr = startTime.toISOString().substr(11, 8);
return `${acc}${marker.description} - ${timeStr}\n`;
}, "");
return parseMarkers({
vod: props.vod,
offset: offset,
});
}, [props, offset]);
const parsedMarkers = useMemo(() => {
if (!props.vod) return;
return parseMarkers({
vod: props.vod,
offset: offset,
});
}, [props.vod, offset]);


function parseOffsetValue(value: string): number | undefined {
// if there are no colons, assume its seconds
if (/^\d+$/.test(value)) return parseInt(value, 10);

// Supports HH:MM:SS, MM:SS, SS
// If it's not in the format, return undefined
if (!/^([0-5]?[0-9]:){0,2}[0-5][0-9]$/.test(value)) return undefined;

return value
.split(":")
.reduce((acc, cur) => (acc = acc * 60 + parseInt(cur, 10)), 0);
}
if (!parsedMarkers) return null;
const { taggedMarkers, filteredMarkers, ytChapters, csv } = parsedMarkers;

return (
<div className="grid min-h-0 flex-1 grid-rows-3 items-start gap-4 overflow-y-hidden p-4 sm:grid-cols-3 sm:grid-rows-1 sm:gap-8 sm:p-8">
Expand Down Expand Up @@ -168,9 +254,7 @@ export const VodPlayer = (props: { id: string; vod: VOD }) => {
</Button>
<a
className="relative inline-flex items-center rounded border border-gray-700 bg-gray-800 px-4 py-2 text-sm font-medium text-white shadow-sm hover:bg-gray-750 hover:text-gray-100"
href={`data:text/csv;charset=utf-8,${encodeURIComponent(
csv.join("\n")
)}`}
href={`data:text/csv;charset=utf-8,${encodeURIComponent(csv)}`}
{...{
download: `${props.vod?.created_at} VOD MARKERS${
offset.totalSeconds ? ` - ${offset.totalSeconds}s` : ""
Expand Down Expand Up @@ -208,22 +292,56 @@ export const VodPlayer = (props: { id: string; vod: VOD }) => {

{props.vod && (
<ul className="flex min-h-0 flex-1 flex-col gap-2 overflow-y-auto rounded-lg border border-gray-950/25 bg-gray-950/25 p-2 shadow-inner">
{mockedMarkers.map((marker, index) => {
{filteredMarkers.map((marker, index) => {
return (
<li key={marker.id}>
<li key={marker.startTime}>
<button
className="w-full"
onClick={() => {
player?.seek(marker.position_seconds);
if (marker.type === "start") {
player?.seek(marker.startTime);
}

if (marker.type === "offset") {
const offset = parseOffsetValue(marker.label);
if (offset) {
setOffset({
presentational: marker.label,
totalSeconds: offset,
});
}
}
}}
>
<Card className="flex animate-fade-in-down flex-col gap-4 p-4 text-left">
<div className="break-words">{marker.description}</div>
<div className="flex justify-between break-words">
{`${marker.label}`}
<div className="flex flex-col gap-0.5">
<div className="font-mono text-right text-xs text-gray-400">
{`${dayjs
.duration(marker.startTime * 1000)
.format("HH:mm:ss")} - ${dayjs
.duration(marker.endTime * 1000)
.format("HH:mm:ss")}`}
</div>
<div className="font-mono text-right text-xs text-gray-400">
{`(${marker.duration})`}
</div>
</div>
</div>
<div className="flex items-center justify-between text-gray-300">
<div className="text-sm">
{dayjs
.duration(marker.position_seconds * 1000)
.format("HH:mm:ss")}
<div
className={`-m-2 rounded-full px-2 py-1 text-sm font-bold ${
marker.type === "start"
? "bg-green-800 text-white"
: marker.type === "end"
? "bg-red-800 text-white"
: marker.type === "offset"
? "bg-blue-800 text-white"
: "bg-gray-800 text-white"
}`}
Comment on lines +335 to +343
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor nested ternary operator for clarity

The nested ternary operator used for className determination reduces readability. Consider using a mapping object or a helper function to enhance clarity.

Example using a mapping object:

const typeClassMap = {
  start: "bg-green-800 text-white",
  end: "bg-red-800 text-white",
  offset: "bg-blue-800 text-white",
  default: "bg-gray-800 text-white",
};

...

className={`-m-2 rounded-full px-2 py-1 text-sm font-bold ${
  typeClassMap[marker.type] || typeClassMap.default
}`}

>
{marker.type}
</div>
</div>
</Card>
Expand Down
Loading