webrtcsink icon indicating copy to clipboard operation
webrtcsink copied to clipboard

Published 20 hours ago • verified publisher icon centricular

Added support for memory NVMM

Open massis08 opened this issue 3 years ago • 6 comments
trafficstars

Added support for memory NVMM

massis08 avatar Jul 13 '22 15:07 massis08

Looks good to me otherwise, thanks for taking the time to propose this :+1:

MathieuDuponchelle avatar Jul 19 '22 20:07 MathieuDuponchelle

Ah, also please fix the history to propose a single commit :)

MathieuDuponchelle avatar Jul 19 '22 20:07 MathieuDuponchelle

ping @massis08 :)

MathieuDuponchelle avatar Jul 27 '22 14:07 MathieuDuponchelle

ping @massis08 :)

MathieuDuponchelle avatar Aug 08 '22 20:08 MathieuDuponchelle

Sorry @MathieuDuponchelle, I have been out of the office these past weeks. As soon as I can I will work on the PR!

massis08 avatar Aug 09 '22 09:08 massis08

Great, thanks :)

MathieuDuponchelle avatar Aug 09 '22 11:08 MathieuDuponchelle

hrm, @massis08 this is a bit of a mess :)

MathieuDuponchelle avatar Sep 05 '22 19:09 MathieuDuponchelle

@MathieuDuponchelle I have seen that there as been many modifications to the code. Maybe it is best if I just cancel this PR, and create a new branch, make the necessary changes to support memory NVMM and then create a new PR. What do you think?

massis08 avatar Sep 16 '22 08:09 massis08

@massis08 yes, Merge Requests should be as atomic as possible. Also you should try to answer questions from reviewers, it otherwise makes reviews pretty pointless ;) https://github.com/centricular/webrtcsink/pull/75#pullrequestreview-1044072059

MathieuDuponchelle avatar Sep 16 '22 10:09 MathieuDuponchelle