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.

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

LU-3176: optimize getImageDataAtTime #52

Merged
merged 5 commits into from
Jan 11, 2024
Merged

Conversation

stepancar
Copy link
Contributor

@stepancar stepancar commented Jan 8, 2024

It's somthing we wanted to do

  1. we wanted to reuse existing buffer for specific video element instead of re allocating it for every frame

  2. we don't need to copy beamcoder.frame, it's enough for us to create a view of typed array if we can guarantee that beamcoder frame will not be detached,

It improves GC

@stepancar stepancar marked this pull request as draft January 8, 2024 04:48
@stepancar stepancar marked this pull request as ready for review January 8, 2024 05:06
@antoineMoPa
Copy link
Contributor

@stepancar did you check that visual output still makes sense?

@@ -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.

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

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

@stepancar stepancar changed the title feat(*): optimize getImageDataAtTime LU-3168: optimize getImageDataAtTime Jan 10, 2024
Copy link

swarmia bot commented Jan 10, 2024

✅  Linked to Task LU-3176 · Implement outcome from electron exploration
➡️  Part of Epic LU-3060 · FC3: Text Assets

@stepancar stepancar changed the title LU-3168: optimize getImageDataAtTime LU-3176: optimize getImageDataAtTime Jan 10, 2024
@stepancar stepancar merged commit 27531e9 into main Jan 11, 2024
1 check passed
@stepancar stepancar deleted the optimize/getImageDataAtTime branch January 11, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants