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

Resolve #1601 by avoiding redundant allocations #1606

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Resolve #1601 by avoiding redundant allocations #1606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
185318b
* vpx_image_t input_frame in VCSession to avoid alloc|free pair for e…
Mikhael-Danilov Jul 18, 2016
e71902a
fix pointer type mismatch (tests passes now)
Mikhael-Danilov Jul 27, 2016
e3776b4
still need frame data copy here (or we get memory leak on each image …
Mikhael-Danilov Jul 28, 2016
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
18 changes: 1 addition & 17 deletions toxav/toxav.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@
#include <stdlib.h>
#include <string.h>

#define MAX_ENCODE_TIME_US ((1000 / 24) * 1000)

typedef struct ToxAVCall_s {
ToxAV *av;

Expand Down Expand Up @@ -787,21 +785,7 @@ bool toxav_video_send_frame(ToxAV *av, uint32_t friend_number, uint16_t width, u
}

{ /* Encode */
vpx_image_t img;
img.w = img.h = img.d_w = img.d_h = 0;
vpx_img_alloc(&img, VPX_IMG_FMT_I420, width, height, 0);

/* I420 "It comprises an NxM Y plane followed by (N/2)x(M/2) V and U planes."
* http://fourcc.org/yuv.php#IYUV
*/
memcpy(img.planes[VPX_PLANE_Y], y, width * height);
memcpy(img.planes[VPX_PLANE_U], u, (width / 2) * (height / 2));
memcpy(img.planes[VPX_PLANE_V], v, (width / 2) * (height / 2));

int vrc = vpx_codec_encode(call->video.second->encoder, &img,
call->video.second->frame_counter, 1, 0, MAX_ENCODE_TIME_US);

vpx_img_free(&img);
int vrc = vc_encode_frame(call->video.second, y, u, v);

if (vrc != VPX_CODEC_OK) {
pthread_mutex_unlock(call->mutex_video);
Expand Down
40 changes: 38 additions & 2 deletions toxav/video.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
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 •
edited
Loading

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
.


#define VIDEO_DECODE_BUFFER_SIZE 20

VCSession *vc_new(ToxAV *av, uint32_t friend_number, toxav_video_receive_frame_cb *cb, void *cb_data)
Expand Down Expand Up @@ -97,7 +99,13 @@ VCSession *vc_new(ToxAV *av, uint32_t friend_number, toxav_video_receive_frame_c
vpx_codec_destroy(vc->encoder);
goto BASE_CLEANUP_1;
}


vc->input_frame.w = vc->input_frame.h = vc->input_frame.d_w = vc->input_frame.d_h = 0;
if(!vpx_img_alloc(&vc->input_frame, VPX_IMG_FMT_I420, cfg.g_w, cfg.g_h, 0)) {
LOGGER_WARNING("Allocation failed! Application might misbehave!");
goto BASE_CLEANUP_1;
}

vc->linfts = current_time_monotonic();
vc->lcfd = 60;
vc->vcb.first = cb;
Expand All @@ -123,6 +131,8 @@ void vc_kill(VCSession *vc)
vpx_codec_destroy(vc->encoder);
vpx_codec_destroy(vc->decoder);

vpx_img_free(&vc->input_frame);

void *p;

while (rb_read(vc->vbuf_raw, (void **)&p))
Expand Down Expand Up @@ -247,7 +257,7 @@ int vc_reconfigure_encoder(VCSession *vc, uint32_t bit_rate, uint16_t width, uin
LOGGER_ERROR("Failed to initialize encoder: %s", vpx_codec_err_to_string(rc));
return -1;
}

rc = vpx_codec_control(&new_c, VP8E_SET_CPUUSED, 8);

if (rc != VPX_CODEC_OK) {
Expand All @@ -258,7 +268,33 @@ int vc_reconfigure_encoder(VCSession *vc, uint32_t bit_rate, uint16_t width, uin

vpx_codec_destroy(vc->encoder);
memcpy(vc->encoder, &new_c, sizeof(new_c));

vpx_img_free(&vc->input_frame);
vc->input_frame.w = vc->input_frame.h = vc->input_frame.d_w = vc->input_frame.d_h = 0;
if(!vpx_img_alloc(&vc->input_frame, VPX_IMG_FMT_I420, cfg.g_w, cfg.g_h, 0)) {
LOGGER_ERROR("Failed to allocate frame");
return -1;
}
}

return 0;
}

int vc_encode_frame(VCSession *vc, const uint8_t *y, const uint8_t *u, const uint8_t *v)
{
/* I420 "It comprises an NxM Y plane followed by (N/2)x(M/2) V and U planes."
* http://fourcc.org/yuv.php#IYUV
*/

vpx_image_t *img = &vc->input_frame;

int plane_bytes_y = img->w * img->h;
int plane_bytes_uv = plane_bytes_y / 4;

memcpy( img->planes[VPX_PLANE_Y], y, plane_bytes_y);
memcpy( img->planes[VPX_PLANE_U], u, plane_bytes_uv);
memcpy( img->planes[VPX_PLANE_V], v, plane_bytes_uv);

return vpx_codec_encode(vc->encoder, img,
vc->frame_counter, 1, 0, MAX_ENCODE_TIME_US);
}
3 changes: 3 additions & 0 deletions toxav/video.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,15 @@ typedef struct VCSession_s {
PAIR(toxav_video_receive_frame_cb *, void *) vcb; /* Video frame receive callback */

pthread_mutex_t queue_mutex[1];

vpx_image_t input_frame;
} VCSession;

VCSession *vc_new(ToxAV *av, uint32_t friend_number, toxav_video_receive_frame_cb *cb, void *cb_data);
void vc_kill(VCSession *vc);
void vc_iterate(VCSession *vc);
int vc_queue_message(void *vcp, struct RTPMessage *msg);
int vc_reconfigure_encoder(VCSession *vc, uint32_t bit_rate, uint16_t width, uint16_t height);
int vc_encode_frame(VCSession *vc, const uint8_t *y, const uint8_t *u, const uint8_t *v);

#endif /* VIDEO_H */