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

Jump to bottom

LU-3176: optimize getImageDataAtTime #52

Merged
merged 5 commits into from
Jan 11, 2024
Merged

LU-3176: optimize getImageDataAtTime #52

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a79f9b9
feat(*): optimize getImageDataAtTime
stepancar Jan 8, 2024
95234c3
feat(*): optimize getImageDataAtTime
stepancar Jan 8, 2024
9121889
feat(*): rollback to prev implementation getImageDataAtTime
stepancar Jan 8, 2024
ce314b4
feat(*): rollback to prev implementation getImageDataAtTime
stepancar Jan 8, 2024
6b6eaca
refactor(*): add RGBA_PIXEL_SIZE
stepancar Jan 8, 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
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@lumen5/framefusion",
"version": "1.0.1",
"version": "1.0.2",
"type": "module",
"scripts": {
"docs": "typedoc framefusion.ts",
Expand Down
21 changes: 13 additions & 8 deletions src/backends/beamcoder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,22 @@ export class BeamcoderExtractor extends BaseExtractor implements Extractor {
* Get imageData for a given time in seconds
* @param targetTime
*/
async getImageDataAtTime(targetTime: number): Promise<ImageData> {
async getImageDataAtTime(targetTime: number, target?: Uint8ClampedArray): Promise<ImageData> {
stepancar marked this conversation as resolved.
Show resolved Hide resolved
const targetPts = Math.round(this._timeToPTS(targetTime));
VERBOSE && console.log('targetTime', targetTime, '-> targetPts', targetPts);
const frame = await this._getFrameAtPts(targetPts);
if (!frame) {
VERBOSE && console.log('no frame found');
return null;
}
const rawData = this._resizeFrameData(frame);

let rawData = target;

if (!target) {
rawData = new Uint8ClampedArray(frame.width * frame.height * 4);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably unhardcode the 4 here too. It would make the code more self-explanatory.

Copy link
Contributor

Choose a reason for hiding this comment

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

(const components = /* whatever is needed to extract component number from the frame */;)

Copy link
Contributor Author

@stepancar stepancar Jan 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@antoineMoPa , I created constant for that
RGBA_PIXEL_SIZE

}

this._setFrameDataToImageData(frame, rawData);

return {
data: rawData,
Expand Down Expand Up @@ -476,25 +483,23 @@ export class BeamcoderExtractor extends BaseExtractor implements Extractor {
return packet as Packet;
}

_resizeFrameData(frame): Uint8ClampedArray {
_setFrameDataToImageData(frame: beamcoder.Frame, target: Uint8ClampedArray) {
const components = 4; // 4 components: r, g, b and a
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: but should components be passed in? I think it's characteristic to the frame and target and doesn't really seem like something this function should be defining?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can read that from the frame object somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added RGBA_PIXEL_SIZE constant

Copy link
Contributor

Choose a reason for hiding this comment

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

@stepancar so we could not find the frame component number? from frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't get we need to. This code works only with RGBA formate. in our logic it's hardcoded. If we want to support other formats - we need to extend public interface too.

But I don't think it makes sense. We use RGBA everywhere in our system

Copy link
Contributor

Choose a reason for hiding this comment

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

True.

const size = frame.width * frame.height * components;
const rawData = new Uint8ClampedArray(size); // we should probably reuse this buffer
const sourceLineSize = frame.linesize as unknown as number;
// frame.data can contain multiple "planes" in other colorspaces, but in rgba, there is just one "plane", so
// our data is in frame.data[0]
const pixels = frame.data[0] as Uint8Array;

// libav creates larger buffers because it makes their internal code simpler.
// we have to trim a part at the right of each pixel row.

for (let i = 0; i < frame.height; i++) {
const sourceStart = i * sourceLineSize;
const sourceEnd = sourceStart + frame.width * components;
const sourceData = pixels.slice(sourceStart, sourceEnd);
const sourceData = pixels.subarray(sourceStart, sourceEnd);
const targetOffset = i * frame.width * components;
rawData.set(sourceData, targetOffset);
target.set(sourceData, targetOffset);
}
return rawData;
}

async dispose() {
Expand Down