Skip to content

Conversation

Mikhael-Danilov
Copy link

@Mikhael-Danilov Mikhael-Danilov commented Jul 18, 2016

Resolve #1601 by avoiding redundant allocations

…ach frame

* input_frame allocated with align 0 so copy here is unnecessary
@Mikhael-Danilov Mikhael-Danilov changed the title Resolve #1601 by avoiding reundant allocations Resolve #1601 by avoiding redundant allocations Jul 18, 2016
#include "../toxcore/network.h"

#define MAX_DECODE_TIME_US 0 /* Good quality encode. */
#define MAX_ENCODE_TIME_US ((1000 / 24) * 1000)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this set the minimum FPS of toxav to 24?

Copy link
Author

Choose a reason for hiding this comment

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

  1. It moved from toxav.c as is
  2. As far as I understand it is a hint for encoder to try maintain 24 fps performance
  3. Not sure if it is good idea to have this hardcoded but it out of this PR scope

Copy link
Collaborator

Choose a reason for hiding this comment

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

kk, you're right then, out of scope.

Thanks again for putting so much work into this!

Copy link

@vassad vassad Jul 22, 2016

Choose a reason for hiding this comment

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

@GrayHatter So... Would you mind merging it? :) Or you are not the main merger?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't merge anything that doesn't have @irungentoo's sign off. I also
haven't had time to test it so I haven't been bugging him to merge in
either.

@wiiaam @mannol @subliun any of you had a chance to test this yet?

On Jul 22, 2016 03:51, "vassad" [email protected] wrote:

In toxav/video.c
#1606 (comment):

@@ -34,6 +34,8 @@
#include "../toxcore/network.h"

#define MAX_DECODE_TIME_US 0 /* Good quality encode. */
+#define MAX_ENCODE_TIME_US ((1000 / 24) * 1000)

So... Would you mind merging it? :) Or you are not the main merger?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/irungentoo/toxcore/pull/1606/files/185318bbce339f1ee908a46c3f4acfdae93a9198#r71861045,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAO20JyofDW4RUAQ5_pnfHcuoH1WZYZOks5qYKCmgaJpZM4JPAmO
.

@Mikhael-Danilov
Copy link
Author

Sadly but looks like we still need frame-data copy here ( only if we not going to provide each libvpx ABI variant using VPX_IMAGE_ABI_VERSION macro )

@GrayHatter
Copy link
Collaborator

anyone still watching this thread, consider opening on https://github.com/TokTok/c-toxcore/

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